Add an implicit argument coercion check. (#43386)
authorSergey Andreenko <seandree@microsoft.com>
Fri, 6 Nov 2020 05:32:40 +0000 (19:32 -1000)
committerGitHub <noreply@github.com>
Fri, 6 Nov 2020 05:32:40 +0000 (21:32 -0800)
* Add `impCheckImplicitArgumentCoercion`.

* Fix tests with type mismatch.

* Try to fix VM signature.

* Allow to pass byref as native int.

* another fix.

* Fix another IL test.

src/coreclr/src/System.Private.CoreLib/src/System/StubHelpers.cs
src/coreclr/src/jit/compiler.h
src/coreclr/src/jit/importer.cpp
src/coreclr/src/vm/corelib.h
src/coreclr/src/vm/metasig.h
src/tests/JIT/Directed/coverage/importer/Desktop/byrefsubbyref1.il
src/tests/JIT/Directed/coverage/importer/byrefsubbyref1.il
src/tests/JIT/Regression/JitBlue/GitHub_18295/GitHub_18295.il
src/tests/JIT/opt/Inline/regression/mismatch32/mismatch32.il
src/tests/JIT/opt/Inline/regression/mismatch64/mismatch64.il

index 1c4a564..a12b3f2 100644 (file)
@@ -584,7 +584,7 @@ namespace System.StubHelpers
         internal static extern IntPtr ConvertToNative(object objSrc, IntPtr itfMT, IntPtr classMT, int flags);
 
         [MethodImpl(MethodImplOptions.InternalCall)]
-        internal static extern object ConvertToManaged(IntPtr ppUnk, IntPtr itfMT, IntPtr classMT, int flags);
+        internal static extern object ConvertToManaged(ref IntPtr ppUnk, IntPtr itfMT, IntPtr classMT, int flags);
 
         [DllImport(RuntimeHelpers.QCall)]
         internal static extern void ClearNative(IntPtr pUnk);
index 49701b3..0770b28 100644 (file)
@@ -4121,6 +4121,8 @@ private:
 
     GenTreeCall::Use* impPopCallArgs(unsigned count, CORINFO_SIG_INFO* sig, GenTreeCall::Use* prefixArgs = nullptr);
 
+    bool impCheckImplicitArgumentCoercion(var_types sigType, var_types nodeType) const;
+
     GenTreeCall::Use* impPopReverseCallArgs(unsigned count, CORINFO_SIG_INFO* sig, unsigned skipReverseCount = 0);
 
     /*
index 0bc54d5..d5b909d 100644 (file)
@@ -946,20 +946,27 @@ GenTreeCall::Use* Compiler::impPopCallArgs(unsigned count, CORINFO_SIG_INFO* sig
 
             CorInfoType corType = strip(info.compCompHnd->getArgType(sig, argLst, &argClass));
 
+            var_types jitSigType = JITtype2varType(corType);
+
+            if (!impCheckImplicitArgumentCoercion(jitSigType, arg->GetNode()->TypeGet()))
+            {
+                BADCODE("the call argument has a type that can't be implicitly converted to the signature type");
+            }
+
             // insert implied casts (from float to double or double to float)
 
-            if ((corType == CORINFO_TYPE_DOUBLE) && (arg->GetNode()->TypeGet() == TYP_FLOAT))
+            if ((jitSigType == TYP_DOUBLE) && (arg->GetNode()->TypeGet() == TYP_FLOAT))
             {
                 arg->SetNode(gtNewCastNode(TYP_DOUBLE, arg->GetNode(), false, TYP_DOUBLE));
             }
-            else if ((corType == CORINFO_TYPE_FLOAT) && (arg->GetNode()->TypeGet() == TYP_DOUBLE))
+            else if ((jitSigType == TYP_FLOAT) && (arg->GetNode()->TypeGet() == TYP_DOUBLE))
             {
                 arg->SetNode(gtNewCastNode(TYP_FLOAT, arg->GetNode(), false, TYP_FLOAT));
             }
 
             // insert any widening or narrowing casts for backwards compatibility
 
-            arg->SetNode(impImplicitIorI4Cast(arg->GetNode(), JITtype2varType(corType)));
+            arg->SetNode(impImplicitIorI4Cast(arg->GetNode(), jitSigType));
 
             if (corType != CORINFO_TYPE_CLASS && corType != CORINFO_TYPE_BYREF && corType != CORINFO_TYPE_PTR &&
                 corType != CORINFO_TYPE_VAR && (argRealClass = info.compCompHnd->getArgClass(sig, argLst)) != nullptr)
@@ -993,6 +1000,111 @@ GenTreeCall::Use* Compiler::impPopCallArgs(unsigned count, CORINFO_SIG_INFO* sig
     return argList;
 }
 
+static bool TypeIs(var_types type1, var_types type2)
+{
+    return type1 == type2;
+}
+
+// Check if type1 matches any type from the list.
+template <typename... T>
+static bool TypeIs(var_types type1, var_types type2, T... rest)
+{
+    return TypeIs(type1, type2) || TypeIs(type1, rest...);
+}
+
+//------------------------------------------------------------------------
+// impCheckImplicitArgumentCoercion: check that the node's type is compatible with
+//   the signature's type using ECMA implicit argument coercion table.
+//
+// Arguments:
+//    sigType  - the type in the call signature;
+//    nodeType - the node type.
+//
+// Return Value:
+//    true if they are compatible, false otherwise.
+//
+// Notes:
+//   - it is currently allowing byref->long passing, should be fixed in VM;
+//   - it can't check long -> native int case on 64-bit platforms,
+//      so the behavior is different depending on the target bitness.
+//
+bool Compiler::impCheckImplicitArgumentCoercion(var_types sigType, var_types nodeType) const
+{
+    if (sigType == nodeType)
+    {
+        return true;
+    }
+
+    if (TypeIs(sigType, TYP_BOOL, TYP_UBYTE, TYP_BYTE, TYP_USHORT, TYP_SHORT, TYP_UINT, TYP_INT))
+    {
+        if (TypeIs(nodeType, TYP_BOOL, TYP_UBYTE, TYP_BYTE, TYP_USHORT, TYP_SHORT, TYP_UINT, TYP_INT, TYP_I_IMPL))
+        {
+            return true;
+        }
+    }
+    else if (TypeIs(sigType, TYP_ULONG, TYP_LONG))
+    {
+        if (TypeIs(nodeType, TYP_LONG))
+        {
+            return true;
+        }
+    }
+    else if (TypeIs(sigType, TYP_FLOAT, TYP_DOUBLE))
+    {
+        if (TypeIs(nodeType, TYP_FLOAT, TYP_DOUBLE))
+        {
+            return true;
+        }
+    }
+    else if (TypeIs(sigType, TYP_BYREF))
+    {
+        if (TypeIs(nodeType, TYP_I_IMPL))
+        {
+            return true;
+        }
+
+        // This condition tolerates such IL:
+        // ;  V00 this              ref  this class-hnd
+        // ldarg.0
+        // call(byref)
+        if (TypeIs(nodeType, TYP_REF))
+        {
+            return true;
+        }
+    }
+    else if (varTypeIsStruct(sigType))
+    {
+        if (varTypeIsStruct(nodeType))
+        {
+            return true;
+        }
+    }
+
+    // This condition should not be under `else` because `TYP_I_IMPL`
+    // intersects with `TYP_LONG` or `TYP_INT`.
+    if (TypeIs(sigType, TYP_I_IMPL, TYP_U_IMPL))
+    {
+        // Note that it allows `ldc.i8 1; call(nint)` on 64-bit platforms,
+        // but we can't distinguish `nint` from `long` there.
+        if (TypeIs(nodeType, TYP_I_IMPL, TYP_U_IMPL, TYP_INT, TYP_UINT))
+        {
+            return true;
+        }
+
+        // It tolerates IL that ECMA does not allow but that is commonly used.
+        // Example:
+        //   V02 loc1           struct <RTL_OSVERSIONINFOEX, 32>
+        //   ldloca.s     0x2
+        //   call(native int)
+        if (TypeIs(nodeType, TYP_BYREF))
+        {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 /*****************************************************************************
  *
  *  Pop the given number of values from the stack in reverse order (STDCALL/CDECL etc.)
index 5b6e7fe..21c8a6c 100644 (file)
@@ -1061,7 +1061,7 @@ DEFINE_METHOD(OBJECTMARSHALER,      CLEAR_NATIVE,           ClearNative,
 
 DEFINE_CLASS(INTERFACEMARSHALER,    StubHelpers,            InterfaceMarshaler)
 DEFINE_METHOD(INTERFACEMARSHALER,   CONVERT_TO_NATIVE,      ConvertToNative,            SM_Obj_IntPtr_IntPtr_Int_RetIntPtr)
-DEFINE_METHOD(INTERFACEMARSHALER,   CONVERT_TO_MANAGED,     ConvertToManaged,           SM_IntPtr_IntPtr_IntPtr_Int_RetObj)
+DEFINE_METHOD(INTERFACEMARSHALER,   CONVERT_TO_MANAGED,     ConvertToManaged,           SM_RefIntPtr_IntPtr_IntPtr_Int_RetObj)
 DEFINE_METHOD(INTERFACEMARSHALER,   CLEAR_NATIVE,           ClearNative,                SM_IntPtr_RetVoid)
 
 
index ca09519..03b2e45 100644 (file)
 DEFINE_METASIG_T(SM(Int_IntPtr_Obj_RetException, i I j, C(EXCEPTION)))
 DEFINE_METASIG_T(SM(Type_ArrType_IntPtr_int_RetType, C(TYPE) a(C(TYPE)) I i, C(TYPE) ))
 DEFINE_METASIG_T(SM(Type_RetIntPtr, C(TYPE), I))
-DEFINE_METASIG(SM(IntPtr_IntPtr_IntPtr_Int_RetObj, I I I i, j))
+DEFINE_METASIG(SM(RefIntPtr_IntPtr_IntPtr_Int_RetObj, r(I) I I i, j))
 DEFINE_METASIG(SM(Obj_IntPtr_RetIntPtr, j I, I))
 DEFINE_METASIG(SM(Obj_IntPtr_RetObj, j I, j))
 DEFINE_METASIG(SM(Obj_RefIntPtr_RetVoid, j r(I), v))
index cdf966c..2e579ca 100644 (file)
 .class a extends [mscorlib]System.Object
 {
 .field static class ctest S_1
-.method public static int32 byrefsubbyref(class ctest& V_1, class ctest& V_2)
+.method public static native int byrefsubbyref(class ctest& V_1, class ctest& V_2)
 {
 ldarg 0
 ldarg 1
 sub
 ret
 }
-.method public static int32 byrefsubi4(class ctest& V_1, int32 V_2)
+.method public static native int byrefsubi4(class ctest& V_1, int32 V_2)
 {
 ldarg 0
 ldarg 1
 sub
 ret
 }
-.method public static int32 i4subbyref(int32, class ctest& V_2)
+.method public static native int i4subbyref(int32, class ctest& V_2)
 {
 ldarg 0
 ldarg 1
@@ -53,54 +53,54 @@ ret
   IL_0010:
        ldloca V_2
        ldloca V_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        dup
        stloc.2
        call       void [System.Console]System.Console::WriteLine(int32)
 
        ldloca V_2
        ldc.i4 1
-       call int32 a::byrefsubi4(class ctest&, int32)
+       call native int a::byrefsubi4(class ctest&, int32)
        call       void [System.Console]System.Console::WriteLine(int32)
 
        ldc.i4 1
        ldloca V_1
-       call int32 a::i4subbyref(int32, class ctest&)
+       call native int a::i4subbyref(int32, class ctest&)
        call       void [System.Console]System.Console::WriteLine(int32)
 
        newobj     instance void ctest::.ctor()
        stloc.0
        ldloca V_1
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        newobj     instance void ctest::.ctor()
        stloc.0
        ldloca V_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        newobj     instance void ctest::.ctor()
        stsfld class ctest a::S_1
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        newobj     instance void ctest::.ctor()
        stloc.0
        ldloca V_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        newobj     instance void ctest::.ctor()
        stloc.0
        ldloca V_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        newobj     instance void ctest::.ctor()
        stloc.0
        ldloca V_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        call       void [System.Console]System.Console::WriteLine(int32)
 
 ldc.i4 100
index 1023f8e..eb9587c 100644 (file)
@@ -11,7 +11,7 @@
 .class a extends [mscorlib]System.Object
 {
 .field static class ctest S_1
-.method public static int32 byrefsubbyref(class ctest& V_1, class ctest& V_2)
+.method public static native int byrefsubbyref(class ctest& V_1, class ctest& V_2)
 {
 // op1: byref, op2: byref
 ldarg 0
@@ -19,7 +19,7 @@ ldarg 1
 sub
 ret
 }
-.method public static int32 byrefsubi4(class ctest& V_1, int32 V_2)
+.method public static native int byrefsubi4(class ctest& V_1, int32 V_2)
 {
 // op1: byref, op2: int32
 ldarg 0
@@ -27,7 +27,7 @@ ldarg 1
 sub
 ret
 }
-.method public static int32 i4subbyref(int32, class ctest& V_2)
+.method public static native int i4subbyref(int32, class ctest& V_2)
 {
 // op1: int32, op2: byref
 ldarg 0
@@ -41,7 +41,7 @@ ret
   .maxstack  2
   .locals init (class ctest V_1,
            class ctest V_2,
-           int32 V_3)
+           native int V_3)
   IL_0004:  newobj     instance void ctest::.ctor()
   IL_0009:  stloc.0
   IL_000a:  newobj     instance void ctest::.ctor()
@@ -53,7 +53,7 @@ ret
   IL_0010:
        ldloca V_2
        ldloca V_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        dup
        stloc.2
        call       void [System.Console]System.Console::WriteLine(int32)
@@ -61,13 +61,13 @@ ret
   // op1: byref, op2: int
        ldloca V_2
        ldc.i4 1
-       call int32 a::byrefsubi4(class ctest&, int32)
+       call native int a::byrefsubi4(class ctest&, int32)
        call       void [System.Console]System.Console::WriteLine(int32)
 
   // op1: int, op2: byref
        ldc.i4 1
        ldloca V_1
-       call int32 a::i4subbyref(int32, class ctest&)
+       call native int a::i4subbyref(int32, class ctest&)
        call       void [System.Console]System.Console::WriteLine(int32)
 
        // op1: byref, op2: byref
@@ -75,35 +75,35 @@ ret
        stloc.0
        ldloca V_1
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        newobj     instance void ctest::.ctor()
        stloc.0
        ldloca V_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        newobj     instance void ctest::.ctor()
        stsfld class ctest a::S_1
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        newobj     instance void ctest::.ctor()
        stloc.0
        ldloca V_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        newobj     instance void ctest::.ctor()
        stloc.0
        ldloca V_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        ldsflda class ctest a::S_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        newobj     instance void ctest::.ctor()
        stloc.0
        ldloca V_1
-       call int32 a::byrefsubbyref(class ctest&, class ctest&)
+       call native int a::byrefsubbyref(class ctest&, class ctest&)
        call       void [System.Console]System.Console::WriteLine(int32)
 
 ldc.i4 100
index 8310732..05c5dd9 100644 (file)
@@ -47,8 +47,8 @@
         stloc      returnVal
 
         // if (Test(1,1) != 1) goto F1
-        ldc.i4     1
         ldc.i8     1
+        ldc.i4     1
         call       int32 GitHub_18295::Test(int64, int32)
 
         ldc.i4.0
index b338de2..9cc64c3 100644 (file)
@@ -3,18 +3,13 @@
 
 // When the jit considers inlining B it can get itself into
 // trouble because of the type mismatch. This test tries to
-// ensure the jit backs out of the inline successfully.
+// ensure the jit backs out of the inline successfully, then
+// throws exception when calls a method with mismatch, then catches it.
 //
-// By default (when no args are passed) execution avoids
-// the problematic callsite, and the app should run without
-// failing.
 
-.assembly extern mscorlib { }
-.assembly extern System.Console
-{
-  .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A )
-  .ver 4:0:0:0
-}
+.assembly extern mscorlib {}
+.assembly extern System.Console {}
+.assembly extern System.Runtime {}
 .assembly main {}
 .module main.exe
 
    ret
 }
 
-.method public hidebysig static int32 Main(string[] args) cil managed
+.method private static void Test() cil managed
 {
-   .entrypoint
    .locals init (valuetype F v_0)
-   ldarg.0
-   ldlen
-   ldc.i4 1
-   blt DONE
    newobj instance void F::.ctor()
    ldc.i4 33
    call  int32 B(int32, int32)
    pop
+}
 
-DONE:
-
-   ldc.i4 100
-   ret
+.method public hidebysig static int32 Main(string[] args) cil managed
+{
+      .entrypoint
+      .locals init (int32 v_0)
+      // Code size       28 (0x1c)
+      .maxstack  2
+      IL_0000:  nop
+      .try
+      {
+        IL_0005:  call void Test()
+        IL_000c:  leave.s    IL_0015
+      }  // end .try
+      catch [System.Runtime]System.InvalidProgramException 
+      {
+        IL_000e:  pop
+        IL_000f:  nop
+        IL_0010:  ldc.i4.s   100
+        IL_0012:  stloc.0
+        IL_0013:  leave.s    IL_001a
+      }  // end handler
+      IL_0015:  ldc.i4.s   101
+      IL_0017:  stloc.0
+      IL_0018:  br.s       IL_001a
+      IL_001a:  ldloc.0
+      IL_001b:  ret
 }
index 8fd72c2..53b2c0e 100644 (file)
@@ -3,18 +3,13 @@
 
 // When the jit considers inlining B it can get itself into
 // trouble because of the type mismatch. This test tries to
-// ensure the jit backs out of the inline successfully.
+// ensure the jit backs out of the inline successfully, then
+// throws exception when calls a method with mismatch, then catches it.
 //
-// By default (when no args are passed) execution avoids
-// the problematic callsite, and the app should run without
-// failing.
 
-.assembly extern mscorlib { }
-.assembly extern System.Console
-{
-  .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A )
-  .ver 4:0:0:0
-}
+.assembly extern mscorlib {}
+.assembly extern System.Console {}
+.assembly extern System.Runtime {}
 .assembly main {}
 .module main.exe
 
 .field public string A
 }
 
-.method private static int64 B(int64 X, int64 Y)
+.method private static int64 B(int64 X, int64 Y) cil managed
 {
    ldarg.0
    ret
 }
 
-.method public hidebysig static int32 Main(string[] args) cil managed
+.method private static void Test() cil managed
 {
-   .entrypoint
    .locals init (valuetype F v_0)
-   ldarg.0
-   ldlen
-   ldc.i4 1
-   blt DONE
    newobj instance void F::.ctor()
    ldc.i8 44
    call  int64 B(int64, int64)
    pop
+}
 
-DONE:
-
-   ldc.i4 100
-   ret
+.method public hidebysig static int32 Main(string[] args) cil managed
+{
+      .entrypoint
+      .locals init (int32 v_0)
+      // Code size       28 (0x1c)
+      .maxstack  2
+      IL_0000:  nop
+      .try
+      {
+        IL_0005:  call void Test()
+        IL_000c:  leave.s    IL_0015
+      }  // end .try
+      catch [System.Runtime]System.InvalidProgramException 
+      {
+        IL_000e:  pop
+        IL_000f:  nop
+        IL_0010:  ldc.i4.s   100
+        IL_0012:  stloc.0
+        IL_0013:  leave.s    IL_001a
+      }  // end handler
+      IL_0015:  ldc.i4.s   101
+      IL_0017:  stloc.0
+      IL_0018:  br.s       IL_001a
+      IL_001a:  ldloc.0
+      IL_001b:  ret
 }