Skip to content

Restore original stack ref enumeration behavior #84034

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.

Sign up for GitHub

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
Mar 28, 2023
Merged

Restore original stack ref enumeration behavior #84034

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/coreclr/debug/daccess/dacdbiimpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7768,17 +7768,16 @@ HRESULT DacStackReferenceWalker::Next(ULONG count, DacGcReference stackRefs[], U
{
stackRefs[i].dwType = CorReferenceStack;
stackRefs[i].vmDomain.SetDacTargetPtr(domain);
stackRefs[i].i64ExtraData = 0;

const SOSStackRefData &sosStackRef = mList.Get(i);
if (sosStackRef.Flags & GC_CALL_INTERIOR)
{
stackRefs[i].i64ExtraData = GC_CALL_INTERIOR;
stackRefs[i].objHnd.SetDacTargetPtr(CLRDATA_ADDRESS_TO_TADDR(sosStackRef.Address));
stackRefs[i].pObject = CLRDATA_ADDRESS_TO_TADDR(sosStackRef.Object) | 1;
Copy link
Contributor

@mikem8361 mikem8361 Mar 28, 2023

Choose a reason for hiding this comment

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

So i64ExtraData no longer needs to be set to GC_CALL_INTERIOR in this branch?

Copy link
Contributor

@mikem8361 mikem8361 Mar 28, 2023

Choose a reason for hiding this comment

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

Or either branch?

Copy link
Contributor Author

@leculver leculver Mar 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The original behavior was that i64ExtraData was always set to 0, for all kinds of stack pointers. (It probably should be set, but it's not.) Setting it to 0 in both cases just makes sure we preserve the previous behavior that VS might have taken a dependency on.

I'm solving this a different way in the upcoming change to improve performance. (That PR will come later, after VS takes a look at it.)

Copy link
Contributor

@mikem8361 mikem8361 Mar 28, 2023

Choose a reason for hiding this comment

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

Sounds good.

}
else
{
stackRefs[i].i64ExtraData = 0;
stackRefs[i].pObject = CLRDATA_ADDRESS_TO_TADDR(sosStackRef.Object) | 1;
stackRefs[i].objHnd.SetDacTargetPtr(CLRDATA_ADDRESS_TO_TADDR(sosStackRef.Address));
}
}

Expand Down