Skip to content

Clamp Shape slice bounds before eliminating Shape - #351

Merged
take-cheeze merged 4 commits into
onnx:mainfrom
Yuhx141:fix-shape-slice-bounds
Oct 2, 2026
Merged

take-cheeze merged 4 commits into
onnx:mainfrom
Yuhx141:fix-shape-slice-bounds

Conversation

@Yuhx141

@Yuhx141 Yuhx141 commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Clamp normalized Shape start/end attributes to the input rank in the shared range helper. This preserves ONNX slicing semantics for bounds outside the rank and prevents eliminate_shape_op from advancing past the stored dimensions.

For a rank-2 input, Shape(start=5) should produce an empty int64[0] tensor. Before this change, the pass computes sizes.cbegin() + 5; an ASan run through onnxsim.simplify reports a heap-buffer-overflow in eliminate_shape_op.h.

Changes

  • Clamp normalized start and end to [0, rank].
  • Treat a reversed forward interval as empty.
  • Add a regression test for start greater than rank.

Validation

  • Built from onnx/optimizer main at c8d77f3.
  • Targeted test: 1 passed, 171 deselected.
  • Native ASan/alignment replay exits 0 after the fix with no sanitizer report.
  • ONNX Runtime 1.30.0 with graph optimizations disabled returns the same empty int64[0] output before and after optimization.

Related: onnxsim/onnxsim#2015

@Yuhx141
Yuhx141 requested review from a team as code owners October 1, 2026 04:58
Signed-off-by: Yuhx141 <155034285+Yuhx141@users.noreply.github.com>
Signed-off-by: Yuhx141 <155034285+Yuhx141@users.noreply.github.com>
Comment thread onnxoptimizer/test/ir_test.py Outdated
Signed-off-by: Yuhx141 <155034285+Yuhx141@users.noreply.github.com>
@Yuhx141

Yuhx141 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

The unrelated sequence-test change remains removed from this Shape fix. I moved the self-contained replacement to #355, where it can be reviewed and tested independently.

@take-cheeze
take-cheeze merged commit 119251b into onnx:main Oct 2, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants