A couple fixes to p/invoke marshalling pregeneration

* Expose the PreserveSig bit on PInvokeMetadata and reject precompiling methods with HResult munging
* Parse CustomMarshaller records to avoid asserting
* Place marshalling cleanup in a new cleanup stream that runs in a finally block
* Place SafeHandle code into the cleanup stream

src/coreclr/src/tools/crossgen2/Common/TypeSystem/Ecma/EcmaMethod.cs

@@ -466,7 +466,12 @@ public override PInvokeMetadata GetPInvokeMethodMetadata()
             Debug.Assert((int)MethodImportAttributes.CharSetUnicode == (int)PInvokeAttributes.CharSetUnicode);
             Debug.Assert((int)MethodImportAttributes.SetLastError == (int)PInvokeAttributes.SetLastError);
 
-            return new PInvokeMetadata(moduleName, name, (PInvokeAttributes)import.Attributes);
+            PInvokeAttributes attributes = (PInvokeAttributes)import.Attributes;
+
+            if ((ImplAttributes & MethodImplAttributes.PreserveSig) != 0)
+                attributes |= PInvokeAttributes.PreserveSig;
+
+            return new PInvokeMetadata(moduleName, name, attributes);
         }
 
         public override ParameterMetadata[] GetParameterMetadata()

src/coreclr/src/tools/crossgen2/Common/TypeSystem/Ecma/EcmaSignatureParser.cs

@@ -469,6 +469,24 @@ public MarshalAsDescriptor ParseMarshalAsDescriptor()
                         }
                     }
                     break;
+                case NativeTypeKind.CustomMarshaler:
+                    {
+                        // There's nobody to consume CustomMarshaller, so let's just parse the data
+                        // to avoid asserting later.
+
+                        // Read typelib guid
+                        _reader.ReadSerializedString();
+
+                        // Read native type name
+                        _reader.ReadSerializedString();
+
+                        // Read managed marshaler name
+                        _reader.ReadSerializedString();
+
+                        // Read cookie
+                        _reader.ReadSerializedString();
+                    }
+                    break;
                 default:
                     break;
             }

src/coreclr/src/tools/crossgen2/Common/TypeSystem/IL/Stubs/PInvokeILCodeStreams.cs

@@ -12,24 +12,28 @@ internal sealed class PInvokeILCodeStreams
         public ILCodeStream CallsiteSetupCodeStream { get; }
         public ILCodeStream ReturnValueMarshallingCodeStream { get; }
         public ILCodeStream UnmarshallingCodestream { get; }
+        public ILCodeStream CleanupCodeStream { get; }
         public PInvokeILCodeStreams()
         {
             Emitter = new ILEmitter();
 
-            // We have 4 code streams:
-            // - _marshallingCodeStream is used to convert each argument into a native type and 
+            // We have these code streams:
+            // - FunctionPointerLoadStream is used to load the function pointer to call
+            // - MarshallingCodeStream is used to convert each argument into a native type and
             // store that into the local
-            // - callsiteSetupCodeStream is used to used to load each previously generated local
+            // - CallsiteSetupCodeStream is used to used to load each previously generated local
             // and call the actual target native method.
-            // - _returnValueMarshallingCodeStream is used to convert the native return value 
+            // - ReturnValueMarshallingCodeStream is used to convert the native return value 
             // to managed one.
-            // - _unmarshallingCodestream is used to propagate [out] native arguments values to 
+            // - UnmarshallingCodestream is used to propagate [out] native arguments values to 
             // managed ones.
+            // - CleanupCodestream is used to perform a guaranteed cleanup
             FunctionPointerLoadStream = Emitter.NewCodeStream();
             MarshallingCodeStream = Emitter.NewCodeStream();
             CallsiteSetupCodeStream = Emitter.NewCodeStream();
             ReturnValueMarshallingCodeStream = Emitter.NewCodeStream();
             UnmarshallingCodestream = Emitter.NewCodeStream();
+            CleanupCodeStream = Emitter.NewCodeStream();
         }
 
         public PInvokeILCodeStreams(ILEmitter emitter, ILCodeStream codeStream)
@@ -38,4 +42,4 @@ public PInvokeILCodeStreams(ILEmitter emitter, ILCodeStream codeStream)
             MarshallingCodeStream = codeStream;
         }
     }
-}
\ No newline at end of file
+}

src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/IL/Marshaller.cs

@@ -433,16 +433,21 @@ public static bool IsMarshallingRequired(MethodDesc targetMethod)
         {
             Debug.Assert(targetMethod.IsPInvoke);
 
-            if (targetMethod.GetPInvokeMethodMetadata().Flags.SetLastError)
+            PInvokeFlags flags = targetMethod.GetPInvokeMethodMetadata().Flags;
+
+            if (flags.SetLastError)
                 return true;
 
-            var marshallers = GetMarshallersForMethod(targetMethod);
+            if (!flags.PreserveSig)
+                return true;
 
+            var marshallers = GetMarshallersForMethod(targetMethod);
             for (int i = 0; i < marshallers.Length; i++)
             {
                 if (marshallers[i].IsMarshallingRequired())
                     return true;
             }
+
             return false;
         }
 
@@ -769,9 +774,7 @@ protected virtual void EmitMarshalArgumentManagedToNative()
                 }
             }
 
-            // TODO This should be in finally block
-            // https://github.com/dotnet/corert/issues/6075
-            EmitCleanupManaged(_ilCodeStreams.UnmarshallingCodestream);
+            EmitCleanupManaged(_ilCodeStreams.CleanupCodeStream);
         }
 
         /// <summary>
@@ -1396,8 +1399,7 @@ protected override void EmitCleanupManaged(ILCodeStream codeStream)
             }
 
             LoadNativeValue(codeStream);
-            codeStream.Emit(ILOpcode.call, emitter.NewToken(
-                                Context.GetHelperEntryPoint("InteropHelpers", "CoTaskMemFree")));
+            codeStream.Emit(ILOpcode.call, emitter.NewToken(InteropTypes.GetMarshal(Context).GetKnownMethod("FreeCoTaskMem", null)));
             codeStream.EmitLabel(lNullArray);
         }
     }
@@ -1742,28 +1744,27 @@ class SafeHandleMarshaller : Marshaller
         private void AllocSafeHandle(ILCodeStream codeStream)
         {
             var ctor = ManagedType.GetParameterlessConstructor();
-            if (ctor == null)
+            if (ctor == null || ((MetadataType)ManagedType).IsAbstract)
             {
+#if READYTORUN
+                // Let the runtime generate the proper MissingMemberException for this.
+                throw new NotSupportedException();
+#else
                 var emitter = _ilCodeStreams.Emitter;
 
                 MethodSignature ctorSignature = new MethodSignature(0, 0, Context.GetWellKnownType(WellKnownType.Void),
                       new TypeDesc[] {
-#if !READYTORUN
                           Context.GetWellKnownType(WellKnownType.String)
-#endif
                       });
                 MethodDesc exceptionCtor = InteropTypes.GetMissingMemberException(Context).GetKnownMethod(".ctor", ctorSignature);
 
-#if !READYTORUN // In ReadyToRun we cannot make new string literals out of thin air
                 string name = ((MetadataType)ManagedType).Name;
                 codeStream.Emit(ILOpcode.ldstr, emitter.NewToken(String.Format("'{0}' does not have a default constructor. Subclasses of SafeHandle must have a default constructor to support marshaling a Windows HANDLE into managed code.", name)));
-#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;
+#endif
             }
 
             codeStream.Emit(ILOpcode.newobj, _ilCodeStreams.Emitter.NewToken(ctor));
@@ -1794,6 +1795,7 @@ protected override void EmitMarshalArgumentManagedToNative()
             ILCodeStream marshallingCodeStream = _ilCodeStreams.MarshallingCodeStream;
             ILCodeStream callsiteCodeStream = _ilCodeStreams.CallsiteSetupCodeStream;
             ILCodeStream unmarshallingCodeStream = _ilCodeStreams.UnmarshallingCodestream;
+            ILCodeStream cleanupCodeStream = _ilCodeStreams.CleanupCodeStream;
 
             SetupArguments();
 
@@ -1823,12 +1825,14 @@ protected override void EmitMarshalArgumentManagedToNative()
                         new MethodSignature(0, 0, Context.GetWellKnownType(WellKnownType.IntPtr), TypeDesc.EmptyTypes))));
                 StoreNativeValue(marshallingCodeStream);
 
-                // TODO: This should be inside finally block and only executed if the handle was addrefed
-                // https://github.com/dotnet/corert/issues/6075
-                LoadManagedValue(unmarshallingCodeStream);
-                unmarshallingCodeStream.Emit(ILOpcode.call, emitter.NewToken(
-                    safeHandleType.GetKnownMethod("DangerousRelease",
+                ILCodeLabel lNotAddrefed = emitter.NewCodeLabel();
+                cleanupCodeStream.EmitLdLoc(vAddRefed);
+                cleanupCodeStream.Emit(ILOpcode.brfalse, lNotAddrefed);
+                LoadManagedValue(cleanupCodeStream);
+                cleanupCodeStream.Emit(ILOpcode.call, emitter.NewToken(
+                    safeHandleType.GetKnownMethod("DangerousRelease", 
                         new MethodSignature(0, 0, Context.GetWellKnownType(WellKnownType.Void), TypeDesc.EmptyTypes))));
+                cleanupCodeStream.EmitLabel(lNotAddrefed);
             }
 
             if (Out && IsManagedByRef)
@@ -1852,23 +1856,23 @@ protected override void EmitMarshalArgumentManagedToNative()
                     LoadNativeValue(marshallingCodeStream);
                     marshallingCodeStream.EmitStLoc(vOriginalValue);
 
-                    unmarshallingCodeStream.EmitLdLoc(vOriginalValue);
-                    LoadNativeValue(unmarshallingCodeStream);
-                    unmarshallingCodeStream.Emit(ILOpcode.beq, lSkipPropagation);
+                    cleanupCodeStream.EmitLdLoc(vOriginalValue);
+                    LoadNativeValue(cleanupCodeStream);
+                    cleanupCodeStream.Emit(ILOpcode.beq, lSkipPropagation);
                 }
 
-                unmarshallingCodeStream.EmitLdLoc(vSafeHandle);
-                LoadNativeValue(unmarshallingCodeStream);
-                unmarshallingCodeStream.Emit(ILOpcode.call, emitter.NewToken(
+                cleanupCodeStream.EmitLdLoc(vSafeHandle);
+                LoadNativeValue(cleanupCodeStream);
+                cleanupCodeStream.Emit(ILOpcode.call, emitter.NewToken(
                     safeHandleType.GetKnownMethod("SetHandle",
                         new MethodSignature(0, 0, Context.GetWellKnownType(WellKnownType.Void),
                             new TypeDesc[] { Context.GetWellKnownType(WellKnownType.IntPtr) }))));
 
-                unmarshallingCodeStream.EmitLdArg(Index - 1);
-                unmarshallingCodeStream.EmitLdLoc(vSafeHandle);
-                unmarshallingCodeStream.EmitStInd(ManagedType);
+                cleanupCodeStream.EmitLdArg(Index - 1);
+                cleanupCodeStream.EmitLdLoc(vSafeHandle);
+                cleanupCodeStream.EmitStInd(ManagedType);
 
-                unmarshallingCodeStream.EmitLabel(lSkipPropagation);
+                cleanupCodeStream.EmitLabel(lSkipPropagation);
             }
 
             LoadNativeArg(callsiteCodeStream);

src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/MarshalAsDescriptor.cs

@@ -35,6 +35,7 @@ public enum NativeTypeKind : byte
         AsAny = 0x28,
         Array = 0x2a,
         LPStruct = 0x2b,    // This is not  defined in Ecma-335(II.23.4)
+        CustomMarshaler = 0x2c,
         LPUTF8Str = 0x30,
         Default = 0x50,      // This is the default value
         Variant = 0x51,

src/coreclr/src/tools/crossgen2/Common/TypeSystem/Interop/MethodDesc.Interop.cs

@@ -78,8 +78,9 @@ public ParameterMetadata(int index, ParameterMetadataAttributes attributes, Mars
     }
 
     [Flags]
-    public enum PInvokeAttributes : short
+    public enum PInvokeAttributes
     {
+        // These should match System.Reflection.MethodImportAttributes
         None = 0,
         ExactSpelling = 1,
         CharSetAnsi = 2,
@@ -98,7 +99,12 @@ public enum PInvokeAttributes : short
         CallingConventionMask = 1792,
         ThrowOnUnmappableCharEnable = 4096,
         ThrowOnUnmappableCharDisable = 8192,
-        ThrowOnUnmappableCharMask = 12288
+        ThrowOnUnmappableCharMask = 12288,
+
+        // Not actually part of MethodImportAttributes.
+        // MethodImportAttributes is limited to `short`. This enum is based on int
+        // and we have 16 spare bytes.
+        PreserveSig = 0x10000,
     }
 
     public struct PInvokeFlags : IEquatable<PInvokeFlags>, IComparable<PInvokeFlags>
@@ -280,6 +286,25 @@ public bool ThrowOnUnmappableChar
             }
         }
 
+        public bool PreserveSig
+        {
+            get
+            {
+                return (_attributes & PInvokeAttributes.PreserveSig) != 0;
+            }
+            set
+            {
+                if (value)
+                {
+                    _attributes |= PInvokeAttributes.PreserveSig;
+                }
+                else
+                {
+                    _attributes &= ~PInvokeAttributes.PreserveSig;
+                }
+            }
+        }
+
         public int CompareTo(PInvokeFlags other)
         {
             return Attributes.CompareTo(other.Attributes);

src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/IL/Stubs/PInvokeILEmitter.cs

@@ -76,9 +76,20 @@ private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams)
 
         private MethodIL EmitIL()
         {
+            if (!_importMetadata.Flags.PreserveSig)
+                throw new NotSupportedException();
+
             PInvokeILCodeStreams pInvokeILCodeStreams = new PInvokeILCodeStreams();
             ILEmitter emitter = pInvokeILCodeStreams.Emitter;
+            ILCodeStream marshallingCodestream = pInvokeILCodeStreams.MarshallingCodeStream;
             ILCodeStream unmarshallingCodestream = pInvokeILCodeStreams.UnmarshallingCodestream;
+            ILCodeStream cleanupCodestream = pInvokeILCodeStreams.CleanupCodeStream;
+
+            // Marshalling is wrapped in a finally block to guarantee cleanup
+            ILExceptionRegionBuilder tryFinally = emitter.NewFinallyRegion();
+
+            marshallingCodestream.BeginTry(tryFinally);
+            cleanupCodestream.BeginHandler(tryFinally);
 
             // Marshal the arguments
             for (int i = 0; i < _marshallers.Length; i++)
@@ -88,8 +99,17 @@ private MethodIL EmitIL()
 
             EmitPInvokeCall(pInvokeILCodeStreams);
 
-            _marshallers[0].LoadReturnValue(unmarshallingCodestream);
-            unmarshallingCodestream.Emit(ILOpcode.ret);
+            ILCodeLabel lReturn = emitter.NewCodeLabel();
+            unmarshallingCodestream.Emit(ILOpcode.leave, lReturn);
+            unmarshallingCodestream.EndTry(tryFinally);
+
+            cleanupCodestream.Emit(ILOpcode.endfinally);
+            cleanupCodestream.EndHandler(tryFinally);
+
+            cleanupCodestream.EmitLabel(lReturn);
+
+            _marshallers[0].LoadReturnValue(cleanupCodestream);
+            cleanupCodestream.Emit(ILOpcode.ret);
 
             return new PInvokeILStubMethodIL((ILStubMethodIL)emitter.Link(_targetMethod));
         }