Skip to content

Zero initialize FrameInfo so that we don't mistake an InlineCallFrame for a CLRToCOMMethodFrame #117252

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 1 commit into from
Jul 3, 2025

Conversation

steveisok
Copy link
Member

This one was a doozy.

If you try to step in to a static function and the static cctor hasn't run, the debugger will treat it as a go. Moving the static helpers to managed altered the debugger flow such that you would have managed -> native interop -> managed (implied .cctor) frames. The debugger would detect the last managed one is an implied .cctor and step out. When it stepped out to the next managed frame, it would then incorrectly calculate the offset and issue a breakpoint the instruction before the qcall into native code. Thus, the code already ran and it's effectively a go.

After a bunch of logging and discussion, I at first thought the stackwalker was at fault where it somehow ignored the unmanaged frames. I then noticed we have a block that intentionally suppresses native frames when stepping out similar to what is now happening.

After even more logging, I noticed that pInfo->fIgnoreThisFrameIfSuppressingUMChainFromCLRToCOMMethodFrameGeneric was true even though nothing ran to set it that way. This lead to the fix where we zero initialize a FrameInfo instance to make sure that bool is false.

Note: I would not be surprised if there are other interop stepping issues lurking. Additionally, there are other places where we don't zero initialize various structs on the stack. As a follow up, we should do that in order to avoid this kind of madness.

Fixes #114820

@Copilot Copilot AI review requested due to automatic review settings July 2, 2025 21:54
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 2, 2025
@steveisok steveisok requested review from hoyosjs and noahfalk July 2, 2025 21:55
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 a FrameInfo struct is zero-initialized in TrackUMChain to avoid uninitialized boolean flags causing incorrect debugger stepping behavior when static constructors haven’t run.

  • Value-initialize FrameInfo f so fIgnoreThisFrameIfSuppressingUMChainFromCLRToCOMMethodFrameGeneric is false by default.
  • Prevents the debugger from mistaking an InlineCallFrame for a CLRToCOMMethodFrame and stepping out incorrectly.
Comments suppressed due to low confidence (1)

src/coreclr/debug/ee/frameinfo.cpp:1182

  • Add a debugger stepping test to verify that FrameInfo is correctly zero-initialized and that InlineCallFrames are not mistaken for CLRToCOMMethodFrame after this change.
        FrameInfo f = {};

@steveisok steveisok requested a review from tommcdon July 2, 2025 21:55
Copy link
Member

@tommcdon tommcdon left a comment

Choose a reason for hiding this comment

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

LGTM!

@steveisok steveisok enabled auto-merge (squash) July 3, 2025 02:13
@steveisok
Copy link
Member Author

/ba-g Infra failure is too generic on win-x86 legs

@steveisok steveisok disabled auto-merge July 3, 2025 02:16
@steveisok steveisok enabled auto-merge (squash) July 3, 2025 02:16
@am11 am11 added area-Diagnostics-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 3, 2025
Copy link
Contributor

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

@steveisok steveisok merged commit d4ae57c into dotnet:main Jul 3, 2025
97 of 100 checks passed
@steveisok steveisok deleted the fix-bogus-frameinfo-init branch July 3, 2025 11:12
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.

Issues stepping through System.Runtime.CompilerServices.StaticsHelpers.GetGCStaticBase
5 participants