Skip to content

Commit

Permalink
Implement handling PreserveSig = false for interfaces (#1206)
Browse files Browse the repository at this point in the history 
* Implement handling PerserveSig = false for interfaces
This was done by pretending return value is out argument passed as last value.
We still need to return that value, so this require change in the Marshaller.cs so it would not throw assertion, since previously only Return values can return value.

* Ignore delegating marshalling thunks

* I add special case for handling blittable types which are return type for PreserveSig=false methods. In this case I fallback to default marshalling implementation.

* Add simple tests for PreservSig=false functionality

Co-authored-by: Jan Kotas <[email protected]>
  • Loading branch information
@kant2002 @jkotas
kant2002 and jkotas authored Jun 8, 2021
1 parent ac4bdf8 commit 2fe0c20
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/coreclr/tools/Common/TypeSystem/Ecma/MetadataExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ public static bool GetAttributeTypeNamespaceAndName(this MetadataReader metadata

public static PInvokeFlags GetDelegatePInvokeFlags(this EcmaType type)
{
PInvokeFlags flags = new PInvokeFlags();

PInvokeFlags flags = new PInvokeFlags(PInvokeAttributes.PreserveSig);
if (!type.IsDelegate)
{
return flags;
Expand Down
46 changes: 36 additions & 10 deletions src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,26 @@ private static Marshaller[] InitializeMarshallers(MethodDesc targetMethod, Inter
parameterMetadata = parameterMetadataArray[parameterIndex++];
}

TypeDesc parameterType = (i == 0)
? methodSig.ReturnType //first item is the return type
: methodSig[i - 1];
TypeDesc parameterType;
bool isHRSwappedRetVal = false;
if (i == 0)
{
// First item is the return type
parameterType = methodSig.ReturnType;
if (!flags.PreserveSig && !parameterType.IsVoid)
{
// PreserveSig = false can only show up an regular forward PInvokes
Debug.Assert(direction == MarshalDirection.Forward);

parameterType = methodSig.Context.GetByRefType(parameterType);
isHRSwappedRetVal = true;
}
}
else
{
parameterType = methodSig[i - 1];
}

marshallers[i] = Marshaller.CreateMarshaller(parameterType,
parameterIndex,
methodSig.GetEmbeddedSignatureData(),
Expand All @@ -108,8 +125,8 @@ private static Marshaller[] InitializeMarshallers(MethodDesc targetMethod, Inter
indexOffset + parameterMetadata.Index,
flags,
parameterMetadata.In,
parameterMetadata.Out,
parameterMetadata.Return
isHRSwappedRetVal ? true : parameterMetadata.Out,
isHRSwappedRetVal ? false : parameterMetadata.Return
);
}

Expand Down Expand Up @@ -231,16 +248,14 @@ private void EmitDelegateCall(DelegateMarshallingMethodThunk delegateMethod, PIn

private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams)
{
if (!_flags.PreserveSig && _targetMethod.Signature.ReturnType != _targetMethod.Context.GetWellKnownType(WellKnownType.Void))
throw new NotSupportedException();

ILEmitter emitter = ilCodeStreams.Emitter;
ILCodeStream fnptrLoadStream = ilCodeStreams.FunctionPointerLoadStream;
ILCodeStream callsiteSetupCodeStream = ilCodeStreams.CallsiteSetupCodeStream;
TypeSystemContext context = _targetMethod.Context;

bool isHRSwappedRetVal = !_flags.PreserveSig && !_targetMethod.Signature.ReturnType.IsVoid;
TypeDesc nativeReturnType = _flags.PreserveSig ? _marshallers[0].NativeParameterType : context.GetWellKnownType(WellKnownType.Int32);
TypeDesc[] nativeParameterTypes = new TypeDesc[_marshallers.Length - 1];
TypeDesc[] nativeParameterTypes = new TypeDesc[isHRSwappedRetVal ? _marshallers.Length : _marshallers.Length - 1];

// if the SetLastError flag is set in DllImport, clear the error code before doing P/Invoke
if (_flags.SetLastError)
Expand All @@ -254,6 +269,11 @@ private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams)
nativeParameterTypes[i - 1] = _marshallers[i].NativeParameterType;
}

if (isHRSwappedRetVal)
{
nativeParameterTypes[_marshallers.Length - 1] = _marshallers[0].NativeParameterType;
}

if (!_pInvokeILEmitterConfiguration.GenerateDirectCall(_targetMethod, out _))
{
MetadataType lazyHelperType = context.GetHelperType("InteropHelpers");
Expand Down Expand Up @@ -357,11 +377,17 @@ private MethodIL EmitIL()
cleanupCodestream.BeginHandler(tryFinally);

// Marshal the arguments
for (int i = 0; i < _marshallers.Length; i++)
bool isHRSwappedRetVal = !_flags.PreserveSig && !_targetMethod.Signature.ReturnType.IsVoid;
for (int i = isHRSwappedRetVal ? 1 : 0; i < _marshallers.Length; i++)
{
_marshallers[i].EmitMarshallingIL(pInvokeILCodeStreams);
}

if (isHRSwappedRetVal)
{
_marshallers[0].EmitMarshallingIL(pInvokeILCodeStreams);
}

// make the call
switch (_targetMethod)
{
Expand Down
17 changes: 16 additions & 1 deletion src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ internal virtual bool CleanupRequired
}
}

internal bool IsHRSwappedRetVal => Index == 0 && !Return;

public bool In;
public bool Out;
public bool Return;
Expand Down Expand Up @@ -512,7 +514,7 @@ protected virtual void EmitMarshalReturnValueManagedToNative()

public virtual void LoadReturnValue(ILCodeStream codeStream)
{
Debug.Assert(Return);
Debug.Assert(Return || IsHRSwappedRetVal);

switch (MarshalDirection)
{
Expand Down Expand Up @@ -653,6 +655,13 @@ protected void PropagateFromByRefArg(ILCodeStream stream, Home home)
/// </summary>
protected void PropagateToByRefArg(ILCodeStream stream, Home home)
{
// If by-ref arg has index == 0 then that argument is used for HR swapping and we just return that value.
if (IsHRSwappedRetVal)
{
// Returning result would be handled by LoadReturnValue
return;
}

stream.EmitLdArg(Index - 1);
home.LoadValue(stream);
stream.EmitStInd(ManagedType);
Expand Down Expand Up @@ -903,6 +912,12 @@ class BlittableValueMarshaller : Marshaller
{
protected override void EmitMarshalArgumentManagedToNative()
{
if (IsHRSwappedRetVal)
{
base.EmitMarshalArgumentManagedToNative();
return;
}

if (IsNativeByRef && MarshalDirection == MarshalDirection.Forward)
{
ILCodeStream marshallingCodeStream = _ilCodeStreams.MarshallingCodeStream;
Expand Down
6 changes: 6 additions & 0 deletions src/tests/nativeaot/SmokeTests/ComWrappers/ComWrappers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ public static void ThrowIfNotEquals(bool expected, bool actual, string message)
[DllImport("ComWrappersNative", CallingConvention = CallingConvention.StdCall)]
static extern int BuildComPointer(out IComInterface foo);

[DllImport("ComWrappersNative", CallingConvention = CallingConvention.StdCall, PreserveSig = false, EntryPoint="BuildComPointer")]
static extern IComInterface BuildComPointerNoPreserveSig();

public static void TestComInteropNullPointers()
{
Console.WriteLine("Testing Marshal APIs for COM interfaces");
Expand Down Expand Up @@ -160,6 +163,9 @@ public static void TestComInteropCCWCreation()
int result = BuildComPointer(out var comPointer);
ThrowIfNotEquals(0, result, "Seems to be COM marshalling behave strange.");
comPointer.DoWork(11);

comPointer = BuildComPointerNoPreserveSig();
comPointer.DoWork(22);
}

[MethodImpl(MethodImplOptions.NoInlining)]
Expand Down
27 changes: 27 additions & 0 deletions src/tests/nativeaot/SmokeTests/PInvoke/PInvoke.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,20 @@ internal struct Callbacks
[DllImport("PInvokeNative", CallingConvention = CallingConvention.StdCall, PreserveSig = false)]
static extern void ValidateSuccessCall(int errorCode);

[DllImport("PInvokeNative", CallingConvention = CallingConvention.StdCall, PreserveSig = false)]
static extern int ValidateIntResult(int errorCode);

[DllImport("PInvokeNative", EntryPoint = "ValidateIntResult", CallingConvention = CallingConvention.StdCall, PreserveSig = false)]
static extern MagicEnum ValidateEnumResult(int errorCode);

[DllImport("PInvokeNative", CallingConvention = CallingConvention.StdCall)]
internal static extern decimal DecimalTest(decimal value);

internal enum MagicEnum
{
MagicResult = 42,
}

public static int Main(string[] args)
{
TestBlittableType();
Expand Down Expand Up @@ -1035,6 +1046,22 @@ private static void TestWithoutPreserveSig()
catch (NotImplementedException)
{
}

var intResult = ValidateIntResult(0);
ThrowIfNotEquals(intResult, 42, "Int32 marshalling failed.");

try
{
const int E_NOTIMPL = -2147467263;
intResult = ValidateIntResult(E_NOTIMPL);
throw new Exception("Exception should be thrown for E_NOTIMPL error code");
}
catch (NotImplementedException)
{
}

var enumResult = ValidateEnumResult(0);
ThrowIfNotEquals(enumResult, MagicEnum.MagicResult, "Enum marshalling failed.");
}

public static unsafe void TestForwardDelegateWithUnmanagedCallersOnly()
Expand Down
6 changes: 6 additions & 0 deletions src/tests/nativeaot/SmokeTests/PInvoke/PInvokeNative.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,12 @@ DLL_EXPORT int __stdcall ValidateSuccessCall(int errorCode)
return errorCode;
}

DLL_EXPORT int __stdcall ValidateIntResult(int errorCode, int* result)
{
*result = 42;
return errorCode;
}

#ifndef DECIMAL_NEG // defined in wtypes.h
typedef struct tagDEC {
uint16_t wReserved;
Expand Down

0 comments on commit 2fe0c20

Please sign in to comment.