-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Set of interpreter related fixes for coreclr tests #118617
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
Set of interpreter related fixes for coreclr tests #118617
Conversation
The changes in this PR were made to fix various issues uncovered by the coreclr tests. * Handle case when a try block is eliminated because it can never be executed. * Ensure call stubs are generated for IL stubs. * Fix shadow stack updating during stack walk * Fix handling managed exceptions thrown as SEH from managed code called by JIT / Interpreter compiler in interpreted code. * Fix resuming after catch in interpreted code for cases when interpreted code calls another interpreted code via a prestub, which currently happens for delegates. * Ensure that ip is saved to the pFrame->ip before calling native methods that can throw to make sure that the stack walk reports correct return IP.
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
There was a problem hiding this 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 PR fixes various issues discovered by the coreclr tests related to interpreter functionality, focusing on exception handling, stack management, and IL stub generation. The changes primarily address edge cases in exception propagation and ensure proper state management during method calls.
Key changes:
- Adds proper handling for managed exceptions thrown from PrepareInitialCode calls
- Fixes shadow stack pointer (SSP) updating during stack walks in interpreter frames
- Ensures call stubs are generated for IL stubs when using the interpreter
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
src/coreclr/vm/interpexec.cpp | Adds exception handling wrapper, fixes IP saving timing, and improves catch resumption logic |
src/coreclr/vm/frames.cpp | Updates shadow stack pointer during register display updates |
src/coreclr/vm/dllimport.cpp | Ensures call stub creation for interpreted IL stubs |
src/coreclr/interpreter/compiler.cpp | Handles unreachable try blocks and validates leave instruction targets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I'm convinced we have bugs around GC reporting here due to moving the ip early, but that is going to require a trickier fix than I had originally thought. My current thought is that for GC reporting purposes we should do a tweak where we report the conservative merge between the stack before/after the current IP when at the top of a series of interpreter stack frames, but if the InterpMethodContextFrame
currently being reported isn't the active frame on the InterpreterFrame
we can report only the current data.
The changes in this PR were made to fix various issues uncovered by the coreclr tests.