Fix CPU segfault in roi_align when a ROI is non-finite (NaN or ±inf) - #9588
Open
ousamabenyounes wants to merge 1 commit into
Open
Fix CPU segfault in roi_align when a ROI is non-finite (NaN or ±inf)#9588ousamabenyounes wants to merge 1 commit into
ousamabenyounes wants to merge 1 commit into
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/9588
Note: Links to docs will display an error until the docs builds have been completed. This comment was automatically generated by Dr. CI and updates every 15 minutes. |
The CPU roi_align forward and backward kernels rely on a bilinear bounds check of the form `if (y < -1.0 || y > height || ...)`. Every comparison against NaN is false, so a ROI whose batch index or coordinates are NaN slips past it; the kernel then casts NaN to int, producing a garbage index that reads out of bounds and segfaults the whole process (and `ceil(NaN)` is UB when sampling_ratio <= 0). +/-inf ROIs hit the same crash by a second path: `roi_end - roi_start` is `inf - inf = NaN`, which then flows into the same non-finite cast. Guarding only on `isnan` (as the first version of this fix did) left the inf case segfaulting, so guard on `std::isfinite` over all five raw ROI values instead — this covers NaN and +/-inf at zero extra cost. The guard is placed on the raw ROI immediately after `offset_rois` is taken, before `int roi_batch_ind = offset_rois[0]` — that cast is itself UB on a non-finite value (a UBSan `float-cast-overflow` build, which the issue reporter uses, flags it even though it does not crash on its own). Skip a non-finite ROI up front, leaving zeros in the output/gradient for it — consistent with how the kernel already treats out-of-bounds sample points as empty. Fixes pytorch#9273
ousamabenyounes
force-pushed
the
fix/issue-9273
branch
from
August 7, 2026 00:26
db56977 to
b3e571e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
roi_alignon CPU segfaults the whole Python process when a ROI contains a non-finite value (NaNor±inf) in its batch index or coordinates. The bilinear bounds checkif (y < -1.0 || y > height || ...)returns false for every NaN comparison, so a malformed ROI slips through, gets cast toint(garbage index), and reads out of bounds.±infreaches the same cast viaroi_end - roi_start = inf - inf = NaN.What changed vs. the first revision
Two points raised in review, both fixed:
std::isnan→std::isfinite. The isnan-only guard let±infROIs through:roi_width = inf - inf = NaN, unclamped whenaligned=True, then the same non-finite cast → segfault (andceil(NaN)withsampling_ratio=-1also feeds a garbagepre_calc.resize).std::isfiniteon the five raw ROI values covers NaN and ±inf at zero extra cost.int roi_batch_ind = offset_rois[0];runs before the old guard and is itself UB on a non-finite value — a UBSanfloat-cast-overflowbuild (which the issue reporter runs) flags it even though it doesn't crash on its own. The guard now sits on the raw ROI immediately afteroffset_roisis taken, before any cast or coordinate math, in both the forward and backward kernels.A non-finite ROI is skipped, leaving zeros in the output/gradient — the output buffers are zero-initialised (
new_zeros), consistent with how the kernel already treats out-of-bounds sample points as empty.Test verification (RED → GREEN)
test/test_ops.py::TestRoIAlign::test_nonfinite_rois_no_segfault— parametrized over{NaN, +inf, -inf} × column {0..4} × dtype{f32,f64,f16} × sampling_ratio{1,-1} × aligned{T,F}(180 cases), asserting finite, all-zero output and a finite input gradient.RED — built on the previous
isnan-only kernel, the new±infcases crash:GREEN — with the
isfinitefix:No regression in the rest of the op tests:
Built CPU-only from source (
pip install -e . --no-build-isolation), torch 2.13.0+cpu. clang-format and flake8/ufmt clean on the touched files.Fixes #9273