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

[cdac] Add GetArrayData to Object contract, partially implement ISOSDacInterface::GetObjectData #105534

Merged
merged 9 commits into from
Jul 29, 2024

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Jul 26, 2024

  • Add Array data descriptor
  • Add GetArrayData to Object contract
    • Introduces a dependency on RuntimeTypeSystem contract
  • Make cDAC implement ISOSDacInterface::GetObjectData except for handling CCW/RCWs
    • Returns E_NOTIMPL when runtime supports COM
    • Validated in debugger that cDAC and DAC return the same values for some test single and multidimensional arrays
  • Pull out shared test helpers for adding different data to the mock target

Contributes to #99302

Ran SOS unit tests from the dotnet/diagnostics repo using a private build to replace the two latest runtimes (not the one that matches the sdk) used in the tests.

Copy link
Contributor

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

Comment on lines +14 to +38
private static readonly Target.TypeInfo MethodTableTypeInfo = new()
{
Fields = {
{ nameof(Data.MethodTable.MTFlags), new() { Offset = 4, Type = DataType.uint32}},
{ nameof(Data.MethodTable.BaseSize), new() { Offset = 8, Type = DataType.uint32}},
{ nameof(Data.MethodTable.MTFlags2), new() { Offset = 12, Type = DataType.uint32}},
{ nameof(Data.MethodTable.EEClassOrCanonMT), new () { Offset = 16, Type = DataType.nuint}},
{ nameof(Data.MethodTable.Module), new () { Offset = 24, Type = DataType.pointer}},
{ nameof(Data.MethodTable.ParentMethodTable), new () { Offset = 40, Type = DataType.pointer}},
{ nameof(Data.MethodTable.NumInterfaces), new () { Offset = 48, Type = DataType.uint16}},
{ nameof(Data.MethodTable.NumVirtuals), new () { Offset = 50, Type = DataType.uint16}},
{ nameof(Data.MethodTable.PerInstInfo), new () { Offset = 56, Type = DataType.pointer}},
}
};

private static readonly Target.TypeInfo EEClassTypeInfo = new Target.TypeInfo()
{
Fields = {
{ nameof (Data.EEClass.MethodTable), new () { Offset = 8, Type = DataType.pointer}},
{ nameof (Data.EEClass.CorTypeAttr), new () { Offset = 16, Type = DataType.uint32}},
{ nameof (Data.EEClass.NumMethods), new () { Offset = 20, Type = DataType.uint16}},
{ nameof (Data.EEClass.InternalCorElementType), new () { Offset = 22, Type = DataType.uint8}},
{ nameof (Data.EEClass.NumNonVirtualSlots), new () { Offset = 24, Type = DataType.uint16}},
}
};
Copy link
Member Author

Choose a reason for hiding this comment

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

These were copied out of MethodTableTests

},
};

public static class RuntimeTypeSystem
Copy link
Member Author

Choose a reason for hiding this comment

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

With the exception of AddGlobalPointers and AddArrayClass, these were copied out of MethodTableTests

Comment on lines +174 to +201
private static MockMemorySpace.Builder AddStringMethodTablePointer(TargetTestHelpers targetTestHelpers, MockMemorySpace.Builder builder)
{
MockMemorySpace.HeapFragment fragment = new() { Name = "Address of String Method Table", Address = TestStringMethodTableGlobalAddress, Data = new byte[targetTestHelpers.PointerSize] };
targetTestHelpers.WritePointer(fragment.Data, TestStringMethodTableAddress);
return builder.AddHeapFragments([
fragment,
new () { Name = "String Method Table", Address = TestStringMethodTableAddress, Data = new byte[targetTestHelpers.PointerSize] }
]);
}

internal static MockMemorySpace.Builder AddObject(TargetTestHelpers targetTestHelpers, MockMemorySpace.Builder builder, TargetPointer address, TargetPointer methodTable)
{
MockMemorySpace.HeapFragment fragment = new() { Name = $"Object : MT = '{methodTable}'", Address = address, Data = new byte[targetTestHelpers.SizeOfTypeInfo(ObjectTypeInfo)] };
Span<byte> dest = fragment.Data;
targetTestHelpers.WritePointer(dest.Slice(ObjectTypeInfo.Fields["m_pMethTab"].Offset), methodTable);
return builder.AddHeapFragment(fragment);
}

internal static MockMemorySpace.Builder AddStringObject(TargetTestHelpers targetTestHelpers, MockMemorySpace.Builder builder, TargetPointer address, string value)
{
int size = targetTestHelpers.SizeOfTypeInfo(ObjectTypeInfo) + targetTestHelpers.SizeOfTypeInfo(StringTypeInfo) + value.Length * sizeof(char);
MockMemorySpace.HeapFragment fragment = new() { Name = $"String = '{value}'", Address = address, Data = new byte[size] };
Span<byte> dest = fragment.Data;
targetTestHelpers.WritePointer(dest.Slice(ObjectTypeInfo.Fields["m_pMethTab"].Offset), TestStringMethodTableAddress);
targetTestHelpers.Write(dest.Slice(StringTypeInfo.Fields["m_StringLength"].Offset), (uint)value.Length);
MemoryMarshal.Cast<char, byte>(value).CopyTo(dest.Slice(StringTypeInfo.Fields["m_FirstChar"].Offset));
return builder.AddHeapFragment(fragment);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

These were copied out of ObjectTests

{
static constexpr size_t m_NumComponents = offsetof(ArrayBase, m_NumComponents);

static constexpr INT32* s_arrayBoundsZero = &ArrayBase::s_arrayBoundsZero;
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 put this in here to expose the static without just making it public, but it is a bit weird to be under cdac_offsets. Do we want to rename offsets to something more general about cdac info? Or we could add something like a separate template<typename T> struct cdac_globals.

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 we should rename it. in #104999 Jeremy is adding some cdac_offsets<Foo>::NestedStructSize constants because Foo has a private NestedStruct.

We could just name it cdac<T> or data_contract<T>

Copy link
Member Author

Choose a reason for hiding this comment

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

Will rename to cdac<T> in a follow-up.

// For an object pointer, this attempts to read the number of components.
// This expects that the first member after the MethodTable pointer (from Object)
// is a 32-bit integer representing the number of components.
// This holds for ArrayBase and StringObject - see coreclr/vm/object.h
Copy link
Member Author

Choose a reason for hiding this comment

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

Is there better / more explicit comments or doc that I could point to here?

@elinor-fung elinor-fung merged commit 868d756 into dotnet:main Jul 29, 2024
148 of 152 checks passed
@elinor-fung elinor-fung deleted the cdac-object-data branch July 29, 2024 18:27
@github-actions github-actions bot locked and limited conversation to collaborators Aug 30, 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.

2 participants