Skip to content

Fix integer overflow in reconstruction cython code#7938

Merged
stefanv merged 2 commits intoscikit-image:mainfrom
jmuhlich:grayreconstruct-cython-locals-int-width
Nov 10, 2025
Merged

Fix integer overflow in reconstruction cython code#7938
stefanv merged 2 commits intoscikit-image:mainfrom
jmuhlich:grayreconstruct-cython-locals-int-width

Conversation

@jmuhlich
Copy link
Copy Markdown
Contributor

@jmuhlich jmuhlich commented Nov 10, 2025

Description

I was experiencing segfaults in the reconstruction_loop cython code when reconstruction was called with very large images, even after #6342 added code to support passing int64 arrays. I discovered that the local variables that hold the values from these arrays are still 32-bit. This change uses the appropriate fused int types instead.

Checklist

Release note

For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.

Avoid potential integer overflow in `skimage.morphology.reconstruction`.

jmuhlich and others added 2 commits November 10, 2025 00:29
While scikit-image#6342 added code to support passing int64 arrays to
reconstruction_loop, the local variables that hold values from these
arrays are still 32-bit. This change uses the appropriate fused int
types instead.
Increases the readability. Naive  profiling suggests that memoryviews
are also slightly faster. We can use those here, because
`negative_indices=False` is equivalent to `@cython.wraparound(False)`
[1].

Also consistently use the same dtype setup here, either `np_ints` or
`np_uints` since these values are added / assigned to each other. Might
save some unnecessary conversations?

[1] https://cython.readthedocs.io/en/3.1.x/src/tutorial/numpy.html#buffer-options
Copy link
Copy Markdown
Member

@lagru lagru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jmuhlich!

I took the opportunity to cleanup the function even further so that it uses the more readable and feature-equivalent memoryviews. Could you confirm that this works for your use case too?

@lagru lagru added the 🩹 type: Bug fix Fixes unexpected or incorrect behavior label Nov 10, 2025
@jmuhlich
Copy link
Copy Markdown
Contributor Author

Confirmed -- your version works fine for me.

@stefanv stefanv merged commit e3d19b2 into scikit-image:main Nov 10, 2025
26 of 27 checks passed
@stefanv
Copy link
Copy Markdown
Member

stefanv commented Nov 10, 2025

Thanks @jmuhlich!

@stefanv stefanv added this to the 0.26 milestone Nov 10, 2025
@jmuhlich jmuhlich deleted the grayreconstruct-cython-locals-int-width branch November 11, 2025 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🩹 type: Bug fix Fixes unexpected or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants