Skip to content
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

Add support for unwinding function fragments in the DAC #53765

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

davmason
Copy link
Member

@davmason davmason commented Jun 5, 2021

With lots of help from @ChrisAhna we discovered that the latest BBT will optimize runtime FCALLs in to multiple function fragments, which the unwinder does not currently support.

This change adds the ability to map from a function fragment back to the owning function.

@davmason davmason added this to the 6.0.0 milestone Jun 5, 2021
@davmason davmason self-assigned this Jun 5, 2021
@ghost
Copy link

ghost commented Jun 5, 2021

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

With lots of help from @ChrisAhna we discovered that the latest BBT will optimize runtime FCALLs in to multiple function fragments, which the unwinder does not currently support.

This change adds the ability to map from a function fragment back to the owning function.

Author: davmason
Assignees: davmason
Labels:

area-Diagnostics-coreclr

Milestone: 6.0.0

@jkotas
Copy link
Member

jkotas commented Jun 5, 2021

What was the manifestation of this bug? Is there issue opened on it?

@davmason
Copy link
Member Author

davmason commented Jun 5, 2021

It was found on desktop after a toolset update. As far as we know coreclr is not currently affected by it, but will be whenever we update to a new enough BBT to start doing function fragment optimizations.

The symptom is that debugger/DAC stackwalking fails on any FCALL frames that have been optimized with fragments.

Copy link
Member

@davidwrighton davidwrighton left a comment

Choose a reason for hiding this comment

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

Change looks good. I assume you've tested the logic in the problematic scenario.

@davmason
Copy link
Member Author

davmason commented Jun 8, 2021

@davidwrighton yes, I verified it fixes the failing desktop testcase.

@davmason davmason merged commit 8671908 into dotnet:main Jun 8, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 8, 2021
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.

3 participants