Skip to content

JIT: check operand def chains for overflow in RangeCheck::ComputeDoesOverflow - #132849

Merged
EgorBo merged 2 commits into
dotnet:mainfrom
EgorBo:fix-132841
Sep 1, 2026
Merged

JIT: check operand def chains for overflow in RangeCheck::ComputeDoesOverflow#132849
EgorBo merged 2 commits into
dotnet:mainfrom
EgorBo:fix-132841

Conversation

@EgorBo

@EgorBoEgorBo commented Aug 27, 2026

Copy link
Copy Markdown
Member

Fixes#132841

Diffs

The existing path "it never overflows" always looked weird to me, I guess GetRange used to fold branches finally uncovered that as a bug.

…Overflow
Fixesdotnet#132841
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 92793277-4d34-41d6-a4d1-d797cbe34769
CopilotAI lite review requested due to automatic review settings August 27, 2026 18:52
@github-actionsgithub-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 27, 2026
@azure-pipelines

Copy link
Copy Markdown
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.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates CoreCLR JIT range analysis overflow validation so that “non-overflowing” operators (e.g., GT_NEG) also consider overflow in their operand definition chains, and adds a regression test for the miscompile reported in #132841.

Changes:

  • Update RangeCheck::ComputeDoesOverflow to recursively check operand def chains for GT_AND, GT_RSH, GT_RSZ, GT_UMOD, and GT_NEG.
  • Add a new JIT regression test Runtime_132841 and include it in Regression_ro_2.csproj.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

FileDescription
src/coreclr/jit/rangecheck.cppExtend overflow checking to walk operand def chains for certain operators so range-derived folding doesn’t rely on overflow-unsafe facts.
src/tests/JIT/Regression/Regression_ro_2.csprojAdd the new regression test source file to the merged test project.
src/tests/JIT/Regression/JitBlue/Runtime_132841/Runtime_132841.csAdd a targeted xUnit regression test reproducing the Math.Abs folding miscompile scenario.

Comment threadsrc/coreclr/jit/rangecheck.cpp
@AndyAyersMS

Copy link
Copy Markdown
Member

Validated this against the randomized generator that found #132841. Re-ran with identical seed bases before and after applying the JIT hunk here (x64 checked, tier0_min / fullopts / tier1_pgo / fullopts_no_poap):

Program shapeSeedsFail beforeFail afterNewly broken
Math.Abs guard folded to a throw50013300
((-v) < 0) folded to a wrong value50022200
Total1,00035500

Plus 450 (and counting) trials on fresh seeds with the patch applied, all clean — that shape failed ~44% of the time before, so this is covering the whole generated distribution (ADD/MUL/SHL wraps × three guard forms × nine consumers), not just the two reduced repros.

The GT_NEG arm is what covers the wrong-value case I commented with on the issue. Since Runtime_132841.cs here only exercises the throwing variant, it might be worth adding that one too — it guards a different consumer shape (GT_NEG feeding a compare, rather than Math.Abs's internal int.MinValue guard), so a regression that only affected non-throwing consumers would slip past the current test.

CopilotAI review requested due to automatic review settings August 31, 2026 17:18
@EgorBo

Copy link
Copy Markdown
MemberAuthor

@AndyAyersMS PTAL, had to restart CI to fix merge conflict in csproj. A few regressions, correctness related.

The existing path "it never overflows" always looked weird to me, I guess GetRange used to fold branches finally uncovered that as a bug.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@EgorBo
EgorBo enabled auto-merge (squash) September 1, 2026 11:18
@EgorBo

Copy link
Copy Markdown
MemberAuthor

/ba-g infra

@EgorBo
EgorBo merged commit c2af89b into dotnet:mainSep 1, 2026
137 of 141 checks passed
@dotnet-milestone-botdotnet-milestone-botBot added this to the 12.0-preview1 milestone Sep 2, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JIT: RangeCheck keeps a stale range when assertion tightening yields an empty range, causing bad conditional folding

3 participants

@EgorBo@AndyAyersMS