Correctly restore floating point context on x86 interop step-in scenarios#117632
Merged
tommcdon merged 3 commits intodotnet:mainfrom Jul 16, 2025
Merged
Conversation
Contributor
|
Tagging subscribers to this area: @steveisok, @dotnet/dotnet-diag |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that the floating-point registers are correctly saved and restored on x86 interop step-in scenarios by including the CONTEXT_FLOATING_POINT flag wherever the thread context is captured.
- Add
DT_CONTEXT_FLOATING_POINTto context flags inrsthread.cpp - Include
DT_CONTEXT_FLOATING_POINTin temp context inprocess.cpp - Update DAC interop context capture in
dacdbiimpl.cppto include floating-point flags
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/debug/di/rsthread.cpp | Include DT_CONTEXT_FLOATING_POINT when setting up thread contexts |
| src/coreclr/debug/di/process.cpp | Add DT_CONTEXT_FLOATING_POINT to temporary context for restore |
| src/coreclr/debug/daccess/dacdbiimpl.cpp | Pass CONTEXT_FLOATING_POINT (and extended registers) to DAC call |
Comments suppressed due to low confidence (2)
src/coreclr/debug/di/rsthread.cpp:3709
- [nitpick] Consider adding a brief comment explaining why CONTEXT_FLOATING_POINT is now required on x86 to clarify the rationale for future maintainers.
context.ContextFlags = DT_CONTEXT_FULL | DT_CONTEXT_FLOATING_POINT | DT_CONTEXT_EXTENDED_REGISTERS;
src/coreclr/debug/di/process.cpp:13284
- Add a unit or integration test that verifies floating-point register preservation on x86 interop step-in scenarios to guard against regressions.
tempContext.ContextFlags = DT_CONTEXT_FULL | DT_CONTEXT_FLOATING_POINT | DT_CONTEXT_EXTENDED_REGISTERS;
hoyosjs
approved these changes
Jul 15, 2025
thaystg
approved these changes
Jul 15, 2025
This was referenced Jul 15, 2025
Open
Member
Author
|
/ba-g failures are unrelated |
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #117389
The
X86 CONTEXT_FULLthread context flag does not includeCONTEXT_FLOATING_POINT, while it is included on x64, arm32, arm64, as well as all other architectures defined insrc\coreclr\debug\inc\dbgtargetcontext.h. Interop debugging uses a thread context save/restore mechanism, but was only savingCONTEXT_FULL, causing the i387 float registers to become corrupted on x86. This change sets theCONTEXT_FLOATING_POINTthread context flag so that we correct save and restore floating point numbers on x86.