[cdac][.NET9] Data descriptor changes for GetMethodDescData #106417
Conversation
ghost
added
the
|
Tagging subscribers to this area: @mangod9 |
lambdageek
requested review from
AaronRobinsonMSFT,
davidwrighton,
elinor-fung and
jkotas
|
@jkotas @AaronRobinsonMSFT @davidwrighton @elinor-fung please help reviewing this PR. @lambdageek has requested that I wait for it before the RC1 snap if possible. |
AaronRobinsonMSFT
left a comment
AaronRobinsonMSFT
left a comment
There was a problem hiding this comment.
Do we need to update any markdown?
| bool m_fMarked; | ||
| int m_nGCCount; | ||
| bool m_IsCollectible; | ||
| BYTE m_IsCollectible; |
There was a problem hiding this comment.
| BYTE m_IsCollectible; | |
| const BYTE m_IsCollectible; |
There was a problem hiding this comment.
So it turns out we can't make m_IsCollectible const. The reason is because LoaderAllocator is a DAC-ized base class and the VPTR_BASE_VTABLE_CLASS macro adds a LoaderAllocator(TADDR addr, TADDR vtAddr) {} constructor which doesn't include an initializer for m_IsCollectible which results in a compilation error:
E:\dotnet-runtime\runtime-m\src\coreclr\vm\loaderallocator.hpp(291): error C2789: 'LoaderAllocator::m_IsCollectible'
: an object of const-qualified type must be initialized
E:\dotnet-runtime\runtime-m\src\coreclr\vm\loaderallocator.hpp(335): note: see declaration of 'LoaderAllocator::m_Is
Collectible'
Not for this portion - this is the data descriptors (and the new struct/global in runtime to better represent needed data), not tied to contracts yet. |
|
|
||
| PrecodeMachineDescriptor g_PrecodeMachineDescriptor; | ||
|
|
||
| void PrecodeMachineDescriptor::Init() |
There was a problem hiding this comment.
Why is there an Init routine as well as a set of non-static data member initializers here?
There was a problem hiding this comment.
I need to delete the initializers, that was an idea that didn't work out
There was a problem hiding this comment.
Are you doing this before you want this checked in? Given the snap deadline, I think we should probably miss it, and do a backport of the data descriptors when we're ready in reality to declare that .NET 9 might actually be useable with the cDAC and CLRMA. In general, I'm leery of stuff that has a static initializer, and that's what we have here now, and I don't see us fixing this before the snap happens.
There was a problem hiding this comment.
Yea, let's miss the snap. I can't fix and validate this in the next couple hours
|
@carlossanlop I'm going to close this and do a back port after the snap, it needs more work |
5 participants
Extracted from #106413
The main new thing here is a
PrecodeMachineDescriptorstruct and global: this is used to copy out theenumvalues and masks that show how to manipulate precode stubs to figure out what kind of a stub we're looking at.Contributes to #99302