Skip to content

Silence some marshalling warnings #690

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Feb 17, 2021
Merged

Silence some marshalling warnings #690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9f2a201
Silence some marshalling warnings
MichalStrehovsky Feb 15, 2021
3fff5f7
Prefer inlining to suppressions
MichalStrehovsky Feb 16, 2021
9e932d8
Update Marshal.cs
MichalStrehovsky Feb 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1847,7 +1847,11 @@ protected override void AllocAndTransformManagedToNative(ILCodeStream codeStream
codeStream.Emit(ILOpcode.brfalse, lNullPointer);

codeStream.Emit(ILOpcode.call, _ilCodeStreams.Emitter.NewToken(
#if READYTORUN
InteropTypes.GetMarshal(Context).GetKnownMethod("GetFunctionPointerForDelegate",
#else
InteropTypes.GetPInvokeMarshal(Context).GetKnownMethod("GetFunctionPointerForDelegate",
#endif
new MethodSignature(MethodSignatureFlags.Static, 0, Context.GetWellKnownType(WellKnownType.IntPtr),
new TypeDesc[] { Context.GetWellKnownType(WellKnownType.MulticastDelegate).BaseType }
))));
Expand All @@ -1872,6 +1876,7 @@ protected override void TransformNativeToManaged(ILCodeStream codeStream)
codeStream.Emit(ILOpcode.dup);
codeStream.Emit(ILOpcode.brfalse, lNullPointer);

#if READYTORUN
TypeDesc systemType = Context.SystemModule.GetKnownType("System", "Type");

codeStream.Emit(ILOpcode.ldtoken, _ilCodeStreams.Emitter.NewToken(ManagedType));
Expand All @@ -1882,6 +1887,15 @@ protected override void TransformNativeToManaged(ILCodeStream codeStream)
new MethodSignature(MethodSignatureFlags.Static, 0, Context.GetWellKnownType(WellKnownType.MulticastDelegate).BaseType,
new TypeDesc[] { Context.GetWellKnownType(WellKnownType.IntPtr), systemType }
))));
#else
codeStream.Emit(ILOpcode.ldtoken, _ilCodeStreams.Emitter.NewToken(ManagedType));

codeStream.Emit(ILOpcode.call, _ilCodeStreams.Emitter.NewToken(
InteropTypes.GetPInvokeMarshal(Context).GetKnownMethod("GetDelegateForFunctionPointer",
new MethodSignature(MethodSignatureFlags.Static, 0, Context.GetWellKnownType(WellKnownType.MulticastDelegate).BaseType,
new TypeDesc[] { Context.GetWellKnownType(WellKnownType.IntPtr), Context.GetWellKnownType(WellKnownType.RuntimeTypeHandle) }
))));
#endif

codeStream.Emit(ILOpcode.br, lDone);

Expand Down
65 changes: 59 additions & 6 deletions src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,16 @@ public static int SizeOf(Type t)
return SizeOfHelper(t, throwIfNotMarshalable: true);
}

[UnconditionalSuppressMessage("AotAnalysis", "IL9700:AotUnfriendlyApi",
Justification = "AOT compilers can see the T.")]
public static int SizeOf<T>() => SizeOf(typeof(T));
public static int SizeOf<T>()
{
Type t = typeof(T);
if (t.IsGenericType)
{
throw new ArgumentException(SR.Argument_NeedNonGenericType, nameof(T));
}

return SizeOfHelper(t, throwIfNotMarshalable: true);
}

/// <summary>
/// IMPORTANT NOTICE: This method does not do any verification on the array.
Expand Down Expand Up @@ -533,6 +540,8 @@ public static void PrelinkAll(Type c)
}
}

[UnconditionalSuppressMessage("AotAnalysis", "IL9700:AotUnfriendlyApi",
Justification = "AOT compilers can see the T.")]
public static void StructureToPtr<T>([DisallowNull] T structure, IntPtr ptr, bool fDeleteOld)
{
StructureToPtr((object)structure!, ptr, fDeleteOld);
Expand Down Expand Up @@ -579,11 +588,33 @@ public static void PtrToStructure(IntPtr ptr, object structure)

public static void PtrToStructure<T>(IntPtr ptr, [DisallowNull] T structure)
{
PtrToStructure(ptr, (object)structure!);
PtrToStructureHelper(ptr, structure, allowValueClasses: false);
}

public static T? PtrToStructure<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>(IntPtr ptr) => (T)PtrToStructure(ptr, typeof(T))!;
public static T? PtrToStructure<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>(IntPtr ptr)
{
if (ptr == IntPtr.Zero)
{
// Compat: this was originally implemented as a call to the non-generic version+cast.
// It would throw for non-nullable valuetypes here and return null for Nullable<T> even
// though it's generic.
if (default(T) != null)
throw new NullReferenceException();

return default;
}

Type structureType = typeof(T);
if (structureType.IsGenericType)
{
throw new ArgumentException(SR.Argument_NeedNonGenericType, nameof(T));
}

return (T)PtrToStructureHelper(ptr, structureType);
}

[UnconditionalSuppressMessage("AotAnalysis", "IL9700:AotUnfriendlyApi",
Justification = "AOT compilers can see the T.")]
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering whether it would be better to call PtrToStructureHelper instead of suppressing the warning. Some of the argument checks would need to be duplicated, but we would be also able to omit some.

Copy link
Member

Choose a reason for hiding this comment

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

(similar to other places like GetDelegateForFunctionPointer)

public static void DestroyStructure<T>(IntPtr ptr) => DestroyStructure(ptr, typeof(T));

/// <summary>
Expand Down Expand Up @@ -891,7 +922,27 @@ public static Delegate GetDelegateForFunctionPointer(IntPtr ptr, Type t)

public static TDelegate GetDelegateForFunctionPointer<TDelegate>(IntPtr ptr)
{
return (TDelegate)(object)GetDelegateForFunctionPointer(ptr, typeof(TDelegate));
if (ptr == IntPtr.Zero)
{
throw new ArgumentNullException(nameof(ptr));
}

Type t = typeof(TDelegate);
if (t.IsGenericType)
{
throw new ArgumentException(SR.Argument_NeedNonGenericType, nameof(TDelegate));
}

// For backward compatibility, we allow lookup of existing delegate to
// function pointer mappings using abstract MulticastDelegate type. We will check
// for the non-abstract delegate type later if no existing mapping is found.
Type? c = t.BaseType;
if (c != typeof(Delegate) && c != typeof(MulticastDelegate))
{
throw new ArgumentException(SR.Arg_MustBeDelegate, nameof(TDelegate));
}

return (TDelegate)(object)GetDelegateForFunctionPointerInternal(ptr, t);
}

[RequiresDynamicCode("Marshalling code for the delegate might not be available. Use the GetFunctionPointerForDelegate<TDelegate> overload instead.")]
Copy link
Member

Choose a reason for hiding this comment

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

Do we also need to update public static IntPtr GetFunctionPointerForDelegate<TDelegate>(TDelegate d) where TDelegate : notnull to not produce warning?

Copy link
Member

Choose a reason for hiding this comment

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

Nit: I just tried this and noticed that there is a double-dot printed in the warning message Use the GetFunctionPointerForDelegate<TDelegate> overload instead... Should we stop appending . to the message in ReflecitonMethodBodyScanner.cs?

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 think we'll never be consistent with this. It might be better to just fix this up when the message is generated (append the dot only if it's not there).

Expand All @@ -905,6 +956,8 @@ public static IntPtr GetFunctionPointerForDelegate(Delegate d)
return GetFunctionPointerForDelegateInternal(d);
}

[UnconditionalSuppressMessage("AotAnalysis", "IL9700:AotUnfriendlyApi",
Justification = "AOT compilers can see the T.")]
public static IntPtr GetFunctionPointerForDelegate<TDelegate>(TDelegate d) where TDelegate : notnull
{
return GetFunctionPointerForDelegate((Delegate)(object)d);
Expand Down