Skip to content

Conversation

tommcdon
Copy link
Member

Fixes #117389
The X86 CONTEXT_FULL thread context flag does not include CONTEXT_FLOATING_POINT, while it is included on x64, arm32, arm64, as well as all other architectures defined in src\coreclr\debug\inc\dbgtargetcontext.h. Interop debugging uses a thread context save/restore mechanism, but was only saving CONTEXT_FULL, causing the i387 float registers to become corrupted on x86. This change sets the CONTEXT_FLOATING_POINTthread context flag so that we correct save and restore floating point numbers on x86.

@tommcdon tommcdon added this to the 10.0.0 milestone Jul 14, 2025
@tommcdon tommcdon requested review from noahfalk and thaystg July 14, 2025 22:55
@tommcdon tommcdon self-assigned this Jul 14, 2025
@Copilot Copilot AI review requested due to automatic review settings July 14, 2025 22:55
Copy link
Contributor

Tagging subscribers to this area: @steveisok, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

@Copilot Copilot AI left a 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 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_POINT to context flags in rsthread.cpp
  • Include DT_CONTEXT_FLOATING_POINT in temp context in process.cpp
  • Update DAC interop context capture in dacdbiimpl.cpp to 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;

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

👍

@tommcdon
Copy link
Member Author

/ba-g failures are unrelated

@tommcdon tommcdon merged commit b212ef0 into dotnet:main Jul 16, 2025
94 of 96 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

x86 Interop step-in causes exception from _controlfp_s in .NET 10 preview 5

5 participants