Validate GridSample input spatial dimensions are non-empty - #29255
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens input validation for the GridSample operator by rejecting malformed inputs where any input spatial dimension is zero, which previously could bypass the “empty output” early return (since output shape is derived from the grid) and reach interpolation with invalid spatial extents.
Changes:
- Add input-shape validation in CPU
GridSample::Computeto reject any zero-size spatial dimension. - Add equivalent validation in CUDA
GridSample::ComputeInternal, handling both NCHW and NHWC layouts for 4-D and 5-D inputs. - Add an expect-failure unit test for the zero-spatial-dimension case, scoped to CPU and CUDA EPs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| onnxruntime/core/providers/cpu/tensor/grid_sample.cc | Adds runtime validation to reject zero-size spatial dimensions before sampling/interpolation. |
| onnxruntime/core/providers/cuda/tensor/grid_sample.cc | Adds the same validation for CUDA (layout-aware for NCHW/NHWC, 4-D/5-D). |
| onnxruntime/test/providers/cpu/tensor/grid_sample_test_custom.cc | Adds an expect-failure test and helpers to run it only on EPs that implement the guard (CPU/CUDA). |
f96a571 to
c975c45
Compare
|
Issues found In grid_sample.cc:374-382, the existing checks use ORT_ENFORCE (throws on failure), but the new check uses ORT_RETURN_IF_NOT (returns error Status). This is actually the correct choice for the new code — ORT_RETURN_IF_NOT is preferable for input validation since it returns a clean Status rather than throwing. However, it means that this kernel mixes error-handling styles: the preceding rank/grid-consistency checks all ORT_ENFORCE, which will abort if exceptions are disabled. The new code is fine as-is; the old code is the problem, not this PR.
Derives output from grid shape (H_out, W_out) Passes H_in/W_in as shader uniforms — if they are 0, the gs_pixel function's bounds checks (r >= i32(uniforms.H_in)) will treat every coordinate as OOB, which for padding_mode=zeros returns 0 (safe), for padding_mode=border clamps to clamp(r, 0, -1) → undefined behavior in WGSL (or at least incorrect), and for padding_mode=reflection divides by 0 in range = v_max - v_min (e.g., f32(0) - 1.0 = -1.0, then dv / range with negative range). The PR description acknowledges this with "WebGPU/CoreML use separate kernels without that guard, so the expect-failure test below must exclude them." This is acceptable as a scoped fix but the WebGPU EP should get the same guard in a follow-up. Missing testsZero W instead of zero H — e.g., {1,1,5,0} to verify the loop catches non-first spatial dimensions padding_mode=zeros — The tests only use border. Adding a zeros variant ensures the guard fires before the interpolation regardless of padding mode. |
Pull request was closed
…UDA kernels) Add ORT_RETURN_IF_NOT guards that every input spatial dimension is greater than zero in GridSample::Compute (CPU) and GridSample::ComputeInternal (CUDA), covering 2D and 3D-spatial inputs in both NCHW and NHWC layouts. The output is sized by the grid, so a zero-size input spatial dimension would otherwise skip the empty-output early return and reach interpolation with an invalid extent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GridSample sizes its output from the grid, so a zero-size input spatial dimension skips the empty-output early return and reaches the clamp/reflect index computation with an invalid extent. Add an ORT_RETURN_IF_NOT check that every input spatial dimension is greater than zero in both the CPU Compute and the CUDA ComputeInternal (covering 2D and 3D-spatial inputs, NCHW and NHWC layouts), with a consistent diagnostic message. Add an expect-failure OpTester case that fans across the registered CPU and CUDA execution providers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Agent-signed-off: Developer (cc69a935) [claude-opus-4.8 via copilot] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply the same non-empty input spatial-dimension check already present in the
CPU and CUDA GridSample kernels to the WebGPU kernel (opset 16-19, NCHW 4-D):
reject an input whose spatial dimensions contain a zero size before dispatch,
using the same message, so the output-from-grid path cannot produce invalid
sample indices.
Broaden the regression tests to cover the cases requested in review:
- a zero trailing spatial dimension ({1,1,5,0}), which also exercises the CUDA
NHWC layout path,
- all spatial dimensions zero ({1,1,0,0}),
- a 5-D input with a zero in a non-first spatial dimension ({1,1,2,0,3}),
- padding_mode "zeros" and "reflection" variants of the rejection,
- a valid non-empty input that must still succeed (no false rejection),
- a WebGPU-only opset-16 rejection test guarded by USE_WEBGPU that runs on the
WebGPU provider where the new guard applies.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
778a37c to
2a3a060
Compare
Tianlei Wu (tianleiwu)
left a comment
There was a problem hiding this comment.
Overall this is a clean, well-tested input-validation hardening: the non-empty spatial-dimension guard is applied consistently across the CPU, CUDA, and WebGPU kernels with a shared, descriptive error message, and the expect-failure tests cover 4-D/5-D, leading/trailing/middle zero dims, and all padding modes, plus a positive sanity case. A few non-blocking notes below.
-
Placement relative to the empty-output early return (inline on CPU/CUDA): the guard runs before the existing
if (Y.Shape().Size() == 0) return Status::OK();short-circuit, so a zero-size input spatial dim combined with a zero-size grid/output — which previously returned OK — now errors. Since no sampling happens when the output is empty, consider placing the guard after that early return to preserve the existing zero-size-output behavior. Non-blocking. -
PR description scope: the "Changes"/"Motivation" sections mention only the CPU and CUDA kernels, but this PR also adds the same guard to
onnxruntime/core/providers/webgpu/tensor/grid_sample.ccand a WebGPU expect-failure test. Worth updating the description to reflect the WebGPU change so it matches the diff.
(The earlier automated suggestion about returning INVALID_ARGUMENT explicitly vs. ORT_RETURN_IF_NOT is still a reasonable consistency point but is left to author discretion.)
Pull request was closed
7bde5d7
into
microsoft:main
Description
GridSamplesizes its output from the grid, so an input with a zero-size spatial dimension skips the empty-output early return and reaches interpolation with an invalid spatial extent. This change validates that every input spatial dimension is non-empty in both the CPU and CUDA kernels, returning a clear error for such malformed inputs.Changes
GridSample::Compute: reject any zero-size input spatial dimension with a descriptive error.GridSample::ComputeInternal: same validation, layout-aware for NCHW and NHWC, covering both 4-D (H, W) and 5-D (D, H, W) inputs. Messages are consistent across both EPs.padding_mode=border) scoped to the CPU and CUDA execution providers, which are the kernels that carry the guard.Motivation
Improves input validation and error diagnostics for malformed
GridSampleinputs. Covers both the CPU and CUDA execution providers; no behavior change for valid inputs.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com