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

Properly handle debugger-enumerating interior pointers and enregistered refs #92313

Merged
merged 4 commits into from
Sep 20, 2023

Conversation

leculver
Copy link
Contributor

@leculver leculver commented Sep 19, 2023

There is an ancient bug in the dac root walking code. If we hit an interior pointer on a callstack that lives outside of the GC heap, we will report that pointer as-is to ICorDebug (specifically CordbRefEnum::Next). This is despite the fact that ICorDebug specifically requests that the dac only enumerate pointers to real objects. This non-gc pointer will be treated as a real object pointer and will cause a failed HRESULT in CordbRefEnum. Since this call is wrapped with IfFailThrow it will halt all further processing of roots, leading to not fully capturing the object graph for memory analysis.

This code makes a small, targeted change to not enumerate non-GC pointers when the caller requests we specifically only enumerate pointers to real objects.

This is not a recent regression, but rather a bug that has likely existed since the original code was written.

FYI @asundheim.

There is an ancient bug in the dac root walking code.  If we hit an interior pointer on a callstack that lives outside of the GC heap, we will report that pointer as-is to ICorDebug (specifically `CordbRefEnum::Next`).  This is despite the fact that ICorDebug *specifically* requests that the dac only enumerate pointers to real objects.  This non-gc pointer will be treated as a real object pointer and will cause a failed HRESULT in CordbRefEnum.  Since this call is wrapped with `IfFailThrow` it will halt all further processing of roots.

This code makes a small, targeted change to not enumerate non-GC pointers when the caller requests we specifically only enumerate pointers to real objects.

This is not a recent regression, but rather a bug that has likely existed since the original code was written.
@ghost
Copy link

ghost commented Sep 19, 2023

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

Issue Details

There is an ancient bug in the dac root walking code. If we hit an interior pointer on a callstack that lives outside of the GC heap, we will report that pointer as-is to ICorDebug (specifically CordbRefEnum::Next). This is despite the fact that ICorDebug specifically requests that the dac only enumerate pointers to real objects. This non-gc pointer will be treated as a real object pointer and will cause a failed HRESULT in CordbRefEnum. Since this call is wrapped with IfFailThrow it will halt all further processing of roots.

This code makes a small, targeted change to not enumerate non-GC pointers when the caller requests we specifically only enumerate pointers to real objects.

This is not a recent regression, but rather a bug that has likely existed since the original code was written.

FYI @asundheim.

Author: leculver
Assignees: -
Labels:

area-Diagnostics-coreclr

Milestone: -

@ghost ghost assigned leculver Sep 19, 2023
@tommcdon
Copy link
Member

@hoyosjs

@leculver
Copy link
Contributor Author

Converting to draft while I investigate #87239 too.

@leculver leculver marked this pull request as ready for review September 20, 2023 17:30
@hoyosjs
Copy link
Member

hoyosjs commented Sep 20, 2023

/backport to release/8.0

@github-actions
Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6252309924

@hoyosjs hoyosjs changed the title ICorDebug: Properly handle interior pointers Properly handle enumerating interior pointers and enregistered refs Sep 20, 2023
@hoyosjs hoyosjs changed the title Properly handle enumerating interior pointers and enregistered refs Properly handle debugger enumerating interior pointers and enregistered refs Sep 20, 2023
@hoyosjs hoyosjs changed the title Properly handle debugger enumerating interior pointers and enregistered refs Properly handle debugger-enumerating interior pointers and enregistered refs Sep 20, 2023
@leculver leculver merged commit 9cbad65 into dotnet:main Sep 20, 2023
108 checks passed
@leculver leculver deleted the interiorPointerICorDebug branch September 20, 2023 22:05
@ghost ghost locked as resolved and limited conversation to collaborators Oct 21, 2023
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.

ICorDebugGCReferenceEnum.Next fails to return stack references in .NET 8 preview 4
4 participants