Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Use rangecheck in assertprop (#112766)" #112872

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

AndyAyersMS
Copy link
Member

This reverts commit 6022adf.

Suspect this is causing a lot of the recent jitstress failures. We'll see.

@Copilot Copilot bot review requested due to automatic review settings February 24, 2025 17:44

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (3)
  • src/coreclr/jit/assertionprop.cpp: Language not supported
  • src/coreclr/jit/rangecheck.cpp: Language not supported
  • src/coreclr/jit/rangecheck.h: Language not supported
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 24, 2025
Copy link
Contributor

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

@AndyAyersMS
Copy link
Member Author

/azp run runtime-coreclr libraries-pgo, runtime-coreclr jitstress

1 similar comment
@AndyAyersMS
Copy link
Member Author

/azp run runtime-coreclr libraries-pgo, runtime-coreclr jitstress

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jakobbotsch
Copy link
Member

/azp run runtime-coreclr libraries-jitstress

@AndyAyersMS
Copy link
Member Author

Jitstress looks good.

Libraries overall looks good. Failures mainly look like #112856

but there are some others in there too:

Assert failure(PID 9076 [0x00002374], Thread: 8544 [0x2160]): Consistency check failed: 
FAILED: pThread->PreemptiveGCDisabled()

CORECLR! CHECK::Trigger + 0x1C4 (0x00007ff9`7e36416c)
CORECLR! HijackHandler + 0x2EC (0x00007ff9`7e27f554)
NTDLL! chkstk + 0x84 (0x00007ff9`dfbbeb44)
NTDLL! RtlPcToFileHeader + 0xF60 (0x00007ff9`dfaa37e0)
<no module>! <no symbol> + 0x0 (0x082a7ff9`dfaa25f4)
    File: D:\a\_work\1\s\src\coreclr\vm\exceptionhandling.cpp:6083

@AndyAyersMS
Copy link
Member Author

Reverting this PR seems to clear up a lot of the new weekend failures.

@EgorBo @dotnet/jit-contrib PTAL

@EgorBo
Copy link
Member

EgorBo commented Feb 24, 2025

With @jakobbotsch's help we identified the root cause, it is not exactly in my PR and was preexisting so is not related to my change, my change just exposed it (mainly, under CSE-related stress modes, but may repro without them too).

@halter73
Copy link
Member

@AndyAyersMS could this be the cause of the index out of bounds errors we're seeing in this dependency update? Assuming the regression was caused by a runtime update, it seems to be caused by something between these two commits: 57ed25449ab954595130997679807a471ef85aef..a47ba19bcd61cb271cad74ae850dd1670cc9675b

@EgorBo
Copy link
Member

EgorBo commented Feb 25, 2025

@AndyAyersMS could this be the cause of the index out of bounds errors we're seeing in this dependency update? Assuming the regression was caused by a runtime update, it seems to be caused by something between these two commits: 57ed25449ab954595130997679807a471ef85aef..a47ba19bcd61cb271cad74ae850dd1670cc9675b

Yes, it could

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.

5 participants