-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Fix debugger initialization #64523
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 debugger initialization #64523
Conversation
Tagging subscribers to this area: @tommcdon Issue DetailsFixes debugger initialization regression caused by #63157
|
src/coreclr/vm/domainassembly.h
Outdated
@@ -65,14 +65,16 @@ enum NotificationStatus | |||
|
|||
class DomainAssembly final | |||
{ | |||
VPTR_BASE_CONCRETE_VTABLE_CLASS(DomainAssembly) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a simple C++ type that nothing inherits from now. It does not need a vtable. This fix seems to be injecting useless vtable into the type.
What is the debugger code that is failing? Should the fix be made in that code instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My goal is to perform a quick fix to unblock the debugger. The debugger is failing in DacDbiInterfaceImpl::GetAssemblyFromDomainAssembly and crashes while handling the assembly load event for System.Private.CoreLib. I'll create an issue to track removing the v-table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the proper fix is to change VPTR
to DPTR
on these lines in vm:
typedef VPTR(class DomainAssembly) PTR_DomainAssembly;
typedef VPTR(PEAssembly) PTR_PEAssembly;
The vtables and DAC are super fragile, and we have multiple workarounds like
runtime/src/coreclr/vm/frames.h
Lines 354 to 360 in 6dcb039
//----------------------------------------------------------------------------- | |
// Frame depends on the location of its vtable within the object. This | |
// superclass ensures that the vtable for Frame objects is in the same | |
// location under both MSVC and GCC. | |
//----------------------------------------------------------------------------- | |
class FrameBase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume there is no way to have a simple test for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume there is no way to have a simple test for this?
CI testing is something we would like to accomplish in the future
test failures look unrelated to this PR, merging |
Fixes debugger initialization regression caused by #63157