From 7a58d6d09f639d2fd0b4edb099ee5e739e337273 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 31 Aug 2021 13:14:10 +0200 Subject: [PATCH 1/3] Unify MethodDesc/MethodSignature IsMarshallingRequired logic For unmanaged calli crossgen2 was not properly checking all necessary conditions for a pinvoke being required. In particular it did not check for managed byrefs. Unify the MethodDesc/MethodSignature logic to get all the checks. Fix #58259 --- .../Interop/IL/Marshaller.ReadyToRun.cs | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs index 7da42e09791fda..927148cf006e26 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs @@ -40,21 +40,18 @@ protected static Marshaller CreateMarshaller(MarshallerKind kind) } } - public static Marshaller[] GetMarshallersForMethod(MethodDesc targetMethod) + private static Marshaller[] GetMarshallers( + MethodSignature methodSig, + PInvokeFlags flags, + ParameterMetadata[] parameterMetadataArray) { - Debug.Assert(targetMethod.IsPInvoke); - - MarshalDirection direction = MarshalDirection.Forward; - MethodSignature methodSig = targetMethod.Signature; - PInvokeFlags flags = targetMethod.GetPInvokeMethodMetadata().Flags; - - ParameterMetadata[] parameterMetadataArray = targetMethod.GetParameterMetadata(); Marshaller[] marshallers = new Marshaller[methodSig.Length + 1]; - ParameterMetadata parameterMetadata; for (int i = 0, parameterIndex = 0; i < marshallers.Length; i++) { Debug.Assert(parameterIndex == parameterMetadataArray.Length || i <= parameterMetadataArray[parameterIndex].Index); + + ParameterMetadata parameterMetadata; if (parameterIndex == parameterMetadataArray.Length || i < parameterMetadataArray[parameterIndex].Index) { // if we don't have metadata for the parameter, create a dummy one @@ -72,7 +69,7 @@ public static Marshaller[] GetMarshallersForMethod(MethodDesc targetMethod) methodSig.GetEmbeddedSignatureData(), MarshallerType.Argument, parameterMetadata.MarshalAsDescriptor, - direction, + MarshalDirection.Forward, marshallers, parameterMetadata.Index, flags, @@ -84,6 +81,24 @@ public static Marshaller[] GetMarshallersForMethod(MethodDesc targetMethod) return marshallers; } + + public static Marshaller[] GetMarshallersForMethod(MethodDesc targetMethod) + { + Debug.Assert(targetMethod.IsPInvoke); + return GetMarshallers( + targetMethod.Signature, + targetMethod.GetPInvokeMethodMetadata().Flags, + targetMethod.GetParameterMetadata()); + } + + public static Marshaller[] GetMarshallersForSignature(MethodSignature methodSig, ParameterMetadata[] paramMetadata) + { + return GetMarshallers( + methodSig, + new PInvokeFlags(PInvokeAttributes.None), + paramMetadata); + } + public static bool IsMarshallingRequired(MethodDesc targetMethod) { Debug.Assert(targetMethod.IsPInvoke); @@ -115,25 +130,10 @@ public static bool IsMarshallingRequired(MethodDesc targetMethod) public static bool IsMarshallingRequired(MethodSignature methodSig, ParameterMetadata[] paramMetadata) { - for (int i = 0, paramIndex = 0; i < methodSig.Length + 1; i++) + Marshaller[] marshallers = GetMarshallersForSignature(methodSig, paramMetadata); + for (int i = 0; i < marshallers.Length; i++) { - ParameterMetadata parameterMetadata = (paramIndex == paramMetadata.Length || i < paramMetadata[paramIndex].Index) ? - new ParameterMetadata(i, ParameterMetadataAttributes.None, null) : - paramMetadata[paramIndex++]; - - TypeDesc parameterType = (i == 0) ? methodSig.ReturnType : methodSig[i - 1]; //first item is the return type - - MarshallerKind marshallerKind = MarshalHelpers.GetMarshallerKind( - parameterType, - parameterIndex: i, - customModifierData: methodSig.GetEmbeddedSignatureData(), - parameterMetadata.MarshalAsDescriptor, - parameterMetadata.Return, - isAnsi: true, - MarshallerType.Argument, - out MarshallerKind elementMarshallerKind); - - if (IsMarshallingRequired(marshallerKind)) + if (marshallers[i].IsMarshallingRequired()) return true; } From 589019225fe0f32546532c4de2489a16c31836fe Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 31 Aug 2021 13:44:27 +0200 Subject: [PATCH 2/3] Add a simple regression test --- .../JitBlue/Runtime_58259/Runtime_58259.cs | 27 +++++++++++++++++++ .../Runtime_58259/Runtime_58259.csproj | 13 +++++++++ 2 files changed, 40 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.csproj diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.cs b/src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.cs new file mode 100644 index 00000000000000..7e49a3c9208310 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.cs @@ -0,0 +1,27 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +public unsafe class Runtime_58259 +{ + public static int Main() + { + M(out _); + return 100; + } + + static delegate* unmanaged _f; + + public static void M(out int index) + { + if (_f != null) + { + _f(out index); + _f(out index); + } + else + { + index = 0; + } + } +} + diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.csproj new file mode 100644 index 00000000000000..115844cca7b079 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.csproj @@ -0,0 +1,13 @@ + + + Exe + 1 + + + None + True + + + + + From bc0cacb21ca7b3dc3ed47df5a51d8e315ad409fc Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 31 Aug 2021 15:12:41 +0200 Subject: [PATCH 3/3] Fix test build --- .../JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.csproj index 115844cca7b079..d1dc80557b728c 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.csproj +++ b/src/tests/JIT/Regression/JitBlue/Runtime_58259/Runtime_58259.csproj @@ -1,6 +1,7 @@ Exe + True 1