Fix stackwalk from interpreted EH funclet #116640
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The stack walking for GC is broken when an interpreted EH funclet was on the stack. The reason is that call to the interpreted funclet doesn't have a transition frame and that case was not handled properly. Also, I have found that the ExInfo::m_csfEHClause was not being set for the interpreted clauses at all. This change fixes it by using the InterpreterFrame address as the m_csfEHClause (so that it is above all the interpreter frames) and fixing the `ExInfo::FindParentStackFrameHelper` to recognize the special instruction pointer value that we get as a parent IP for the first interpreted frame under an InterpreterFrame. That special value has been zero and I have added a named constant for it so that it is clear at the points where it is being used.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the broken stack walking for GC when an interpreted EH funclet is on the stack by using the InterpreterFrame address as m_csfEHClause and updating the unwinding logic to properly detect the special caller IP value.
- Defines a special constant (DummyCallerIP) in the InterpreterFrame for clarity.
- Updates exception handling to recognize DummyCallerIP as a marker of interpreted frames.
- Adjusts the funclet call and virtual unwind logic to use the new DummyCallerIP value.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/vm/frames.h | Added a named constant DummyCallerIP to clarify interpreter frames. |
| src/coreclr/vm/exceptionhandling.cpp | Updated logic to check for DummyCallerIP when walking the stack. |
| src/coreclr/vm/eetwain.cpp | Revised funclet call and unwind handling to use DummyCallerIP. |
Comments suppressed due to low confidence (2)
src/coreclr/vm/exceptionhandling.cpp:2790
- Ensure that sufficient test coverage exists for the new execution path when callerIP equals DummyCallerIP to prevent regression in stack walking behavior.
if (callerIP == InterpreterFrame::DummyCallerIP)
src/coreclr/vm/eetwain.cpp:2446
- Verify that using GetFirstArgReg(pContext) to obtain the InterpreterFrame address for setting SP yields the expected result; consider adding a clarifying comment if necessary.
SetIP(pContext, InterpreterFrame::DummyCallerIP);
janvorli
commented
Jun 13, 2025
| @@ -2784,11 +2784,15 @@ StackFrame ExInfo::FindParentStackFrameHelper(CrawlFrame* pCF, | |||
| // Check for out-of-line finally funclets. Filter funclets can't be out-of-line. | |||
| if (!fIsFilterFunclet) | |||
| { | |||
| if (pRegDisplay->IsCallerContextValid) | |||
There was a problem hiding this comment.
This check was reduntant, we always have the parent frame here. The CallerStackFrame::FromRegDisplay asserts that.
jkotas
reviewed
Jun 14, 2025
|
Build breaks look related: |
jkotas
reviewed
Jun 14, 2025
jkotas
reviewed
Jun 16, 2025
jkotas
approved these changes
Jun 16, 2025
Update comment Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The stack walking for GC is broken when an interpreted EH funclet was on the stack. The reason is that call to the interpreted funclet doesn't have a transition frame and that case was not handled properly. Also, I have found that the ExInfo::m_csfEHClause was not being set for the interpreted clauses at all.
This change fixes it by using the InterpreterFrame address as the m_csfEHClause (so that it is above all the interpreter frames) and fixing the
ExInfo::FindParentStackFrameHelperto recognize the special instruction pointer value that we get as a parent IP for the first interpreted frame under an InterpreterFrame.That special value has been zero and I have added a named constant for it so that it is clear at the points where it is being used.