Skip to content

[cDAC] X86 support HijackFrame #116829

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

Merged
merged 2 commits into from
Jun 20, 2025
Merged

Conversation

max-charlamb
Copy link
Contributor

Contributes to #114019

Verified using the following script to get a [HijackFrame] on the stack:

class HijackTest()
{
    public volatile bool flag;
    public volatile int num;

    // Set breakpoint at ThreadSuspend::SuspendEE then step out and look at the main thread stack
    // (bu coreclr!ThreadSuspend::SuspendEE)
    // Note: HijackFrames are not used on Windows if CET is enabled. Either test on non-Windows
    // or disable CET by modifying Thread::AreShadowStacksEnabled to return false.
    [MethodImpl(MethodImplOptions.AggressiveOptimization)]
    public void Test()
    {
        // start other thread that will force a GC collection.
        Task.Run(Work);

        // run loop checking volatile variable to generate non-interruptible code.
        while (!flag)
        {
            TestLoop();
        }
    }
    public void Work()
    {
        Thread.Sleep(500);
        GC.Collect();
    }

    [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
    public void TestLoop()
    {
        num++; num++; num++; num++; num++;
        num++; num++; num++; num++; num++;
        num++; num++; num++; num++; num++;
        num++; num++; num++; num++; num++;
        num++; num++; num++; num++; num++;
        num++; num++; num++; num++; num++;
    }
}

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 adds support for capturing and handling x86 hijack frames in the DataContractReader, extending both the data contract and the frame handler, plus updating the core runtime descriptor and documentation.

  • Introduce HijackArgsX86 to read x86-specific hijack registers into a dictionary
  • Implement HandleHijackFrame in X86FrameHandler to restore registers and adjust the stack pointer
  • Extend datadescriptor.h for x86 register fields and update docs to include x86 HijackArgs

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/native/managed/cdac/.../Data/Frames/HijackArgsX86.cs Add x86-specific HijackArgsX86 data reader
src/native/managed/cdac/.../StackWalk/FrameHandling/X86FrameHandler.cs Implement HandleHijackFrame for x86
src/coreclr/debug/runtimeinfo/datadescriptor.h Add x86 register fields (Edi, Esi, etc.)
docs/design/datacontracts/StackWalk.md Update docs table to include x86 in HijackArgs entries
Comments suppressed due to low confidence (2)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/X86FrameHandler.cs:29

  • No tests were added to verify the new x86 HijackFrame handling path; consider adding unit or integration tests to cover this functionality.
        HijackArgsX86 args = _target.ProcessedData.GetOrAdd<HijackArgsX86>(frame.HijackArgsPtr);

docs/design/datacontracts/StackWalk.md:59

  • [nitpick] The entry 'HijackArgs (arm64/x86)' may be ambiguous; consider splitting into separate rows for arm64 and x86 or clarifying platform-specific details.
| `HijackArgs` (arm64/x86) | For each register `r` saved in HijackArgs, `r` | Register names associated with stored register values |

@max-charlamb max-charlamb merged commit aa6cca9 into dotnet:main Jun 20, 2025
145 of 151 checks passed
@max-charlamb max-charlamb deleted the x86-hijack branch June 20, 2025 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants