Fix negative-axis handling in ExpandDims shape inference#29448
Open
tianleiwu wants to merge 3 commits into
Open
Fix negative-axis handling in ExpandDims shape inference#29448tianleiwu wants to merge 3 commits into
tianleiwu wants to merge 3 commits into
Conversation
…rence The com.microsoft.ExpandDims type/shape inference normalized a negative axis with an off-by-two formula (rank + axis - 1), which could yield a negative dimension index and an out-of-bounds read during graph resolution. Normalize against the output rank (rank + axis + 1), and read the scalar axis via ParseScalar so both raw_data and int32_data encodings are handled safely (the previous int32_data()[0] read could go out of bounds when the value is stored as raw_data). Add a regression test that supplies the axis as a constant initializer to exercise the shape-inference path with negative axes.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes com.microsoft.ExpandDims type/shape inference for constant axis inputs by correctly normalizing negative axes against the output rank (rank + 1) and by decoding the scalar axis value safely from either raw_data or int32_data. It also adds a regression test that forces the shape-inference path to run during Graph::Resolve by providing axis as a constant initializer.
Changes:
- Fix negative
axisnormalization to compute the correct insertion index for output rankrank + 1. - Read constant
axisvia the existingParseScalarhelper to avoid out-of-bounds reads when the initializer is stored asraw_data. - Add a regression test that supplies
axisas an initializer to exercise the shape-inference codepath (not just the runtime kernel).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| onnxruntime/core/graph/contrib_ops/contrib_defs.cc | Fix ExpandDims shape inference by safely parsing constant axis and correcting negative-axis normalization against the output rank. |
| onnxruntime/test/contrib_ops/expand_dims_test.cc | Add a constant-initializer axis test helper and a regression test that exercises shape inference during graph resolution. |
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.
PR: Fix negative-axis handling in ExpandDims shape inference
Description
The
com.microsoft.ExpandDimstype/shape-inference function mishandled negativeaxisvalues, which could lead to an out-of-bounds read during graph resolution(
Graph::Resolve). This PR corrects the axis normalization and makes the scalaraxisread robust to both tensor encodings, and adds a regression test thatexercises the shape-inference path.
Summary of Changes
Fix
onnxruntime/core/graph/contrib_ops/contrib_defs.ccaxisagainst the output rank (rank + axis + 1) instead of the off-by-tworank + axis - 1, so the insertion index stays within[0, rank]. Read the scalaraxisvia the existingParseScalarhelper, which handles bothraw_dataandint32_dataencodings and validates the element count.Test
onnxruntime/test/contrib_ops/expand_dims_test.ccExpandDimsTest.NegativeAxisConstInitializerShapeInferenceplus aRunExpandDimsConstAxisTesthelper that suppliesaxisas a constant initializer so the operator's shape-inference function is exercised (the existing tests passaxisas a runtime input, which skips that path).Details
rank + 1dimensions, a negativeaxismust be normalized asaxis + (rank + 1). The previousrank + axis - 1formula produced a negativeinsertion index for the most-negative valid axes, which was then used to index
the protobuf dimension list out of bounds.
int32_data()[0]. When the value isstored as
raw_data(the common encoding for serialized models and the oneproduced by the test harness),
int32_data()is empty and the access is out ofbounds.
ParseScalardecodes either encoding and validates the count.Testing
onnxruntime_provider_testand ran the ExpandDims suite:./onnxruntime_provider_test --gtest_filter="ExpandDimsTest.*"— all 6 tests pass.passes with it.
Checklist