From b87f0e4ae2592a3ac57c3e9311f629863d79409d Mon Sep 17 00:00:00 2001 From: Fadi Hanna Date: Fri, 15 Nov 2019 15:40:01 -0800 Subject: [PATCH 1/3] Use precompiled PInvoke stubs from R2R image without a shared IL stub This is only for images compiled with crossgen2. Changes include some refactoring work around the signature parsing to compute marshalling requirements. We had two separate implementations after pulling the PInvoke work from the single-exe branch. Consolidated into one implementation. --- src/coreclr/src/inc/readytorun.h | 11 +- src/coreclr/src/jit/importer.cpp | 5 +- .../Common/Internal/Runtime/ModuleHeaders.cs | 8 ++ .../Common/JitInterface/CorInfoImpl.cs | 17 +-- .../PInvokeTargetNativeMethod.Diagnostic.cs | 19 +++ .../PInvokeTargetNativeMethod.Mangling.cs | 27 +++++ .../PInvokeTargetNativeMethod.Sorting.cs | 20 ++++ .../IL/Stubs/PInvokeTargetNativeMethod.cs | 92 ++++++++++++++ .../TypeSystem/Interop/IL/Marshaller.cs | 81 +++++++++++-- .../TypeSystem/Interop/IL/MarshallersCache.cs | 85 +++++++++++++ .../ReadyToRun/HeaderNode.cs | 2 +- .../Compiler/MethodExtensions.cs | 22 ++++ .../Compiler/ReadyToRunCodegenCompilation.cs | 3 +- ...RunSingleAssemblyCompilationModuleGroup.cs | 18 ++- .../IL/Stubs/PInvokeILEmitter.cs | 97 ++++----------- .../ILCompiler.ReadyToRun.csproj | 2 + .../JitInterface/CorInfoImpl.ReadyToRun.cs | 112 +++++------------- .../ILCompiler.TypeSystem.ReadyToRun.csproj | 12 ++ src/coreclr/src/vm/prestub.cpp | 67 ++++++----- src/coreclr/src/vm/readytoruninfo.h | 6 + 20 files changed, 479 insertions(+), 227 deletions(-) create mode 100644 src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Diagnostic.cs create mode 100644 src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Mangling.cs create mode 100644 src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Sorting.cs create mode 100644 src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.cs create mode 100644 src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/MarshallersCache.cs create mode 100644 src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/MethodExtensions.cs diff --git a/src/coreclr/src/inc/readytorun.h b/src/coreclr/src/inc/readytorun.h index 514bb0c163bad1..e453d7b5d5cfa7 100644 --- a/src/coreclr/src/inc/readytorun.h +++ b/src/coreclr/src/inc/readytorun.h @@ -44,12 +44,11 @@ struct READYTORUN_SECTION }; enum ReadyToRunFlag -{ - // Set if the original IL assembly was platform-neutral - READYTORUN_FLAG_PLATFORM_NEUTRAL_SOURCE = 0x00000001, - READYTORUN_FLAG_SKIP_TYPE_VALIDATION = 0x00000002, - // Set of methods with native code was determined using profile data - READYTORUN_FLAG_PARTIAL = 0x00000004, +{ + READYTORUN_FLAG_PLATFORM_NEUTRAL_SOURCE = 0x00000001, // Set if the original IL assembly was platform-neutral + READYTORUN_FLAG_SKIP_TYPE_VALIDATION = 0x00000002, // Set of methods with native code was determined using profile data + READYTORUN_FLAG_PARTIAL = 0x00000004, + READYTORUN_FLAG_CROSSGEN2_IMAGE = 0x00000008, // Set if image was compiled using crossgen2 }; enum ReadyToRunSectionType diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 0558622db43425..cdd44f743b795d 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -6483,9 +6483,8 @@ void Compiler::impCheckForPInvokeCall( return; } - // Legal PInvoke CALL in PInvoke IL stubs must be inlined to avoid infinite recursive - // inlining in CoreRT. Skip the ambient conditions checks and profitability checks. - if (!IsTargetAbi(CORINFO_CORERT_ABI) || (info.compFlags & CORINFO_FLG_PINVOKE) == 0) + // Skip the ambient conditions checks and profitability checks if forced inlining was requested. + if ((mflags & CORINFO_FLG_FORCEINLINE) == 0) { if (!impCanPInvokeInline()) { diff --git a/src/coreclr/src/tools/crossgen2/Common/Internal/Runtime/ModuleHeaders.cs b/src/coreclr/src/tools/crossgen2/Common/Internal/Runtime/ModuleHeaders.cs index c18b0d0566da1b..37e36a7ac3b63c 100644 --- a/src/coreclr/src/tools/crossgen2/Common/Internal/Runtime/ModuleHeaders.cs +++ b/src/coreclr/src/tools/crossgen2/Common/Internal/Runtime/ModuleHeaders.cs @@ -36,6 +36,14 @@ internal struct ReadyToRunHeader }; #pragma warning restore 0169 + enum ReadyToRunFlag + { + PlatformNeutralSource = 1, // Set if the original IL assembly was platform-neutral + SkipTypeValidation = 2, + Partial = 4, // Set of methods with native code was determined using profile data + Crossgen2Image = 8 // Set if image was compiled using crossgen2 + }; + // // ReadyToRunSectionType IDs are used by the runtime to look up specific global data sections // from each module linked into the final binary. New sections should be added at the bottom diff --git a/src/coreclr/src/tools/crossgen2/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/src/tools/crossgen2/Common/JitInterface/CorInfoImpl.cs index c8aa9897f7af2b..86fdeea87439da 100644 --- a/src/coreclr/src/tools/crossgen2/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/src/tools/crossgen2/Common/JitInterface/CorInfoImpl.cs @@ -659,6 +659,11 @@ private uint getMethodAttribsInternal(MethodDesc method) if (method.IsPInvoke) { result |= CorInfoFlag.CORINFO_FLG_PINVOKE; + + if (method.IsRawPInvoke()) + { + result |= CorInfoFlag.CORINFO_FLG_FORCEINLINE; + } } #if READYTORUN @@ -774,14 +779,6 @@ private CorInfoInline canInline(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHO MethodDesc callerMethod = HandleToObject(callerHnd); MethodDesc calleeMethod = HandleToObject(calleeHnd); -#if READYTORUN - // IL stubs don't inline well - if (calleeMethod.IsPInvoke) - { - return CorInfoInline.INLINE_NEVER; - } -#endif - if (_compilation.CanInline(callerMethod, calleeMethod)) { // No restrictions on inlining @@ -3066,10 +3063,6 @@ private uint getJitFlags(ref CORJIT_FLAGS flags, uint sizeInBytes) if (this.MethodBeingCompiled.IsPInvoke) { flags.Set(CorJitFlag.CORJIT_FLAG_IL_STUB); - -#if READYTORUN - flags.Set(CorJitFlag.CORJIT_FLAG_PUBLISH_SECRET_PARAM); -#endif } if (this.MethodBeingCompiled.IsNoOptimization) diff --git a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Diagnostic.cs b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Diagnostic.cs new file mode 100644 index 00000000000000..2cb4d1ea33c4b1 --- /dev/null +++ b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Diagnostic.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Internal.TypeSystem; + +namespace Internal.IL.Stubs +{ + partial class PInvokeTargetNativeMethod + { + public override string DiagnosticName + { + get + { + return _declMethod.DiagnosticName; + } + } + } +} diff --git a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Mangling.cs b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Mangling.cs new file mode 100644 index 00000000000000..79ea39a9001f17 --- /dev/null +++ b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Mangling.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. +// See the LICENSE file in the project root for more information. + +using Internal.TypeSystem; + +namespace Internal.IL.Stubs +{ + partial class PInvokeTargetNativeMethod : IPrefixMangledMethod + { + MethodDesc IPrefixMangledMethod.BaseMethod + { + get + { + return _declMethod; + } + } + + string IPrefixMangledMethod.Prefix + { + get + { + return "rawpinvoke"; + } + } + } +} diff --git a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Sorting.cs b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Sorting.cs new file mode 100644 index 00000000000000..ea754346f7a636 --- /dev/null +++ b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.Sorting.cs @@ -0,0 +1,20 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Internal.TypeSystem; + +namespace Internal.IL.Stubs +{ + // Functionality related to deterministic ordering of types + partial class PInvokeTargetNativeMethod + { + protected internal override int ClassCode => -1626939381; + + protected internal override int CompareToImpl(MethodDesc other, TypeSystemComparer comparer) + { + var otherMethod = (PInvokeTargetNativeMethod)other; + return comparer.Compare(_declMethod, otherMethod._declMethod); + } + } +} diff --git a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.cs b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.cs new file mode 100644 index 00000000000000..519dee4e9abfca --- /dev/null +++ b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeTargetNativeMethod.cs @@ -0,0 +1,92 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Internal.TypeSystem; + +namespace Internal.IL.Stubs +{ + /// + /// Synthetic method that represents the actual PInvoke target method. + /// All parameters are simple types. There will be no code + /// generated for this method. Instead, a static reference to a symbol will be emitted. + /// + public sealed partial class PInvokeTargetNativeMethod : MethodDesc + { + private readonly MethodDesc _declMethod; + private readonly MethodSignature _signature; + + public MethodDesc Target + { + get + { + return _declMethod; + } + } + + public PInvokeTargetNativeMethod(MethodDesc declMethod, MethodSignature signature) + { + _declMethod = declMethod; + _signature = signature; + } + + public override TypeSystemContext Context + { + get + { + return _declMethod.Context; + } + } + + public override TypeDesc OwningType + { + get + { + return _declMethod.OwningType; + } + } + + public override MethodSignature Signature + { + get + { + return _signature; + } + } + + public override string Name + { + get + { + return _declMethod.Name; + } + } + + public override bool HasCustomAttribute(string attributeNamespace, string attributeName) + { + return false; + } + + public override bool IsPInvoke + { + get + { + return true; + } + } + + public override bool IsNoInlining + { + get + { + // This method does not have real IL body. NoInlining stops the JIT asking for it. + return true; + } + } + + public override PInvokeMetadata GetPInvokeMethodMetadata() + { + return _declMethod.GetPInvokeMethodMetadata(); + } + } +} diff --git a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs index 624ff457e07ed3..d7c5d56336f366 100644 --- a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs +++ b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs @@ -70,6 +70,8 @@ public enum MarshallerType // and also argument specific marshalling informaiton abstract partial class Marshaller { + static MethodMarshallersCache s_marshallersCache = new MethodMarshallersCache(); + #region Instance state information public TypeSystemContext Context; #if !READYTORUN @@ -155,7 +157,7 @@ internal virtual bool CleanupRequired protected PInvokeILCodeStreams _ilCodeStreams; protected Home _managedHome; protected Home _nativeHome; -#endregion + #endregion enum HomeType { @@ -255,7 +257,7 @@ public void StoreValue(ILCodeStream stream) int _argIndex; } -#region Creation of marshallers + #region Creation of marshallers /// /// Protected ctor @@ -365,12 +367,18 @@ public static Marshaller CreateMarshaller(TypeDesc parameterType, return marshaller; } - public bool IsMarshallingRequired() + public static Marshaller[] GetMarshallersForMethod(MethodDesc targetMethod) { - if (Out) - return true; + Debug.Assert(targetMethod.IsPInvoke); + return s_marshallersCache.GetOrCreateValue(targetMethod).Marshallers; + } + #endregion - switch (MarshallerKind) + + #region Marshalling Requirement Checking + private static bool UsesCustomMarshaller(MarshallerKind kind) + { + switch (kind) { case MarshallerKind.Enum: case MarshallerKind.BlittableValue: @@ -381,7 +389,54 @@ public bool IsMarshallingRequired() } return true; } -#endregion + + private bool UsesCustomMarshaller() + { + return Out || UsesCustomMarshaller(MarshallerKind); + } + + public static bool IsCustomMarshallingRequired(MethodDesc targetMethod) + { + Debug.Assert(targetMethod.IsPInvoke); + + if (targetMethod.GetPInvokeMethodMetadata().Flags.SetLastError) + return true; + + var marshallers = s_marshallersCache.GetOrCreateValue(targetMethod).Marshallers; + + for (int i = 0; i < marshallers.Length; i++) + { + if (marshallers[i].UsesCustomMarshaller()) + return true; + } + return false; + } + + public static bool IsCustomMarshallingRequired(MethodSignature methodSig, ParameterMetadata[] paramMetadata) + { + for (int i = 0, paramIndex = 0; i < methodSig.Length + 1; 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, + parameterMetadata.MarshalAsDescriptor, + parameterMetadata.Return, + isAnsi: true, + MarshallerType.Argument, + out MarshallerKind elementMarshallerKind); + + if (UsesCustomMarshaller(marshallerKind)) + return true; + } + + return false; + } + #endregion public virtual void EmitMarshallingIL(PInvokeILCodeStreams pInvokeILCodeStreams) { @@ -1095,17 +1150,17 @@ protected override void TransformManagedToNative(ILCodeStream codeStream) var lRangeCheck = emitter.NewCodeLabel(); var lLoopHeader = emitter.NewCodeLabel(); - var lNullArray = emitter.NewCodeLabel(); + var lNullArray = emitter.NewCodeLabel(); var vNativeTemp = emitter.NewLocal(NativeType); var vIndex = emitter.NewLocal(Context.GetWellKnownType(WellKnownType.Int32)); var vSizeOf = emitter.NewLocal(Context.GetWellKnownType(WellKnownType.IntPtr)); var vLength = emitter.NewLocal(Context.GetWellKnownType(WellKnownType.Int32)); - + // Check for null array LoadManagedValue(codeStream); codeStream.Emit(ILOpcode.brfalse, lNullArray); - + // loads the number of elements EmitElementCount(codeStream, MarshalDirection.Forward); codeStream.EmitStLoc(vLength); @@ -1405,7 +1460,7 @@ private bool ShouldBePinned { get { - return MarshalDirection == MarshalDirection.Forward + return MarshalDirection == MarshalDirection.Forward && MarshallerType != MarshallerType.Field && !IsManagedByRef && In @@ -1529,7 +1584,7 @@ protected override void AllocAndTransformManagedToNative(ILCodeStream codeStream // // ANSI marshalling. Allocate a byte array, copy characters // - + #if READYTORUN var stringToAnsi = Context.SystemModule.GetKnownType("System.StubHelpers", "AnsiBSTRMarshaler") @@ -1671,7 +1726,7 @@ private void AllocSafeHandle(ILCodeStream codeStream) #endif codeStream.Emit(ILOpcode.newobj, emitter.NewToken(exceptionCtor)); codeStream.Emit(ILOpcode.throw_); - + // This is unreachable, but it maintains invariants about stack height prescribed by ECMA-335 codeStream.Emit(ILOpcode.ldnull); return; diff --git a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/MarshallersCache.cs b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/MarshallersCache.cs new file mode 100644 index 00000000000000..51dfd9ac53b041 --- /dev/null +++ b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/MarshallersCache.cs @@ -0,0 +1,85 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Diagnostics; + +namespace Internal.TypeSystem.Interop +{ + internal sealed class MethodMarshallersCache : LockFreeReaderHashtable + { + protected override int GetKeyHashCode(MethodDesc key) + { + return key.GetHashCode(); + } + protected override int GetValueHashCode(MethodMarshallers value) + { + return value.Method.GetHashCode(); + } + protected override bool CompareKeyToValue(MethodDesc key, MethodMarshallers value) + { + return Object.ReferenceEquals(key, value.Method); + } + protected override bool CompareValueToValue(MethodMarshallers value1, MethodMarshallers value2) + { + return Object.ReferenceEquals(value1.Method, value2.Method); + } + protected override MethodMarshallers CreateValueFromKey(MethodDesc key) + { + Debug.Assert(key.IsPInvoke); + + return new MethodMarshallers(key, InitializeMarshallers(key, key.GetPInvokeMethodMetadata().Flags)); + } + private static Marshaller[] InitializeMarshallers(MethodDesc targetMethod, PInvokeFlags flags) + { + MarshalDirection direction = MarshalDirection.Forward; + MethodSignature methodSig = targetMethod.Signature; + + 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); + if (parameterIndex == parameterMetadataArray.Length || i < parameterMetadataArray[parameterIndex].Index) + { + // if we don't have metadata for the parameter, create a dummy one + parameterMetadata = new ParameterMetadata(i, ParameterMetadataAttributes.None, null); + } + else + { + Debug.Assert(i == parameterMetadataArray[parameterIndex].Index); + parameterMetadata = parameterMetadataArray[parameterIndex++]; + } + + TypeDesc parameterType = (i == 0) ? methodSig.ReturnType : methodSig[i - 1]; //first item is the return type + marshallers[i] = Marshaller.CreateMarshaller(parameterType, + MarshallerType.Argument, + parameterMetadata.MarshalAsDescriptor, + direction, + marshallers, + parameterMetadata.Index, + flags, + parameterMetadata.In, + parameterMetadata.Out, + parameterMetadata.Return); + } + + return marshallers; + } + + internal class MethodMarshallers + { + internal MethodMarshallers(MethodDesc targetMethod, Marshaller[] marshallers) + { + Method = targetMethod; + Marshallers = marshallers; + } + + public readonly MethodDesc Method; + public readonly Marshaller[] Marshallers; + } + } +} diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/HeaderNode.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/HeaderNode.cs index 757ef62aca7d05..3fd611a2454ac2 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/HeaderNode.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/HeaderNode.cs @@ -119,7 +119,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) builder.EmitShort((short)(ReadyToRunHeaderConstants.CurrentMinorVersion)); // ReadyToRunHeader.Flags - builder.EmitInt(0); + builder.EmitInt((int)ReadyToRunFlag.Crossgen2Image); // ReadyToRunHeader.NumberOfSections ObjectDataBuilder.Reservation sectionCountReservation = builder.ReserveInt(); diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/MethodExtensions.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/MethodExtensions.cs new file mode 100644 index 00000000000000..8bbe4255a4de1f --- /dev/null +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/MethodExtensions.cs @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Internal.TypeSystem; + +namespace ILCompiler +{ + static class MethodExtensions + { + /// + /// Returns true if is an actual native entrypoint. + /// There's a distinction between when a method reports it's a PInvoke in the metadata + /// versus how it's treated in the compiler. For many PInvoke methods the compiler will generate + /// an IL body. The methods with an IL method body shouldn't be treated as PInvoke within the compiler. + /// + public static bool IsRawPInvoke(this MethodDesc method) + { + return method.IsPInvoke && (method is Internal.IL.Stubs.PInvokeTargetNativeMethod); + } + } +} diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs index 059dceaba1b92a..e9cfb44703e4a4 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs @@ -140,8 +140,7 @@ protected override MethodILData CreateValueFromKey(MethodDesc key) && key.IsPInvoke && _compilationModuleGroup.GeneratesPInvoke(key)) { - // TODO: enable when IL Stubs are fixed to be non-shared - // methodIL = PInvokeILEmitter.EmitIL(key); + methodIL = PInvokeILEmitter.EmitIL(key); } return new MethodILData() { Method = key, MethodIL = methodIL }; diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs index 271dcdf8bf2ad8..33e21c3e967573 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs @@ -7,7 +7,7 @@ using Internal.TypeSystem; using Internal.TypeSystem.Ecma; - +using Internal.TypeSystem.Interop; using Debug = System.Diagnostics.Debug; namespace ILCompiler @@ -191,13 +191,19 @@ public override bool CanInline(MethodDesc callerMethod, MethodDesc calleeMethod) public override bool GeneratesPInvoke(MethodDesc method) { - // PInvokes depend on details of the core library. + // PInvokes depend on details of the core library, so for now only compile them if: + // 1) We're compiling the core library module, or + // 2) We're compiling any module, and no complex marshalling is needed // - // We allow compiling them if both the module of the pinvoke and the core library - // are in the version bubble. + // TODO Future: consider compiling PInvokes with complex marshalling in version bubble + // mode when the core library is included in the bubble. + Debug.Assert(method is EcmaMethod); - return _versionBubbleModuleSet.Contains(((EcmaMethod)method).Module) - && _versionBubbleModuleSet.Contains(method.Context.SystemModule); + + if (((EcmaMethod)method).Module.Equals(method.Context.SystemModule)) + return true; + + return !Marshaller.IsCustomMarshallingRequired(method); } } } diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/IL/Stubs/PInvokeILEmitter.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/IL/Stubs/PInvokeILEmitter.cs index 954abaf6c23e4e..1b8897c14ea984 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/IL/Stubs/PInvokeILEmitter.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/IL/Stubs/PInvokeILEmitter.cs @@ -3,7 +3,11 @@ // See the LICENSE file in the project root for more information. using System; +using System.Net; +using System.Collections.Concurrent; +using System.Collections.Generic; +using ILCompiler; using Internal.TypeSystem; using Internal.TypeSystem.Interop; using Internal.JitInterface; @@ -17,6 +21,8 @@ namespace Internal.IL.Stubs /// public struct PInvokeILEmitter { + private static ConcurrentDictionary s_emittedPinvokeILStubs = new ConcurrentDictionary(); + private readonly MethodDesc _targetMethod; private readonly Marshaller[] _marshallers; private readonly PInvokeMetadata _importMetadata; @@ -26,48 +32,7 @@ private PInvokeILEmitter(MethodDesc targetMethod) Debug.Assert(targetMethod.IsPInvoke); _targetMethod = targetMethod; _importMetadata = targetMethod.GetPInvokeMethodMetadata(); - - _marshallers = InitializeMarshallers(targetMethod, _importMetadata.Flags); - } - - private static Marshaller[] InitializeMarshallers(MethodDesc targetMethod, PInvokeFlags flags) - { - MarshalDirection direction = MarshalDirection.Forward; - MethodSignature methodSig = targetMethod.Signature; - - ParameterMetadata[] parameterMetadataArray = targetMethod.GetParameterMetadata(); - Marshaller[] marshallers = new Marshaller[methodSig.Length + 1]; - int parameterIndex = 0; - ParameterMetadata parameterMetadata; - - for (int i = 0; i < marshallers.Length; i++) - { - Debug.Assert(parameterIndex == parameterMetadataArray.Length || i <= parameterMetadataArray[parameterIndex].Index); - if (parameterIndex == parameterMetadataArray.Length || i < parameterMetadataArray[parameterIndex].Index) - { - // if we don't have metadata for the parameter, create a dummy one - parameterMetadata = new ParameterMetadata(i, ParameterMetadataAttributes.None, null); - } - else - { - Debug.Assert(i == parameterMetadataArray[parameterIndex].Index); - parameterMetadata = parameterMetadataArray[parameterIndex++]; - } - TypeDesc parameterType = (i == 0) ? methodSig.ReturnType : methodSig[i - 1]; //first item is the return type - marshallers[i] = Marshaller.CreateMarshaller(parameterType, - MarshallerType.Argument, - parameterMetadata.MarshalAsDescriptor, - direction, - marshallers, - parameterMetadata.Index, - flags, - parameterMetadata.In, - parameterMetadata.Out, - parameterMetadata.Return - ); - } - - return marshallers; + _marshallers = Marshaller.GetMarshallersForMethod(targetMethod); } private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams) @@ -93,18 +58,13 @@ private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams) nativeParameterTypes[i - 1] = _marshallers[i].NativeParameterType; } - callsiteSetupCodeStream.Emit(ILOpcode.call, emitter.NewToken( - stubHelpersType.GetKnownMethod("GetStubContext", null))); - callsiteSetupCodeStream.Emit(ILOpcode.call, emitter.NewToken( - stubHelpersType.GetKnownMethod("GetNDirectTarget", null))); - - MethodSignatureFlags unmanagedCallConv = _importMetadata.Flags.UnmanagedCallingConvention; - MethodSignature nativeSig = new MethodSignature( - _targetMethod.Signature.Flags | unmanagedCallConv, 0, nativeReturnType, + _targetMethod.Signature.Flags, 0, nativeReturnType, nativeParameterTypes); - callsiteSetupCodeStream.Emit(ILOpcode.calli, emitter.NewToken(nativeSig)); + var rawTargetMethod = new PInvokeTargetNativeMethod(_targetMethod, nativeSig); + + callsiteSetupCodeStream.Emit(ILOpcode.call, emitter.NewToken(rawTargetMethod)); // if the SetLastError flag is set in DllImport, call the PInvokeMarshal. // SaveLastWin32Error so that last error can be used later by calling @@ -133,14 +93,21 @@ private MethodIL EmitIL() _marshallers[0].LoadReturnValue(unmarshallingCodestream); unmarshallingCodestream.Emit(ILOpcode.ret); - return new PInvokeILStubMethodIL((ILStubMethodIL)emitter.Link(_targetMethod), IsStubRequired()); + return new PInvokeILStubMethodIL((ILStubMethodIL)emitter.Link(_targetMethod)); } public static MethodIL EmitIL(MethodDesc method) { try { - return new PInvokeILEmitter(method).EmitIL(); + MethodIL methodIL; + if (!s_emittedPinvokeILStubs.TryGetValue(method, out methodIL)) + { + methodIL = new PInvokeILEmitter(method).EmitIL(); + s_emittedPinvokeILStubs.TryAdd(method, methodIL); + } + + return methodIL; } catch (NotSupportedException) { @@ -151,31 +118,15 @@ public static MethodIL EmitIL(MethodDesc method) throw new RequiresRuntimeJitException(method); } } - - private bool IsStubRequired() - { - Debug.Assert(_targetMethod.IsPInvoke); - - if (_importMetadata.Flags.SetLastError) - { - return true; - } - - for (int i = 0; i < _marshallers.Length; i++) - { - if (_marshallers[i].IsMarshallingRequired()) - return true; - } - return false; - } } public sealed class PInvokeILStubMethodIL : ILStubMethodIL { - public bool IsStubRequired { get; } - public PInvokeILStubMethodIL(ILStubMethodIL methodIL, bool isStubRequired) : base(methodIL) + public bool IsCustomMarshallingRequired { get; } + + public PInvokeILStubMethodIL(ILStubMethodIL methodIL) : base(methodIL) { - IsStubRequired = isStubRequired; + IsCustomMarshallingRequired = Marshaller.IsCustomMarshallingRequired(methodIL.OwningMethod); } } } diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj index 49afbe7fb49341..1336482eab8917 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj @@ -44,6 +44,7 @@ + @@ -169,6 +170,7 @@ + diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index e44aa29d650a04..42f4c77f5d5535 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -11,6 +11,7 @@ using System.Runtime.InteropServices; using Internal.IL; +using Internal.IL.Stubs; using Internal.Text; using Internal.TypeSystem; using Internal.TypeSystem.Ecma; @@ -661,6 +662,9 @@ private ModuleToken HandleToModuleToken(ref CORINFO_RESOLVED_TOKEN pResolvedToke if (resultDef is MethodDesc) { + if (resultDef is IL.Stubs.PInvokeTargetNativeMethod rawPinvoke) + resultDef = rawPinvoke.Target; + Debug.Assert(resultDef is EcmaMethod); Debug.Assert(_compilation.NodeFactory.CompilationModuleGroup.VersionsWithType(((EcmaMethod)resultDef).OwningType)); token = (mdToken)MetadataTokens.GetToken(((EcmaMethod)resultDef).Handle); @@ -1403,6 +1407,10 @@ private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESO { nonUnboxingMethod = methodToCall.GetUnboxedMethod(); } + if (nonUnboxingMethod is IL.Stubs.PInvokeTargetNativeMethod rawPinvoke) + { + nonUnboxingMethod = rawPinvoke.Target; + } // READYTORUN: FUTURE: Direct calls if possible pResult->codePointerOrStubLookup.constLookup = CreateConstLookupToSymbol( @@ -1926,7 +1934,10 @@ private HRESULT getMethodBlockCounts(CORINFO_METHOD_STRUCT_* ftnHnd, ref uint pC private void getAddressOfPInvokeTarget(CORINFO_METHOD_STRUCT_* method, ref CORINFO_CONST_LOOKUP pLookup) { - EcmaMethod ecmaMethod = (EcmaMethod)HandleToObject(method); + MethodDesc methodDesc = HandleToObject(method); + if (methodDesc is IL.Stubs.PInvokeTargetNativeMethod rawPInvoke) + methodDesc = rawPInvoke.Target; + EcmaMethod ecmaMethod = (EcmaMethod)methodDesc; ModuleToken moduleToken = new ModuleToken(ecmaMethod.Module, ecmaMethod.Handle); MethodWithToken methodWithToken = new MethodWithToken(ecmaMethod, moduleToken, constrainedType: null); @@ -1944,13 +1955,26 @@ private bool pInvokeMarshalingRequired(CORINFO_METHOD_STRUCT_* handle, CORINFO_S if (handle != null) { - EcmaMethod method = (EcmaMethod)HandleToObject(handle); - return MethodRequiresMarshaling(method); + var method = HandleToObject(handle); + if (method.IsRawPInvoke()) + { + return false; + } + + MethodIL stubIL = _compilation.GetMethodIL(method); + if (stubIL == null) + { + // This is the case of a PInvoke method that requires custom marshallers that we can't use. + Debug.Assert(!_compilation.NodeFactory.CompilationModuleGroup.GeneratesPInvoke(method)); + return true; + } + + return ((PInvokeILStubMethodIL)stubIL).IsCustomMarshallingRequired; } else { var sig = (MethodSignature)HandleToObject((IntPtr)callSiteSig->pSig); - return SignatureRequiresMarshaling(sig); + return Marshaller.IsCustomMarshallingRequired(sig, Array.Empty()); } } @@ -1966,86 +1990,6 @@ private bool canGetCookieForPInvokeCalliSig(CORINFO_SIG_INFO* szMetaSig) throw new RequiresRuntimeJitException($"{MethodBeingCompiled} -> {nameof(canGetCookieForPInvokeCalliSig)}"); } - private bool MethodRequiresMarshaling(EcmaMethod method) - { - if (method.GetPInvokeMethodMetadata().Flags.SetLastError) - { - // SetLastError is handled by stub - return true; - } - - if ((method.ImplAttributes & MethodImplAttributes.PreserveSig) == 0) - { - // HRESULT swapping is handled by stub - return true; - } - - return SignatureRequiresMarshaling(method.Signature); - } - - private bool SignatureRequiresMarshaling(MethodSignature sig) - { - if (TypeRequiresMarshaling(sig.ReturnType, isReturnType: true)) - { - return true; - } - - foreach (TypeDesc argType in sig) - { - if (TypeRequiresMarshaling(argType, isReturnType: false)) - { - return true; - } - } - - return false; - } - - private bool TypeRequiresMarshaling(TypeDesc type, bool isReturnType) - { - switch (type.Category) - { - case TypeFlags.Pointer: - // TODO: custom modifiers S(NeedsCopyConstructorModifier, IsCopyConstructed) - break; - - case TypeFlags.Enum: - if (!_compilation.NodeFactory.CompilationModuleGroup.VersionsWithType(type)) - { - return true; - } - break; - - case TypeFlags.ValueType: - if (!MarshalUtils.IsBlittableType(type)) - { - return true; - } - if (!_compilation.NodeFactory.CompilationModuleGroup.VersionsWithType(type)) - { - return true; - } - if (isReturnType && !type.IsPrimitive) - { - return true; - } - break; - - case TypeFlags.Boolean: - case TypeFlags.Char: - return true; - - default: - if (!type.IsPrimitive) - { - return true; - } - break; - } - - return false; - } - private int SizeOfPInvokeTransitionFrame => ReadyToRunRuntimeConstants.READYTORUN_PInvokeTransitionFrameSizeInPointerUnits * _compilation.NodeFactory.Target.PointerSize; } } diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.TypeSystem.ReadyToRun/ILCompiler.TypeSystem.ReadyToRun.csproj b/src/coreclr/src/tools/crossgen2/ILCompiler.TypeSystem.ReadyToRun/ILCompiler.TypeSystem.ReadyToRun.csproj index df1d68c0de758b..83edec65cde8bc 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.TypeSystem.ReadyToRun/ILCompiler.TypeSystem.ReadyToRun.csproj +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.TypeSystem.ReadyToRun/ILCompiler.TypeSystem.ReadyToRun.csproj @@ -441,6 +441,18 @@ IL\Stubs\ILEmitter.cs + + IL\Stubs\PInvokeTargetNativeMethod.cs + + + IL\Stubs\PInvokeTargetNativeMethod.Diagnostic.cs + + + IL\Stubs\PInvokeTargetNativeMethod.Mangling.cs + + + IL\Stubs\PInvokeTargetNativeMethod.Sorting.cs + TypeSystem\CodeGen\FieldDesc.Serialization.cs diff --git a/src/coreclr/src/vm/prestub.cpp b/src/coreclr/src/vm/prestub.cpp index 509d3d315b5822..025fb98ed8f291 100644 --- a/src/coreclr/src/vm/prestub.cpp +++ b/src/coreclr/src/vm/prestub.cpp @@ -72,6 +72,14 @@ void MethodDesc::Init() #endif +#define LOG_USING_R2R_CODE(method) LOG((LF_ZAP, LL_INFO10000, \ + "ZAP: Using R2R precompiled code" FMT_ADDR " for %s.%s sig=\"%s\" (token %x).\n", \ + DBG_ADDR(pCode), \ + m_pszDebugClassName, \ + m_pszDebugMethodName, \ + m_pszDebugMethodSignature, \ + GetMemberDef())); + //========================================================================== PCODE MethodDesc::DoBackpatch(MethodTable * pMT, MethodTable *pDispatchingMT, BOOL fFullBackPatch) @@ -352,30 +360,27 @@ PCODE MethodDesc::PrepareILBasedCode(PrepareCodeConfig* pConfig) if (pConfig->MayUsePrecompiledCode()) { #ifdef FEATURE_READYTORUN - // TODO: Remove IsSystem check when IL Stubs are fixed to be non-shared - if (this->IsDynamicMethod() && GetLoaderModule()->IsSystem() && MayUsePrecompiledILStub()) + if (IsDynamicMethod() && GetLoaderModule()->IsSystem() && MayUsePrecompiledILStub()) { - DynamicMethodDesc *stubMethodDesc = this->AsDynamicMethodDesc(); - if (stubMethodDesc->IsILStub() && stubMethodDesc->IsPInvokeStub()) + // Images produced using crossgen2 have non-shareable pinvoke stubs which can't be used with the IL + // stubs that the runtime generates (they take no secret parameter, and each pinvoke has a separate code) + if (GetModule()->IsReadyToRun() && !GetModule()->GetReadyToRunInfo()->IsCrossgen2Image()) { - ILStubResolver *pStubResolver = stubMethodDesc->GetILStubResolver(); - if (pStubResolver->IsCLRToNativeInteropStub()) + DynamicMethodDesc* stubMethodDesc = this->AsDynamicMethodDesc(); + if (stubMethodDesc->IsILStub() && stubMethodDesc->IsPInvokeStub()) { - MethodDesc *pTargetMD = stubMethodDesc->GetILStubResolver()->GetStubTargetMethodDesc(); - if (pTargetMD != NULL) + ILStubResolver* pStubResolver = stubMethodDesc->GetILStubResolver(); + if (pStubResolver->IsCLRToNativeInteropStub()) { - pCode = pTargetMD->GetPrecompiledR2RCode(pConfig); - if (pCode != NULL) + MethodDesc* pTargetMD = stubMethodDesc->GetILStubResolver()->GetStubTargetMethodDesc(); + if (pTargetMD != NULL) { - LOG((LF_ZAP, LL_INFO10000, - "ZAP: Using R2R precompiled code" FMT_ADDR " for %s.%s sig=\"%s\" (token %x).\n", - DBG_ADDR(pCode), - m_pszDebugClassName, - m_pszDebugMethodName, - m_pszDebugMethodSignature, - GetMemberDef())); - - pConfig->SetNativeCode(pCode, &pCode); + pCode = pTargetMD->GetPrecompiledR2RCode(pConfig); + if (pCode != NULL) + { + LOG_USING_R2R_CODE(this); + pConfig->SetNativeCode(pCode, &pCode); + } } } } @@ -430,13 +435,7 @@ PCODE MethodDesc::GetPrecompiledCode(PrepareCodeConfig* pConfig) pCode = GetPrecompiledR2RCode(pConfig); if (pCode != NULL) { - LOG((LF_ZAP, LL_INFO10000, - "ZAP: Using R2R precompiled code" FMT_ADDR " for %s.%s sig=\"%s\" (token %x).\n", - DBG_ADDR(pCode), - m_pszDebugClassName, - m_pszDebugMethodName, - m_pszDebugMethodSignature, - GetMemberDef())); + LOG_USING_R2R_CODE(this); if (pConfig->SetNativeCode(pCode, &pCode)) { @@ -2025,7 +2024,21 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT) } // end else if (IsIL() || IsNoMetadata()) else if (IsNDirect()) { - pCode = GetStubForInteropMethod(this); + if (GetModule()->IsReadyToRun() && GetModule()->GetReadyToRunInfo()->IsCrossgen2Image() && MayUsePrecompiledILStub()) + { + // In crossgen2, we compile non-shareable IL stubs for pinvokes. If we can find code for such + // a stub, we'll use it directly instead and avoid emitting an IL stub. + PrepareCodeConfig config(NativeCodeVersion(this), TRUE, TRUE); + pCode = GetPrecompiledR2RCode(&config); + if (pCode != NULL) + { + LOG_USING_R2R_CODE(this); + } + } + + if (pCode == NULL) + pCode = GetStubForInteropMethod(this); + GetOrCreatePrecode(); } else if (IsFCall()) diff --git a/src/coreclr/src/vm/readytoruninfo.h b/src/coreclr/src/vm/readytoruninfo.h index c6057c7fdeb522..7d217711b0ea90 100644 --- a/src/coreclr/src/vm/readytoruninfo.h +++ b/src/coreclr/src/vm/readytoruninfo.h @@ -74,6 +74,12 @@ class ReadyToRunInfo return m_pHeader->Flags & READYTORUN_FLAG_PARTIAL; } + BOOL IsCrossgen2Image() + { + LIMITED_METHOD_CONTRACT; + return m_pHeader->Flags & READYTORUN_FLAG_CROSSGEN2_IMAGE; + } + PTR_PEImageLayout GetImage() { LIMITED_METHOD_CONTRACT; From 69bcb8b7e4a54397bc3237dc3d1431ca323efdc0 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Date: Mon, 18 Nov 2019 13:47:17 -0800 Subject: [PATCH 2/3] CR feedback --- src/coreclr/src/inc/readytorun.h | 4 ++-- src/coreclr/src/jit/importer.cpp | 5 +++-- .../Common/Internal/Runtime/ModuleHeaders.cs | 8 ++++---- .../Common/TypeSystem/Interop/IL/Marshaller.cs | 14 +++++++------- .../DependencyAnalysis/ReadyToRun/HeaderNode.cs | 2 +- ...dyToRunSingleAssemblyCompilationModuleGroup.cs | 4 ++-- .../IL/Stubs/PInvokeILEmitter.cs | 15 +++------------ .../JitInterface/CorInfoImpl.ReadyToRun.cs | 6 +++--- src/coreclr/src/tools/r2rdump/R2RHeader.cs | 6 +++--- src/coreclr/src/vm/prestub.cpp | 4 ++-- src/coreclr/src/vm/readytoruninfo.h | 4 ++-- 11 files changed, 32 insertions(+), 40 deletions(-) diff --git a/src/coreclr/src/inc/readytorun.h b/src/coreclr/src/inc/readytorun.h index e453d7b5d5cfa7..c78ec74d56727b 100644 --- a/src/coreclr/src/inc/readytorun.h +++ b/src/coreclr/src/inc/readytorun.h @@ -44,11 +44,11 @@ struct READYTORUN_SECTION }; enum ReadyToRunFlag -{ +{ READYTORUN_FLAG_PLATFORM_NEUTRAL_SOURCE = 0x00000001, // Set if the original IL assembly was platform-neutral READYTORUN_FLAG_SKIP_TYPE_VALIDATION = 0x00000002, // Set of methods with native code was determined using profile data READYTORUN_FLAG_PARTIAL = 0x00000004, - READYTORUN_FLAG_CROSSGEN2_IMAGE = 0x00000008, // Set if image was compiled using crossgen2 + READYTORUN_FLAG_NONSHARED_PINVOKE_STUBS = 0x00000008 // PInvoke stubs compiled into image are non-shareable (no secret parameter) }; enum ReadyToRunSectionType diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index cdd44f743b795d..0558622db43425 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -6483,8 +6483,9 @@ void Compiler::impCheckForPInvokeCall( return; } - // Skip the ambient conditions checks and profitability checks if forced inlining was requested. - if ((mflags & CORINFO_FLG_FORCEINLINE) == 0) + // Legal PInvoke CALL in PInvoke IL stubs must be inlined to avoid infinite recursive + // inlining in CoreRT. Skip the ambient conditions checks and profitability checks. + if (!IsTargetAbi(CORINFO_CORERT_ABI) || (info.compFlags & CORINFO_FLG_PINVOKE) == 0) { if (!impCanPInvokeInline()) { diff --git a/src/coreclr/src/tools/crossgen2/Common/Internal/Runtime/ModuleHeaders.cs b/src/coreclr/src/tools/crossgen2/Common/Internal/Runtime/ModuleHeaders.cs index 37e36a7ac3b63c..5c374310df9b76 100644 --- a/src/coreclr/src/tools/crossgen2/Common/Internal/Runtime/ModuleHeaders.cs +++ b/src/coreclr/src/tools/crossgen2/Common/Internal/Runtime/ModuleHeaders.cs @@ -38,10 +38,10 @@ internal struct ReadyToRunHeader enum ReadyToRunFlag { - PlatformNeutralSource = 1, // Set if the original IL assembly was platform-neutral - SkipTypeValidation = 2, - Partial = 4, // Set of methods with native code was determined using profile data - Crossgen2Image = 8 // Set if image was compiled using crossgen2 + READYTORUN_FLAG_PLATFORM_NEUTRAL_SOURCE = 0x00000001, // Set if the original IL assembly was platform-neutral + READYTORUN_FLAG_SKIP_TYPE_VALIDATION = 0x00000002, // Set of methods with native code was determined using profile data + READYTORUN_FLAG_PARTIAL = 0x00000004, + READYTORUN_FLAG_NONSHARED_PINVOKE_STUBS = 0x00000008 // PInvoke stubs compiled into image are non-shareable (no secret parameter) }; // diff --git a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs index d7c5d56336f366..de83b1411b186e 100644 --- a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs +++ b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs @@ -376,7 +376,7 @@ public static Marshaller[] GetMarshallersForMethod(MethodDesc targetMethod) #region Marshalling Requirement Checking - private static bool UsesCustomMarshaller(MarshallerKind kind) + private static bool IsMarshallingRequired(MarshallerKind kind) { switch (kind) { @@ -390,12 +390,12 @@ private static bool UsesCustomMarshaller(MarshallerKind kind) return true; } - private bool UsesCustomMarshaller() + private bool IsMarshallingRequired() { - return Out || UsesCustomMarshaller(MarshallerKind); + return Out || IsMarshallingRequired(MarshallerKind); } - public static bool IsCustomMarshallingRequired(MethodDesc targetMethod) + public static bool IsMarshallingRequired(MethodDesc targetMethod) { Debug.Assert(targetMethod.IsPInvoke); @@ -406,13 +406,13 @@ public static bool IsCustomMarshallingRequired(MethodDesc targetMethod) for (int i = 0; i < marshallers.Length; i++) { - if (marshallers[i].UsesCustomMarshaller()) + if (marshallers[i].IsMarshallingRequired()) return true; } return false; } - public static bool IsCustomMarshallingRequired(MethodSignature methodSig, ParameterMetadata[] paramMetadata) + public static bool IsMarshallingRequired(MethodSignature methodSig, ParameterMetadata[] paramMetadata) { for (int i = 0, paramIndex = 0; i < methodSig.Length + 1; i++) { @@ -430,7 +430,7 @@ public static bool IsCustomMarshallingRequired(MethodSignature methodSig, Parame MarshallerType.Argument, out MarshallerKind elementMarshallerKind); - if (UsesCustomMarshaller(marshallerKind)) + if (IsMarshallingRequired(marshallerKind)) return true; } diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/HeaderNode.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/HeaderNode.cs index 3fd611a2454ac2..b984f8fc7a683e 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/HeaderNode.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/HeaderNode.cs @@ -119,7 +119,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) builder.EmitShort((short)(ReadyToRunHeaderConstants.CurrentMinorVersion)); // ReadyToRunHeader.Flags - builder.EmitInt((int)ReadyToRunFlag.Crossgen2Image); + builder.EmitInt((int)ReadyToRunFlag.READYTORUN_FLAG_NONSHARED_PINVOKE_STUBS); // ReadyToRunHeader.NumberOfSections ObjectDataBuilder.Reservation sectionCountReservation = builder.ReserveInt(); diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs index 33e21c3e967573..a079c20134351f 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs @@ -193,7 +193,7 @@ public override bool GeneratesPInvoke(MethodDesc method) { // PInvokes depend on details of the core library, so for now only compile them if: // 1) We're compiling the core library module, or - // 2) We're compiling any module, and no complex marshalling is needed + // 2) We're compiling any module, and no marshalling is needed // // TODO Future: consider compiling PInvokes with complex marshalling in version bubble // mode when the core library is included in the bubble. @@ -203,7 +203,7 @@ public override bool GeneratesPInvoke(MethodDesc method) if (((EcmaMethod)method).Module.Equals(method.Context.SystemModule)) return true; - return !Marshaller.IsCustomMarshallingRequired(method); + return !Marshaller.IsMarshallingRequired(method); } } } diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/IL/Stubs/PInvokeILEmitter.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/IL/Stubs/PInvokeILEmitter.cs index 1b8897c14ea984..bf0975621aab13 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/IL/Stubs/PInvokeILEmitter.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/IL/Stubs/PInvokeILEmitter.cs @@ -21,8 +21,6 @@ namespace Internal.IL.Stubs /// public struct PInvokeILEmitter { - private static ConcurrentDictionary s_emittedPinvokeILStubs = new ConcurrentDictionary(); - private readonly MethodDesc _targetMethod; private readonly Marshaller[] _marshallers; private readonly PInvokeMetadata _importMetadata; @@ -100,14 +98,7 @@ public static MethodIL EmitIL(MethodDesc method) { try { - MethodIL methodIL; - if (!s_emittedPinvokeILStubs.TryGetValue(method, out methodIL)) - { - methodIL = new PInvokeILEmitter(method).EmitIL(); - s_emittedPinvokeILStubs.TryAdd(method, methodIL); - } - - return methodIL; + return new PInvokeILEmitter(method).EmitIL(); } catch (NotSupportedException) { @@ -122,11 +113,11 @@ public static MethodIL EmitIL(MethodDesc method) public sealed class PInvokeILStubMethodIL : ILStubMethodIL { - public bool IsCustomMarshallingRequired { get; } + public bool IsMarshallingRequired { get; } public PInvokeILStubMethodIL(ILStubMethodIL methodIL) : base(methodIL) { - IsCustomMarshallingRequired = Marshaller.IsCustomMarshallingRequired(methodIL.OwningMethod); + IsMarshallingRequired = Marshaller.IsMarshallingRequired(methodIL.OwningMethod); } } } diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 42f4c77f5d5535..4e51eabdb4f929 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1964,17 +1964,17 @@ private bool pInvokeMarshalingRequired(CORINFO_METHOD_STRUCT_* handle, CORINFO_S MethodIL stubIL = _compilation.GetMethodIL(method); if (stubIL == null) { - // This is the case of a PInvoke method that requires custom marshallers that we can't use. + // This is the case of a PInvoke method that requires marshallers, which we can't use in this compilation Debug.Assert(!_compilation.NodeFactory.CompilationModuleGroup.GeneratesPInvoke(method)); return true; } - return ((PInvokeILStubMethodIL)stubIL).IsCustomMarshallingRequired; + return ((PInvokeILStubMethodIL)stubIL).IsMarshallingRequired; } else { var sig = (MethodSignature)HandleToObject((IntPtr)callSiteSig->pSig); - return Marshaller.IsCustomMarshallingRequired(sig, Array.Empty()); + return Marshaller.IsMarshallingRequired(sig, Array.Empty()); } } diff --git a/src/coreclr/src/tools/r2rdump/R2RHeader.cs b/src/coreclr/src/tools/r2rdump/R2RHeader.cs index 71ee6cab24a048..f237e379889cc1 100644 --- a/src/coreclr/src/tools/r2rdump/R2RHeader.cs +++ b/src/coreclr/src/tools/r2rdump/R2RHeader.cs @@ -16,10 +16,10 @@ public class R2RHeader [Flags] public enum ReadyToRunFlag { - NONE = 0x00000000, - READYTORUN_FLAG_PLATFORM_NEUTRAL_SOURCE = 0x00000001, - READYTORUN_FLAG_SKIP_TYPE_VALIDATION = 0x00000002, + READYTORUN_FLAG_PLATFORM_NEUTRAL_SOURCE = 0x00000001, // Set if the original IL assembly was platform-neutral + READYTORUN_FLAG_SKIP_TYPE_VALIDATION = 0x00000002, // Set of methods with native code was determined using profile data READYTORUN_FLAG_PARTIAL = 0x00000004, + READYTORUN_FLAG_CROSSGEN2_IMAGE = 0x00000008 // Set if image was compiled using crossgen2 } /// diff --git a/src/coreclr/src/vm/prestub.cpp b/src/coreclr/src/vm/prestub.cpp index 025fb98ed8f291..c369ba8d46c2b4 100644 --- a/src/coreclr/src/vm/prestub.cpp +++ b/src/coreclr/src/vm/prestub.cpp @@ -364,7 +364,7 @@ PCODE MethodDesc::PrepareILBasedCode(PrepareCodeConfig* pConfig) { // Images produced using crossgen2 have non-shareable pinvoke stubs which can't be used with the IL // stubs that the runtime generates (they take no secret parameter, and each pinvoke has a separate code) - if (GetModule()->IsReadyToRun() && !GetModule()->GetReadyToRunInfo()->IsCrossgen2Image()) + if (GetModule()->IsReadyToRun() && !GetModule()->GetReadyToRunInfo()->HasNonShareablePInvokeStubs()) { DynamicMethodDesc* stubMethodDesc = this->AsDynamicMethodDesc(); if (stubMethodDesc->IsILStub() && stubMethodDesc->IsPInvokeStub()) @@ -2024,7 +2024,7 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT) } // end else if (IsIL() || IsNoMetadata()) else if (IsNDirect()) { - if (GetModule()->IsReadyToRun() && GetModule()->GetReadyToRunInfo()->IsCrossgen2Image() && MayUsePrecompiledILStub()) + if (GetModule()->IsReadyToRun() && GetModule()->GetReadyToRunInfo()->HasNonShareablePInvokeStubs() && MayUsePrecompiledILStub()) { // In crossgen2, we compile non-shareable IL stubs for pinvokes. If we can find code for such // a stub, we'll use it directly instead and avoid emitting an IL stub. diff --git a/src/coreclr/src/vm/readytoruninfo.h b/src/coreclr/src/vm/readytoruninfo.h index 7d217711b0ea90..370dca33ce3452 100644 --- a/src/coreclr/src/vm/readytoruninfo.h +++ b/src/coreclr/src/vm/readytoruninfo.h @@ -74,10 +74,10 @@ class ReadyToRunInfo return m_pHeader->Flags & READYTORUN_FLAG_PARTIAL; } - BOOL IsCrossgen2Image() + BOOL HasNonShareablePInvokeStubs() { LIMITED_METHOD_CONTRACT; - return m_pHeader->Flags & READYTORUN_FLAG_CROSSGEN2_IMAGE; + return m_pHeader->Flags & READYTORUN_FLAG_NONSHARED_PINVOKE_STUBS; } PTR_PEImageLayout GetImage() From b4210a1ccf82e6c1aafd6f94b6fe4464460ea988 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Date: Wed, 20 Nov 2019 13:44:34 -0800 Subject: [PATCH 3/3] Remove MarshallersCache.cs --- .../TypeSystem/Interop/IL/Marshaller.cs | 42 ++++++++- .../TypeSystem/Interop/IL/MarshallersCache.cs | 85 ------------------- .../ILCompiler.ReadyToRun.csproj | 1 - 3 files changed, 38 insertions(+), 90 deletions(-) delete mode 100644 src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/MarshallersCache.cs diff --git a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs index de83b1411b186e..2c27239f17ec49 100644 --- a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs +++ b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs @@ -70,8 +70,6 @@ public enum MarshallerType // and also argument specific marshalling informaiton abstract partial class Marshaller { - static MethodMarshallersCache s_marshallersCache = new MethodMarshallersCache(); - #region Instance state information public TypeSystemContext Context; #if !READYTORUN @@ -370,7 +368,43 @@ public static Marshaller CreateMarshaller(TypeDesc parameterType, public static Marshaller[] GetMarshallersForMethod(MethodDesc targetMethod) { Debug.Assert(targetMethod.IsPInvoke); - return s_marshallersCache.GetOrCreateValue(targetMethod).Marshallers; + + 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); + if (parameterIndex == parameterMetadataArray.Length || i < parameterMetadataArray[parameterIndex].Index) + { + // if we don't have metadata for the parameter, create a dummy one + parameterMetadata = new ParameterMetadata(i, ParameterMetadataAttributes.None, null); + } + else + { + Debug.Assert(i == parameterMetadataArray[parameterIndex].Index); + parameterMetadata = parameterMetadataArray[parameterIndex++]; + } + + TypeDesc parameterType = (i == 0) ? methodSig.ReturnType : methodSig[i - 1]; //first item is the return type + marshallers[i] = CreateMarshaller(parameterType, + MarshallerType.Argument, + parameterMetadata.MarshalAsDescriptor, + direction, + marshallers, + parameterMetadata.Index, + flags, + parameterMetadata.In, + parameterMetadata.Out, + parameterMetadata.Return); + } + + return marshallers; } #endregion @@ -402,7 +436,7 @@ public static bool IsMarshallingRequired(MethodDesc targetMethod) if (targetMethod.GetPInvokeMethodMetadata().Flags.SetLastError) return true; - var marshallers = s_marshallersCache.GetOrCreateValue(targetMethod).Marshallers; + var marshallers = GetMarshallersForMethod(targetMethod); for (int i = 0; i < marshallers.Length; i++) { diff --git a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/MarshallersCache.cs b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/MarshallersCache.cs deleted file mode 100644 index 51dfd9ac53b041..00000000000000 --- a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/MarshallersCache.cs +++ /dev/null @@ -1,85 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using System.Diagnostics; - -namespace Internal.TypeSystem.Interop -{ - internal sealed class MethodMarshallersCache : LockFreeReaderHashtable - { - protected override int GetKeyHashCode(MethodDesc key) - { - return key.GetHashCode(); - } - protected override int GetValueHashCode(MethodMarshallers value) - { - return value.Method.GetHashCode(); - } - protected override bool CompareKeyToValue(MethodDesc key, MethodMarshallers value) - { - return Object.ReferenceEquals(key, value.Method); - } - protected override bool CompareValueToValue(MethodMarshallers value1, MethodMarshallers value2) - { - return Object.ReferenceEquals(value1.Method, value2.Method); - } - protected override MethodMarshallers CreateValueFromKey(MethodDesc key) - { - Debug.Assert(key.IsPInvoke); - - return new MethodMarshallers(key, InitializeMarshallers(key, key.GetPInvokeMethodMetadata().Flags)); - } - private static Marshaller[] InitializeMarshallers(MethodDesc targetMethod, PInvokeFlags flags) - { - MarshalDirection direction = MarshalDirection.Forward; - MethodSignature methodSig = targetMethod.Signature; - - 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); - if (parameterIndex == parameterMetadataArray.Length || i < parameterMetadataArray[parameterIndex].Index) - { - // if we don't have metadata for the parameter, create a dummy one - parameterMetadata = new ParameterMetadata(i, ParameterMetadataAttributes.None, null); - } - else - { - Debug.Assert(i == parameterMetadataArray[parameterIndex].Index); - parameterMetadata = parameterMetadataArray[parameterIndex++]; - } - - TypeDesc parameterType = (i == 0) ? methodSig.ReturnType : methodSig[i - 1]; //first item is the return type - marshallers[i] = Marshaller.CreateMarshaller(parameterType, - MarshallerType.Argument, - parameterMetadata.MarshalAsDescriptor, - direction, - marshallers, - parameterMetadata.Index, - flags, - parameterMetadata.In, - parameterMetadata.Out, - parameterMetadata.Return); - } - - return marshallers; - } - - internal class MethodMarshallers - { - internal MethodMarshallers(MethodDesc targetMethod, Marshaller[] marshallers) - { - Method = targetMethod; - Marshallers = marshallers; - } - - public readonly MethodDesc Method; - public readonly Marshaller[] Marshallers; - } - } -} diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj index 1336482eab8917..89aa91e3f5b5e0 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj @@ -44,7 +44,6 @@ -