JIT: guard negative-offset bounds inference against underflow - #133272
JIT: guard negative-offset bounds inference against underflow#133272EgorBo wants to merge 2 commits into
Conversation
Fixes dotnet#133271. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 56f763ef-1e54-421c-9e79-fe14ed2b906a
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
github-actions
Bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, improves correctness by guarding an unsafe inference, and includes a targeted regression test that would fail without the fix.
Pull request overview
This PR fixes a JIT range-check optimization bug where an unsigned compare on an expression with a negative constant offset could be used to infer an incorrect non-negative lower bound even when the addition/subtraction could underflow and wrap. The change strengthens the inference logic to require proof that the offset addition cannot underflow before using it to derive a lower-bound assertion, and adds a regression test covering the reported scenario.
Changes:
- Tighten
MergeEdgeAssertionsWorkernegative-offset inference by requiring a constant lower bound and provinglower + addOpCnscannot overflow/underflow before inferringnormalLclVN >= -addOpCns. - Add an xUnit regression test for the underflow/wrap case that previously led to a missing
IndexOutOfRangeException. - Register the new regression test in
Regression_ro_2.csproj.
File summaries
| File | Description |
|---|---|
| src/coreclr/jit/rangecheck.cpp | Adds an underflow-safety precondition before inferring a lower bound from an unsigned compare involving a negative constant offset. |
| src/tests/JIT/Regression/JitBlue/Runtime_133271/Runtime_133271.cs | New regression test validating the bounds check is not incorrectly removed in the underflow/wrap scenario. |
| src/tests/JIT/Regression/Regression_ro_2.csproj | Includes the new test source file in the regression test project. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The JIT change is a conservative correctness guard and is paired with a focused regression test that would fail under the reported miscompilation.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
PTAL @jakobbotsch @dotnet/jit-contrib - a small correctness issue found by AI. |
Fixes #133271. Require proof that subtraction cannot underflow before inferring the index bound.