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

Fix a couple .NET 9 testing issues in the SOS tests in the diagnostics repo #99255

Merged
merged 2 commits into from
Mar 4, 2024

Conversation

mikem8361
Copy link
Member

Fix SOS DAC GetAssemblyData failure from ClassLoader not being in core dump

Bump the MacOS stack overflow stack size

@ghost
Copy link

ghost commented Mar 4, 2024

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

Issue Details

Fix SOS DAC GetAssemblyData failure from ClassLoader not being in core dump

Bump the MacOS stack overflow stack size

Author: mikem8361
Assignees: mikem8361
Labels:

area-Diagnostics-coreclr

Milestone: -

@@ -849,7 +849,7 @@ HijackFaultingThread(
if (fIsStackOverflow)
{
// Allocate the minimal stack necessary for handling stack overflow
int stackOverflowStackSize = 7 * 4096;
int stackOverflowStackSize = 15 * 4096;
Copy link
Member

Choose a reason for hiding this comment

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

@janvorli This looks like part of the fix for #98477 that you have been working on.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, @mikem8361 has hit the issue, so I've told him he could make this part of the change if he wants to. I'll fix the stackoverflow detection in the presence of the macos issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to get the couple of changes in so I can get my diagnostics repo .NET testing in soon.

@@ -2577,7 +2577,7 @@ ClrDataAccess::GetAssemblyData(CLRDATA_ADDRESS cdBaseDomainPtr, CLRDATA_ADDRESS
}

assemblyData->AssemblyPtr = HOST_CDADDR(pAssembly);
assemblyData->ClassLoader = HOST_CDADDR(pAssembly->GetLoader());
assemblyData->ClassLoader = 0;
Copy link
Member

Choose a reason for hiding this comment

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

This change LGTM. Are you going to make a matching change in the SOS to delete ClassLoader from the DumpAssembly output?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought I would leave it in SOS for now. It will still print something valid for older runtimes and 0 for .NET 9. But if you insist, I'll remove it.

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be an improvement to delete it from SOS output. The information that it prints for older runtimes is not very useful.

@mikem8361 mikem8361 merged commit eb1fbe5 into dotnet:main Mar 4, 2024
111 checks passed
@mikem8361 mikem8361 deleted the dacfix branch March 4, 2024 22:44
@github-actions github-actions bot locked and limited conversation to collaborators Apr 4, 2024
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