-
Notifications
You must be signed in to change notification settings - Fork 5k
Fix the amd64 cross-DAC stack unwinding #114836
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
Conversation
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 reorders the member variables in SoftwareExceptionFrame to correct the memory layout for cross-DAC stack unwinding on amd64.
- Reorders m_Context to appear after m_ContextPointers
- Aligns the struct layout with the expected offsets for Windows and Linux environments
Comments suppressed due to low confidence (1)
src/coreclr/vm/frames.h:1045
- Reordering m_Context to follow m_ContextPointers addresses the offset misalignment for cross-DAC. Please verify that any layout-dependent code (e.g., assembly offsets) has been appropriately reviewed to ensure correctness.
T_CONTEXT m_Context;
Tagging subscribers to this area: @mangod9 |
edd0280
to
ecb426a
Compare
2198ea2
to
58416d2
Compare
Thanks @mikem8361 for the investigations. So the problem is when running on Windows debugging non-windows, right? I wonder if the best solution to the problem is actually to finally move away from the hacky extension of the |
I think as a quick fix of the problem, this PR is fine though. |
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, thank you!
@mikem8361 I have thought I have already approved it, but I've forgotten to push the final button. I am sorry for that. |
Move m_Context to the end of SoftwareExceptionFrame to fix cross-DAC because CONTEXT on Windows defined in winnt.h doesn't contain the extra XSTATE (ymm/zmm/etc) registers that are in the pal.h version used on Linux. This causes causes read failures in SoftwareExceptionFrame:: UpdateRegDisplay_Impl when reading the m_ContextPointers because the offsets are off in the cross-DAC.
It looks like this has always been the case but we have never had some with this order:
Fixing the CONTEXT definition would be the best fix, but I couldn't figure out how to get the cross-DAC build to use pal.h's CONTEXT. FaultingExceptionFrame also has this problem where
T_CONTEXT m_ctx
is defined beforem_SSP
on amd64 but some assembly code offsets need to be adjusted. If we are ok with this approach then I'll make those changes.