Fix GatherND zero-dimension index validation bug#28006
Merged
Merged
Conversation
Replace `int64_t err_index = 0` sentinel in PrepareForCompute with `std::atomic<const Tind*> invalid_index{nullptr}`. The old sentinel failed to detect out-of-bounds index 0 when a dimension has size 0, since the final check `err_index == 0` treated it as success.
This also fixes a pre-existing data race where multiple threads in TryParallelFor could write to err_index without synchronization.
Add GatherND_zero_dim_error regression test.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes GatherND index validation for zero-sized non-batch dimensions by replacing a sentinel-based error flag with an atomic pointer to the first invalid index, also removing a data race in the parallel validation loop.
Changes:
- Replace
err_indexsentinel withstd::atomic<const Tind*> invalid_indexto correctly detect invalid index0and avoid concurrent writes. - Add a CPU-only regression test for zero-dimension index validation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| onnxruntime/core/providers/cpu/tensor/gather_nd.cc | Fixes parallel index validation to be race-free and correctly report invalid index 0 for zero-sized dims. |
| onnxruntime/test/providers/cpu/tensor/gather_nd_op_test.cc | Adds regression coverage for the zero-dimension invalid index case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Consistent with the other loads/stores in the parallel lambda. The TryParallelFor join provides the happens-before edge.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
adrianlizarraga
approved these changes
Apr 8, 2026
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.
Description
Replace
int64_t err_index = 0sentinel in PrepareForCompute withstd::atomic<const Tind*> invalid_index{nullptr}. The old sentinel failed to detect out-of-bounds index 0 when a dimension has size 0, since the final checkerr_index == 0treated it as success.This also fixes a pre-existing data race where multiple threads in TryParallelFor could write to err_index without synchronization.
Add GatherND_zero_dim_error regression test.
Motivation and Context
Fix GatherND index validation issue.