From ba68a4b11c7a20cb720e038bd8ea577f0fae3762 Mon Sep 17 00:00:00 2001 From: Andrii Kurdiumov Date: Mon, 7 Jun 2021 11:54:37 +0600 Subject: [PATCH 1/9] 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. --- .../TypeSystem/IL/Stubs/PInvokeILEmitter.cs | 31 ++++++++++++++----- .../TypeSystem/Interop/IL/Marshaller.cs | 17 +++++++--- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs index aa67eaf60d89..166c74c9d4d2 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs @@ -97,7 +97,7 @@ private static Marshaller[] InitializeMarshallers(MethodDesc targetMethod, Inter TypeDesc parameterType = (i == 0) ? methodSig.ReturnType //first item is the return type : methodSig[i - 1]; - marshallers[i] = Marshaller.CreateMarshaller(parameterType, + marshallers[i] = Marshaller.CreateMarshaller((i == 0 && !flags.PreserveSig && !parameterType.IsVoid) ? methodSig.Context.GetByRefType(parameterType): parameterType, parameterIndex, methodSig.GetEmbeddedSignatureData(), MarshallerType.Argument, @@ -108,8 +108,8 @@ private static Marshaller[] InitializeMarshallers(MethodDesc targetMethod, Inter indexOffset + parameterMetadata.Index, flags, parameterMetadata.In, - parameterMetadata.Out, - parameterMetadata.Return + (i == 0 && !flags.PreserveSig && !parameterType.IsVoid) ? true : parameterMetadata.Out, + (i == 0 && !flags.PreserveSig && !parameterType.IsVoid) ? false : parameterMetadata.Return ); } @@ -231,16 +231,22 @@ 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(); + if (!_flags.PreserveSig) + { + TypeDesc managedReturnType = _targetMethod.Signature.ReturnType; + bool supportedType = managedReturnType.IsPrimitive || managedReturnType.IsInterface || managedReturnType.IsObject; + if (!supportedType) + throw new NotSupportedException(); + } ILEmitter emitter = ilCodeStreams.Emitter; ILCodeStream fnptrLoadStream = ilCodeStreams.FunctionPointerLoadStream; ILCodeStream callsiteSetupCodeStream = ilCodeStreams.CallsiteSetupCodeStream; TypeSystemContext context = _targetMethod.Context; + bool isHRSwapping = !_flags.PreserveSig && _targetMethod.Signature.ReturnType != _targetMethod.Context.GetWellKnownType(WellKnownType.Void); TypeDesc nativeReturnType = _flags.PreserveSig ? _marshallers[0].NativeParameterType : context.GetWellKnownType(WellKnownType.Int32); - TypeDesc[] nativeParameterTypes = new TypeDesc[_marshallers.Length - 1]; + TypeDesc[] nativeParameterTypes = new TypeDesc[isHRSwapping ? _marshallers.Length : _marshallers.Length - 1]; // if the SetLastError flag is set in DllImport, clear the error code before doing P/Invoke if (_flags.SetLastError) @@ -254,6 +260,11 @@ private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams) nativeParameterTypes[i - 1] = _marshallers[i].NativeParameterType; } + if (isHRSwapping) + { + nativeParameterTypes[_marshallers.Length - 1] = _marshallers[0].NativeParameterType; + } + if (!_pInvokeILEmitterConfiguration.GenerateDirectCall(_targetMethod, out _)) { MetadataType lazyHelperType = context.GetHelperType("InteropHelpers"); @@ -357,11 +368,17 @@ private MethodIL EmitIL() cleanupCodestream.BeginHandler(tryFinally); // Marshal the arguments - for (int i = 0; i < _marshallers.Length; i++) + bool isHRSwapping = !_flags.PreserveSig && _targetMethod.Signature.ReturnType != _targetMethod.Context.GetWellKnownType(WellKnownType.Void); + for (int i = isHRSwapping ? 1 : 0; i < _marshallers.Length; i++) { _marshallers[i].EmitMarshallingIL(pInvokeILCodeStreams); } + if (isHRSwapping) + { + _marshallers[0].EmitMarshallingIL(pInvokeILCodeStreams); + } + // make the call switch (_targetMethod) { diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs index 46c53157b465..612a63099b71 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs @@ -512,7 +512,7 @@ protected virtual void EmitMarshalReturnValueManagedToNative() public virtual void LoadReturnValue(ILCodeStream codeStream) { - Debug.Assert(Return); + Debug.Assert(Index == 0); switch (MarshalDirection) { @@ -653,9 +653,18 @@ protected void PropagateFromByRefArg(ILCodeStream stream, Home home) /// protected void PropagateToByRefArg(ILCodeStream stream, Home home) { - stream.EmitLdArg(Index - 1); - home.LoadValue(stream); - stream.EmitStInd(ManagedType); + // If by-ref arg has index == 0 then that argument is used for HR swapping and we just return that value. + if (Index == 0) + { + // Returning result would be handled by LoadReturnValue + return; + } + else + { + stream.EmitLdArg(Index - 1); + home.LoadValue(stream); + stream.EmitStInd(ManagedType); + } } protected virtual void EmitMarshalArgumentManagedToNative() From f61421213bd350bf05219d37adeeb4bce2329be4 Mon Sep 17 00:00:00 2001 From: Andrii Kurdiumov Date: Mon, 7 Jun 2021 16:04:28 +0600 Subject: [PATCH 2/9] Ignore delegating marshalling thunks --- .../Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs | 14 +++++++++----- .../Common/TypeSystem/Interop/IL/Marshaller.cs | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs index 166c74c9d4d2..78187d663d2f 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs @@ -97,7 +97,9 @@ private static Marshaller[] InitializeMarshallers(MethodDesc targetMethod, Inter TypeDesc parameterType = (i == 0) ? methodSig.ReturnType //first item is the return type : methodSig[i - 1]; - marshallers[i] = Marshaller.CreateMarshaller((i == 0 && !flags.PreserveSig && !parameterType.IsVoid) ? methodSig.Context.GetByRefType(parameterType): parameterType, + bool isHRSwapping = !flags.PreserveSig && !parameterType.IsVoid + && targetMethod is not DelegateMarshallingMethodThunk; + marshallers[i] = Marshaller.CreateMarshaller((i == 0 && isHRSwapping) ? methodSig.Context.GetByRefType(parameterType): parameterType, parameterIndex, methodSig.GetEmbeddedSignatureData(), MarshallerType.Argument, @@ -108,8 +110,8 @@ private static Marshaller[] InitializeMarshallers(MethodDesc targetMethod, Inter indexOffset + parameterMetadata.Index, flags, parameterMetadata.In, - (i == 0 && !flags.PreserveSig && !parameterType.IsVoid) ? true : parameterMetadata.Out, - (i == 0 && !flags.PreserveSig && !parameterType.IsVoid) ? false : parameterMetadata.Return + (i == 0 && isHRSwapping) ? true : parameterMetadata.Out, + (i == 0 && isHRSwapping) ? false : parameterMetadata.Return ); } @@ -244,7 +246,8 @@ private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams) ILCodeStream callsiteSetupCodeStream = ilCodeStreams.CallsiteSetupCodeStream; TypeSystemContext context = _targetMethod.Context; - bool isHRSwapping = !_flags.PreserveSig && _targetMethod.Signature.ReturnType != _targetMethod.Context.GetWellKnownType(WellKnownType.Void); + bool isHRSwapping = !_flags.PreserveSig && _targetMethod.Signature.ReturnType != _targetMethod.Context.GetWellKnownType(WellKnownType.Void) + && _targetMethod is not DelegateMarshallingMethodThunk; TypeDesc nativeReturnType = _flags.PreserveSig ? _marshallers[0].NativeParameterType : context.GetWellKnownType(WellKnownType.Int32); TypeDesc[] nativeParameterTypes = new TypeDesc[isHRSwapping ? _marshallers.Length : _marshallers.Length - 1]; @@ -368,7 +371,8 @@ private MethodIL EmitIL() cleanupCodestream.BeginHandler(tryFinally); // Marshal the arguments - bool isHRSwapping = !_flags.PreserveSig && _targetMethod.Signature.ReturnType != _targetMethod.Context.GetWellKnownType(WellKnownType.Void); + bool isHRSwapping = !_flags.PreserveSig && _targetMethod.Signature.ReturnType != _targetMethod.Context.GetWellKnownType(WellKnownType.Void) + && _targetMethod is not DelegateMarshallingMethodThunk; for (int i = isHRSwapping ? 1 : 0; i < _marshallers.Length; i++) { _marshallers[i].EmitMarshallingIL(pInvokeILCodeStreams); diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs index 612a63099b71..0f285722f690 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs @@ -512,7 +512,7 @@ protected virtual void EmitMarshalReturnValueManagedToNative() public virtual void LoadReturnValue(ILCodeStream codeStream) { - Debug.Assert(Index == 0); + Debug.Assert(Return || Index == 0); switch (MarshalDirection) { From 15a5c68d0b9e233948d53ed4772ec7cfa5b681c5 Mon Sep 17 00:00:00 2001 From: Andrii Kurdiumov Date: Mon, 7 Jun 2021 17:23:33 +0600 Subject: [PATCH 3/9] 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. --- .../tools/Common/TypeSystem/Interop/IL/Marshaller.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs index 0f285722f690..ee3c4a3aea10 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs @@ -912,7 +912,7 @@ class BlittableValueMarshaller : Marshaller { protected override void EmitMarshalArgumentManagedToNative() { - if (IsNativeByRef && MarshalDirection == MarshalDirection.Forward) + if (IsNativeByRef && MarshalDirection == MarshalDirection.Forward && Index > 0) { ILCodeStream marshallingCodeStream = _ilCodeStreams.MarshallingCodeStream; ILEmitter emitter = _ilCodeStreams.Emitter; @@ -926,10 +926,14 @@ protected override void EmitMarshalArgumentManagedToNative() marshallingCodeStream.EmitStLoc(native); _ilCodeStreams.CallsiteSetupCodeStream.EmitLdLoc(native); } - else + else if (Index > 0) { _ilCodeStreams.CallsiteSetupCodeStream.EmitLdArg(Index - 1); } + else + { + base.EmitMarshalArgumentManagedToNative(); + } } protected override void EmitMarshalArgumentNativeToManaged() From 48de7aa4faed7cf037b7f03ac6ed6b89833dc16a Mon Sep 17 00:00:00 2001 From: Andrii Kurdiumov Date: Mon, 7 Jun 2021 23:41:28 +0600 Subject: [PATCH 4/9] Remove artificial constraing on PreserveSig=false --- .../tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs index 78187d663d2f..0fee490f9c74 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs @@ -233,14 +233,6 @@ private void EmitDelegateCall(DelegateMarshallingMethodThunk delegateMethod, PIn private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams) { - if (!_flags.PreserveSig) - { - TypeDesc managedReturnType = _targetMethod.Signature.ReturnType; - bool supportedType = managedReturnType.IsPrimitive || managedReturnType.IsInterface || managedReturnType.IsObject; - if (!supportedType) - throw new NotSupportedException(); - } - ILEmitter emitter = ilCodeStreams.Emitter; ILCodeStream fnptrLoadStream = ilCodeStreams.FunctionPointerLoadStream; ILCodeStream callsiteSetupCodeStream = ilCodeStreams.CallsiteSetupCodeStream; From 0b5bb1c40b6de174549d0cacaa6fd1d9150cf41e Mon Sep 17 00:00:00 2001 From: Andrii Kurdiumov Date: Tue, 8 Jun 2021 00:55:39 +0600 Subject: [PATCH 5/9] Address PR feedback --- .../TypeSystem/Ecma/MetadataExtensions.cs | 4 +- .../TypeSystem/IL/Stubs/PInvokeILEmitter.cs | 45 ++++++++++++------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Ecma/MetadataExtensions.cs b/src/coreclr/tools/Common/TypeSystem/Ecma/MetadataExtensions.cs index 4cbc5f3b2a19..35650f9044ca 100644 --- a/src/coreclr/tools/Common/TypeSystem/Ecma/MetadataExtensions.cs +++ b/src/coreclr/tools/Common/TypeSystem/Ecma/MetadataExtensions.cs @@ -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; diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs index 0fee490f9c74..e1387be6b77f 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs @@ -94,12 +94,27 @@ 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]; - bool isHRSwapping = !flags.PreserveSig && !parameterType.IsVoid - && targetMethod is not DelegateMarshallingMethodThunk; - marshallers[i] = Marshaller.CreateMarshaller((i == 0 && isHRSwapping) ? methodSig.Context.GetByRefType(parameterType): parameterType, + 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(), MarshallerType.Argument, @@ -110,8 +125,8 @@ private static Marshaller[] InitializeMarshallers(MethodDesc targetMethod, Inter indexOffset + parameterMetadata.Index, flags, parameterMetadata.In, - (i == 0 && isHRSwapping) ? true : parameterMetadata.Out, - (i == 0 && isHRSwapping) ? false : parameterMetadata.Return + isHRSwappedRetVal ? true : parameterMetadata.Out, + isHRSwappedRetVal ? false : parameterMetadata.Return ); } @@ -238,10 +253,9 @@ private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams) ILCodeStream callsiteSetupCodeStream = ilCodeStreams.CallsiteSetupCodeStream; TypeSystemContext context = _targetMethod.Context; - bool isHRSwapping = !_flags.PreserveSig && _targetMethod.Signature.ReturnType != _targetMethod.Context.GetWellKnownType(WellKnownType.Void) - && _targetMethod is not DelegateMarshallingMethodThunk; + bool isHRSwappedRetVal = !_flags.PreserveSig && _targetMethod.Signature.ReturnType.IsVoid; TypeDesc nativeReturnType = _flags.PreserveSig ? _marshallers[0].NativeParameterType : context.GetWellKnownType(WellKnownType.Int32); - TypeDesc[] nativeParameterTypes = new TypeDesc[isHRSwapping ? _marshallers.Length : _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) @@ -255,7 +269,7 @@ private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams) nativeParameterTypes[i - 1] = _marshallers[i].NativeParameterType; } - if (isHRSwapping) + if (isHRSwappedRetVal) { nativeParameterTypes[_marshallers.Length - 1] = _marshallers[0].NativeParameterType; } @@ -363,14 +377,13 @@ private MethodIL EmitIL() cleanupCodestream.BeginHandler(tryFinally); // Marshal the arguments - bool isHRSwapping = !_flags.PreserveSig && _targetMethod.Signature.ReturnType != _targetMethod.Context.GetWellKnownType(WellKnownType.Void) - && _targetMethod is not DelegateMarshallingMethodThunk; - for (int i = isHRSwapping ? 1 : 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 (isHRSwapping) + if (isHRSwappedRetVal) { _marshallers[0].EmitMarshallingIL(pInvokeILCodeStreams); } From dbdb2395b073b4353d168d386742b18bf6d155e9 Mon Sep 17 00:00:00 2001 From: Andrii Kurdiumov Date: Tue, 8 Jun 2021 09:30:46 +0600 Subject: [PATCH 6/9] Fix --- .../tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs index e1387be6b77f..d77bb5994ec0 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs @@ -253,7 +253,7 @@ private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams) ILCodeStream callsiteSetupCodeStream = ilCodeStreams.CallsiteSetupCodeStream; TypeSystemContext context = _targetMethod.Context; - bool isHRSwappedRetVal = !_flags.PreserveSig && _targetMethod.Signature.ReturnType.IsVoid; + bool isHRSwappedRetVal = !_flags.PreserveSig && !_targetMethod.Signature.ReturnType.IsVoid; TypeDesc nativeReturnType = _flags.PreserveSig ? _marshallers[0].NativeParameterType : context.GetWellKnownType(WellKnownType.Int32); TypeDesc[] nativeParameterTypes = new TypeDesc[isHRSwappedRetVal ? _marshallers.Length : _marshallers.Length - 1]; @@ -377,7 +377,7 @@ private MethodIL EmitIL() cleanupCodestream.BeginHandler(tryFinally); // Marshal the arguments - bool isHRSwappedRetVal = !_flags.PreserveSig && _targetMethod.Signature.ReturnType.IsVoid; + bool isHRSwappedRetVal = !_flags.PreserveSig && !_targetMethod.Signature.ReturnType.IsVoid; for (int i = isHRSwappedRetVal ? 1 : 0; i < _marshallers.Length; i++) { _marshallers[i].EmitMarshallingIL(pInvokeILCodeStreams); From 5903921f150d6b03059519c510da300b73b22de9 Mon Sep 17 00:00:00 2001 From: Andrii Kurdiumov Date: Tue, 8 Jun 2021 10:19:44 +0600 Subject: [PATCH 7/9] Add simple tests for PreservSig=false functionality --- .../SmokeTests/ComWrappers/ComWrappers.cs | 6 +++++ .../nativeaot/SmokeTests/PInvoke/PInvoke.cs | 27 +++++++++++++++++++ .../SmokeTests/PInvoke/PInvokeNative.cpp | 6 +++++ 3 files changed, 39 insertions(+) diff --git a/src/tests/nativeaot/SmokeTests/ComWrappers/ComWrappers.cs b/src/tests/nativeaot/SmokeTests/ComWrappers/ComWrappers.cs index 5fa111cba18a..df2c9942cef2 100644 --- a/src/tests/nativeaot/SmokeTests/ComWrappers/ComWrappers.cs +++ b/src/tests/nativeaot/SmokeTests/ComWrappers/ComWrappers.cs @@ -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"); @@ -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)] diff --git a/src/tests/nativeaot/SmokeTests/PInvoke/PInvoke.cs b/src/tests/nativeaot/SmokeTests/PInvoke/PInvoke.cs index f1698766d895..4e5375389455 100644 --- a/src/tests/nativeaot/SmokeTests/PInvoke/PInvoke.cs +++ b/src/tests/nativeaot/SmokeTests/PInvoke/PInvoke.cs @@ -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(); @@ -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() diff --git a/src/tests/nativeaot/SmokeTests/PInvoke/PInvokeNative.cpp b/src/tests/nativeaot/SmokeTests/PInvoke/PInvokeNative.cpp index 9cc98b3f72f6..8e3c15108c10 100644 --- a/src/tests/nativeaot/SmokeTests/PInvoke/PInvokeNative.cpp +++ b/src/tests/nativeaot/SmokeTests/PInvoke/PInvokeNative.cpp @@ -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; From df9bdb10ba4b447d779889f971c5c124c34f0591 Mon Sep 17 00:00:00 2001 From: Andrii Kurdiumov Date: Tue, 8 Jun 2021 12:22:46 +0600 Subject: [PATCH 8/9] Update src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs Co-authored-by: Jan Kotas --- .../tools/Common/TypeSystem/Interop/IL/Marshaller.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs index ee3c4a3aea10..3d90c4f82ba8 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs @@ -659,12 +659,10 @@ protected void PropagateToByRefArg(ILCodeStream stream, Home home) // Returning result would be handled by LoadReturnValue return; } - else - { - stream.EmitLdArg(Index - 1); - home.LoadValue(stream); - stream.EmitStInd(ManagedType); - } + + stream.EmitLdArg(Index - 1); + home.LoadValue(stream); + stream.EmitStInd(ManagedType); } protected virtual void EmitMarshalArgumentManagedToNative() From 5e098f60739d025bef45eac64f637a71881f722e Mon Sep 17 00:00:00 2001 From: Andrii Kurdiumov Date: Tue, 8 Jun 2021 12:31:47 +0600 Subject: [PATCH 9/9] Reformat code based on PR feedback --- .../TypeSystem/Interop/IL/Marshaller.cs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs index 3d90c4f82ba8..d79d54c618a2 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs @@ -149,6 +149,8 @@ internal virtual bool CleanupRequired } } + internal bool IsHRSwappedRetVal => Index == 0 && !Return; + public bool In; public bool Out; public bool Return; @@ -512,7 +514,7 @@ protected virtual void EmitMarshalReturnValueManagedToNative() public virtual void LoadReturnValue(ILCodeStream codeStream) { - Debug.Assert(Return || Index == 0); + Debug.Assert(Return || IsHRSwappedRetVal); switch (MarshalDirection) { @@ -654,7 +656,7 @@ protected void PropagateFromByRefArg(ILCodeStream stream, Home home) 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 (Index == 0) + if (IsHRSwappedRetVal) { // Returning result would be handled by LoadReturnValue return; @@ -910,7 +912,13 @@ class BlittableValueMarshaller : Marshaller { protected override void EmitMarshalArgumentManagedToNative() { - if (IsNativeByRef && MarshalDirection == MarshalDirection.Forward && Index > 0) + if (IsHRSwappedRetVal) + { + base.EmitMarshalArgumentManagedToNative(); + return; + } + + if (IsNativeByRef && MarshalDirection == MarshalDirection.Forward) { ILCodeStream marshallingCodeStream = _ilCodeStreams.MarshallingCodeStream; ILEmitter emitter = _ilCodeStreams.Emitter; @@ -924,13 +932,9 @@ protected override void EmitMarshalArgumentManagedToNative() marshallingCodeStream.EmitStLoc(native); _ilCodeStreams.CallsiteSetupCodeStream.EmitLdLoc(native); } - else if (Index > 0) - { - _ilCodeStreams.CallsiteSetupCodeStream.EmitLdArg(Index - 1); - } else { - base.EmitMarshalArgumentManagedToNative(); + _ilCodeStreams.CallsiteSetupCodeStream.EmitLdArg(Index - 1); } }