Skip to content

Commit

Permalink
Stop guessing if a type is blittable (dotnet#102645)
Browse files Browse the repository at this point in the history 
Fixes dotnet#75666.

CoreLib was making an assumption that if a type didn't have pregenerated interop data and it didn't have GC pointers, it was probably blittable. This PR stops making that guess. We're generating an AOT warning whenever the compiler is not sure we have interop data to make an API call succeed. It should be fine to just say "I don't have interop data" instead of trying too hard to make the API call succeed.
  • Loading branch information
@MichalStrehovsky @Ruihan-Yin
MichalStrehovsky authored and Ruihan-Yin committed May 30, 2024
1 parent a676883 commit e014c7e
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 184 deletions.
160 changes: 62 additions & 98 deletions ...tiveaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/RuntimeInteropData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@


using System;
using System.Diagnostics;
using System.Runtime;
using System.Runtime.InteropServices;

using Internal.NativeFormat;
using Internal.Runtime.Augments;
Expand All @@ -16,39 +16,63 @@ internal static class RuntimeInteropData
{
public static uint GetStructFieldOffset(RuntimeTypeHandle structureTypeHandle, string fieldName)
{
if (TryGetStructFieldOffset(structureTypeHandle, fieldName, out bool structExists, out uint offset))
if (TryGetStructData(structureTypeHandle, out _, out NativeParser entryParser))
{
return offset;
}
uint mask = entryParser.GetUnsigned();
if ((mask & InteropDataConstants.HasInvalidLayout) != 0)
{
throw new ArgumentException(SR.Format(SR.Arg_CannotMarshal, Type.GetTypeFromHandle(structureTypeHandle)), nameof(structureTypeHandle));
}

Type structureType = Type.GetTypeFromHandle(structureTypeHandle)!;
if ((mask & InteropDataConstants.HasMarshallers) != 0)
{
// skip the first 4 IntPtrs(3 stubs and size)
entryParser.SkipInteger();
entryParser.SkipInteger();
entryParser.SkipInteger();
entryParser.SkipInteger();
}

// if we can find the struct but couldn't find its field, throw Argument Exception
if (structExists)
{
throw new ArgumentException(SR.Format(SR.Argument_OffsetOfFieldNotFound, structureType), nameof(fieldName));
uint fieldCount = mask >> InteropDataConstants.FieldCountShift;
for (uint index = 0; index < fieldCount; index++)
{
string name = entryParser.GetString();
uint offset = entryParser.GetUnsigned();
if (name == fieldName)
{
return offset;
}
}

throw new ArgumentException(SR.Format(SR.Argument_OffsetOfFieldNotFound, Type.GetTypeFromHandle(structureTypeHandle)), nameof(fieldName));
}

throw new NotSupportedException(SR.Format(SR.StructMarshalling_MissingInteropData, structureType));
throw new NotSupportedException(SR.Format(SR.StructMarshalling_MissingInteropData, Type.GetTypeFromHandle(structureTypeHandle)!));
}

public static unsafe int GetStructUnsafeStructSize(RuntimeTypeHandle structureTypeHandle)
{
if (TryGetStructUnsafeStructSize(structureTypeHandle, out int size))
MethodTable* structureMT = structureTypeHandle.ToMethodTable();
if (TryGetStructData(structureTypeHandle, out _, out NativeParser entryParser))
{
return size;
}
uint mask = entryParser.GetUnsigned();
if ((mask & InteropDataConstants.HasInvalidLayout) != 0)
{
throw new ArgumentException(SR.Format(SR.Arg_CannotMarshal, Type.GetTypeFromHandle(structureTypeHandle)), nameof(structureTypeHandle));
}

MethodTable* structureMT = structureTypeHandle.ToMethodTable();
if ((mask & InteropDataConstants.HasMarshallers) != 0)
{
return (int)entryParser.GetUnsigned();
}

// IsBlittable() checks whether the type contains GC references. It is approximate check with false positives.
// This fallback path will return incorrect answer for types that do not contain GC references, but that are
// not actually blittable; e.g. for types with bool fields.
if (structureTypeHandle.IsBlittable() && structureMT->IsValueType)
{
// We expect a blittable value type at this point
Debug.Assert(!structureMT->ContainsGCPointers);
Debug.Assert(structureMT->IsValueType);
return (int)structureMT->ValueTypeSize;
}

// No interop data.
// If the type is an interface or a generic type, the reason is likely that.
Type structureType = Type.GetTypeFromHandle(structureTypeHandle)!;
if (structureMT->IsInterface || structureMT->IsGeneric)
Expand All @@ -62,35 +86,22 @@ public static unsafe int GetStructUnsafeStructSize(RuntimeTypeHandle structureTy

public static IntPtr GetStructUnmarshalStub(RuntimeTypeHandle structureTypeHandle)
{
if (TryGetStructUnmarshalStub(structureTypeHandle, out IntPtr stub))
{
return stub;
}

throw new NotSupportedException(SR.Format(SR.StructMarshalling_MissingInteropData, Type.GetTypeFromHandle(structureTypeHandle)));
GetMarshallersForStruct(structureTypeHandle, out _, out IntPtr unmarshal, out _);
return unmarshal;
}

public static IntPtr GetStructMarshalStub(RuntimeTypeHandle structureTypeHandle)
{
if (TryGetStructMarshalStub(structureTypeHandle, out IntPtr stub))
{
return stub;
}

throw new NotSupportedException(SR.Format(SR.StructMarshalling_MissingInteropData, Type.GetTypeFromHandle(structureTypeHandle)));
GetMarshallersForStruct(structureTypeHandle, out IntPtr marshal, out _, out _);
return marshal;
}

public static IntPtr GetDestroyStructureStub(RuntimeTypeHandle structureTypeHandle, out bool hasInvalidLayout)
public static IntPtr GetDestroyStructureStub(RuntimeTypeHandle structureTypeHandle)
{
if (TryGetDestroyStructureStub(structureTypeHandle, out IntPtr stub, out hasInvalidLayout))
{
return stub;
}

throw new NotSupportedException(SR.Format(SR.StructMarshalling_MissingInteropData, Type.GetTypeFromHandle(structureTypeHandle)));
GetMarshallersForStruct(structureTypeHandle, out _, out _, out IntPtr destroyStub);
return destroyStub;
}


public static IntPtr GetForwardDelegateCreationStub(RuntimeTypeHandle delegateTypeHandle)
{
GetMarshallersForDelegate(delegateTypeHandle, out _, out _, out IntPtr delegateCreationStub);
Expand All @@ -108,53 +119,6 @@ public static IntPtr GetDelegateMarshallingStub(RuntimeTypeHandle delegateTypeHa
return pStub;
}

#region "Struct Data"
public static bool TryGetStructUnmarshalStub(RuntimeTypeHandle structureTypeHandle, out IntPtr unmarshalStub)
=> TryGetMarshallersForStruct(structureTypeHandle, out _, out unmarshalStub, out _, out _, out _);

public static bool TryGetStructMarshalStub(RuntimeTypeHandle structureTypeHandle, out IntPtr marshalStub)
=> TryGetMarshallersForStruct(structureTypeHandle, out marshalStub, out _, out _, out _, out _);

public static bool TryGetDestroyStructureStub(RuntimeTypeHandle structureTypeHandle, out IntPtr destroyStub, out bool hasInvalidLayout)
=> TryGetMarshallersForStruct(structureTypeHandle, out _, out _, out destroyStub, out hasInvalidLayout, out _);

public static bool TryGetStructUnsafeStructSize(RuntimeTypeHandle structureTypeHandle, out int size)
=> TryGetMarshallersForStruct(structureTypeHandle, out _, out _, out _, out _, out size);

public static bool TryGetStructFieldOffset(RuntimeTypeHandle structureTypeHandle, string fieldName, out bool structExists, out uint offset)
{
NativeParser entryParser;
structExists = false;
if (TryGetStructData(structureTypeHandle, out _, out entryParser))
{
structExists = true;

uint mask = entryParser.GetUnsigned();
if ((mask & InteropDataConstants.HasMarshallers) != 0)
{
// skip the first 4 IntPtrs(3 stubs and size)
entryParser.SkipInteger();
entryParser.SkipInteger();
entryParser.SkipInteger();
entryParser.SkipInteger();
}

uint fieldCount = mask >> InteropDataConstants.FieldCountShift;
for (uint index = 0; index < fieldCount; index++)
{
string name = entryParser.GetString();
offset = entryParser.GetUnsigned();
if (name == fieldName)
{
return true;
}
}
}
offset = 0;
return false;
}
#endregion

private static unsafe bool TryGetNativeReaderForBlob(TypeManagerHandle module, ReflectionMapBlob blob, out NativeReader reader)
{
byte* pBlob;
Expand Down Expand Up @@ -235,32 +199,32 @@ private static unsafe bool TryGetStructData(RuntimeTypeHandle structTypeHandle,
return false;
}

private static unsafe bool TryGetMarshallersForStruct(RuntimeTypeHandle structTypeHandle, out IntPtr marshalStub, out IntPtr unmarshalStub, out IntPtr destroyStub, out bool hasInvalidLayout, out int size)
private static unsafe void GetMarshallersForStruct(RuntimeTypeHandle structTypeHandle, out IntPtr marshalStub, out IntPtr unmarshalStub, out IntPtr destroyStub)
{
marshalStub = IntPtr.Zero;
unmarshalStub = IntPtr.Zero;
destroyStub = IntPtr.Zero;
hasInvalidLayout = true;
size = 0;

ExternalReferencesTable externalReferences;
NativeParser entryParser;
if (TryGetStructData(structTypeHandle, out externalReferences, out entryParser))
if (TryGetStructData(structTypeHandle, out ExternalReferencesTable externalReferences, out NativeParser entryParser))
{
uint mask = entryParser.GetUnsigned();
if ((mask & InteropDataConstants.HasMarshallers) != 0)
if ((mask & InteropDataConstants.HasInvalidLayout) != 0)
{
hasInvalidLayout = (mask & InteropDataConstants.HasInvalidLayout) != 0;
throw new ArgumentException(SR.Format(SR.Arg_CannotMarshal, Type.GetTypeFromHandle(structTypeHandle)));
}

size = (int)entryParser.GetUnsigned();
if ((mask & InteropDataConstants.HasMarshallers) != 0)
{
entryParser.GetUnsigned(); // skip size
marshalStub = externalReferences.GetIntPtrFromIndex(entryParser.GetUnsigned());
unmarshalStub = externalReferences.GetIntPtrFromIndex(entryParser.GetUnsigned());
destroyStub = externalReferences.GetIntPtrFromIndex(entryParser.GetUnsigned());

return true;
}
}
return false;
else
{
throw new NotSupportedException(SR.Format(SR.StructMarshalling_MissingInteropData, Type.GetTypeFromHandle(structTypeHandle)));
}
}
}
}
1 change: 0 additions & 1 deletion src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@
<Compile Include="System\Runtime\InteropServices\ComEventsHelper.NativeAot.cs" Condition="'$(FeatureCominterop)' == 'true'" />
<Compile Include="System\Runtime\InteropServices\ComWrappers.NativeAot.cs" Condition="'$(FeatureComWrappers)' == 'true'" />
<Compile Include="System\Runtime\InteropServices\GCHandle.NativeAot.cs" />
<Compile Include="System\Runtime\InteropServices\InteropExtensions.cs" />
<Compile Include="System\Runtime\InteropServices\NativeFunctionPointerWrapper.cs" />
<Compile Include="System\Runtime\InteropServices\NativeLibrary.NativeAot.cs" />
<Compile Include="System\Runtime\InteropServices\PInvokeMarshal.cs" />
Expand Down
17 changes: 0 additions & 17 deletions .../nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/InteropExtensions.cs
View file

This file was deleted.

38 changes: 3 additions & 35 deletions .../nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.NativeAot.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,7 @@ internal static unsafe void PtrToStructureImpl(IntPtr ptr, object structure)
MethodTable* structureMT = structure.GetMethodTable();
RuntimeTypeHandle structureTypeHandle = new RuntimeTypeHandle(structureMT);

IntPtr unmarshalStub;
if (structureTypeHandle.IsBlittable())
{
if (!RuntimeInteropData.TryGetStructUnmarshalStub(structureTypeHandle, out unmarshalStub))
{
unmarshalStub = IntPtr.Zero;
}
}
else
{
unmarshalStub = RuntimeInteropData.GetStructUnmarshalStub(structureTypeHandle);
}

IntPtr unmarshalStub = RuntimeInteropData.GetStructUnmarshalStub(structureTypeHandle);
if (unmarshalStub != IntPtr.Zero)
{
if (structureMT->IsValueType)
Expand Down Expand Up @@ -116,15 +104,7 @@ public static unsafe void DestroyStructure(IntPtr ptr, Type structuretype)
throw new ArgumentException(SR.Format(SR.Argument_MustHaveLayoutOrBeBlittable, structuretype));
}

if (structureTypeHandle.IsBlittable())
{
// ok to call with blittable structure, but no work to do in this case.
return;
}

IntPtr destroyStructureStub = RuntimeInteropData.GetDestroyStructureStub(structureTypeHandle, out bool hasInvalidLayout);
if (hasInvalidLayout)
throw new ArgumentException(SR.Format(SR.Argument_MustHaveLayoutOrBeBlittable, structuretype));
IntPtr destroyStructureStub = RuntimeInteropData.GetDestroyStructureStub(structureTypeHandle);
// DestroyStructureStub == IntPtr.Zero means its fields don't need to be destroyed
if (destroyStructureStub != IntPtr.Zero)
{
Expand All @@ -147,24 +127,12 @@ public static unsafe void StructureToPtr(object structure, IntPtr ptr, bool fDel
throw new ArgumentException(SR.Argument_NeedNonGenericObject, nameof(structure));
}

IntPtr marshalStub;
if (structureTypeHandle.IsBlittable())
{
if (!RuntimeInteropData.TryGetStructMarshalStub(structureTypeHandle, out marshalStub))
{
marshalStub = IntPtr.Zero;
}
}
else
{
marshalStub = RuntimeInteropData.GetStructMarshalStub(structureTypeHandle);
}

if (fDeleteOld)
{
DestroyStructure(ptr, structure.GetType());
}

IntPtr marshalStub = RuntimeInteropData.GetStructMarshalStub(structureTypeHandle);
if (marshalStub != IntPtr.Zero)
{
if (structureMT->IsValueType)
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/TypeSystem/Interop/IL/NativeStructType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public NativeStructType(ModuleDesc owningModule, MetadataType managedStructType,
Module = owningModule;
ManagedStructType = managedStructType;
_interopStateManager = interopStateManager;
_hasInvalidLayout = false;
_hasInvalidLayout = !managedStructType.HasLayout();
_typeForFieldIteration = managedStructType.IsInlineArray ? new TypeWithRepeatedFields(managedStructType) : managedStructType;

Stack<MetadataType> typesBeingLookedAt = (s_typesBeingLookedAt ??= new Stack<MetadataType>());
Expand Down
Loading

0 comments on commit e014c7e

Please sign in to comment.