Skip to content

Track span.Length's negativity + remove unsound assumption from MergeEdgeAssertions#124387

Merged
EgorBo merged 4 commits intodotnet:mainfrom
EgorBo:track-span-len-negativity
Feb 16, 2026
Merged

Track span.Length's negativity + remove unsound assumption from MergeEdgeAssertions#124387
EgorBo merged 4 commits intodotnet:mainfrom
EgorBo:track-span-len-negativity

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented Feb 13, 2026

This PR does:

  • Remove an unsound assumption from MergeEdgeAssertions (IsVnCheckedBound = non-negative).
    • IsVnCheckedBound just consults with a global VN hash of VNs ever participating in BoundsCheck's length arg. While in reality we can only rely on such VNs in certain contexts, e.g. after BoundsCheck nodes. Fixes AVE instead of IndexOutOfRangeException #124404
  • Track span.Length's non-negativity via assertions (won't work for non-promoted spans).
    • This is needed to mitigate some of the regressions from (1)
  • Support VNF_Not unary op in GetRangeFromAssertions
  • Add more debug checks to AssertionDsc

Diffs - some regressions from the removed IsVnCheckedBound hack and now non-promoted Spans are less likely to be recognized. Likely to be fixed when the legacy promotion is gone?

…EdgeAssertions + support VNF_Not in GetRangeFromAssertions
Copilot AI review requested due to automatic review settings February 13, 2026 15:20
@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 13, 2026
@dotnet-policy-service
Copy link
Contributor

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

@EgorBo EgorBo marked this pull request as ready for review February 13, 2026 15:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request enhances range check analysis in the JIT compiler by:

  • Adding support for tracking span.Length's non-negativity through assertions
  • Adding support for VNF_NOT (logical not) operations in range analysis
  • Removing unsound assumptions from MergeEdgeAssertions that incorrectly used checked bounds
  • Adding debug-only assertions to AssertionDsc to catch mode confusion between local and global assertion propagation

Changes:

  • Added RangeOps::Not() for logical negation on constant boolean ranges
  • Added VNF_NOT case to GetRangeFromAssertions with proper range computation
  • Removed unsafe code in MergeEdgeAssertions that made unsound assumptions about checked bounds
  • Added debug-only m_compiler fields to AssertionDscOp1/Op2 with mode-checking assertions in getters
  • Removed unused O2K_INVALID enum value
  • Added CreateEmptyAssertion factory to ensure proper initialization of debug fields
  • Added GT_LCL_VAR case in optAssertionGen to generate assertions for never-negative locals

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/coreclr/jit/rangecheck.h Added RangeOps::Not() method for logical negation on boolean ranges
src/coreclr/jit/rangecheck.cpp Added VNF_NOT support in GetRangeFromAssertions; removed unsound assumptions in MergeEdgeAssertions
src/coreclr/jit/compiler.h Added debug assertions to AssertionDsc with CreateEmptyAssertion factory; removed O2K_INVALID
src/coreclr/jit/assertionprop.cpp Added GT_LCL_VAR case for never-negative locals; updated factory method calls to pass compiler

@EgorBo EgorBo closed this Feb 13, 2026
@EgorBo EgorBo reopened this Feb 13, 2026
@EgorBo
Copy link
Member Author

EgorBo commented Feb 13, 2026

PTAL @jakobbotsch I remember you pointed me to that assumption. cc @dotnet/jit-contrib

@EgorBo EgorBo requested a review from jakobbotsch February 13, 2026 18:06
@EgorBo
Copy link
Member Author

EgorBo commented Feb 13, 2026

Managed to come up with a bug-repro that this PR fixes: #124404

Copilot AI review requested due to automatic review settings February 16, 2026 14:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/jit/rangecheck.cpp:712

  • RangeOps::Not() method does not exist. This code calls RangeOps::Not(r1) but the RangeOps struct in rangecheck.h only defines methods like Negate, Add, Subtract, Multiply, etc. There is no Not method implemented. This will cause a compilation error. You need to either implement RangeOps::Not() or remove the VNF_NOT case from this switch statement.

@EgorBo
Copy link
Member Author

EgorBo commented Feb 16, 2026

/ba-g filed #124487 for the failure

@EgorBo EgorBo enabled auto-merge (squash) February 16, 2026 23:12
@EgorBo EgorBo merged commit 8e49d6b into dotnet:main Feb 16, 2026
122 of 125 checks passed
@EgorBo EgorBo deleted the track-span-len-negativity branch February 16, 2026 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR 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.

AVE instead of IndexOutOfRangeException

2 participants