From: Sergey Andreenko Date: Fri, 6 Nov 2020 05:32:40 +0000 (-1000) Subject: Add an implicit argument coercion check. (#43386) X-Git-Tag: submit/tizen/20210909.063632~4738 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d7c4602d05939fb262c1d6230e970277f3a3591d;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Add an implicit argument coercion check. (#43386) * 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. --- diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/StubHelpers.cs b/src/coreclr/src/System.Private.CoreLib/src/System/StubHelpers.cs index 1c4a564..a12b3f2 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/StubHelpers.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/StubHelpers.cs @@ -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); diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index 49701b3..0770b28 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -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); /* diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 0bc54d5..d5b909d 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -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 +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 + // 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.) diff --git a/src/coreclr/src/vm/corelib.h b/src/coreclr/src/vm/corelib.h index 5b6e7fe..21c8a6c 100644 --- a/src/coreclr/src/vm/corelib.h +++ b/src/coreclr/src/vm/corelib.h @@ -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) diff --git a/src/coreclr/src/vm/metasig.h b/src/coreclr/src/vm/metasig.h index ca09519..03b2e45 100644 --- a/src/coreclr/src/vm/metasig.h +++ b/src/coreclr/src/vm/metasig.h @@ -174,7 +174,7 @@ 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)) diff --git a/src/tests/JIT/Directed/coverage/importer/Desktop/byrefsubbyref1.il b/src/tests/JIT/Directed/coverage/importer/Desktop/byrefsubbyref1.il index cdf966c..2e579ca 100644 --- a/src/tests/JIT/Directed/coverage/importer/Desktop/byrefsubbyref1.il +++ b/src/tests/JIT/Directed/coverage/importer/Desktop/byrefsubbyref1.il @@ -15,21 +15,21 @@ .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 diff --git a/src/tests/JIT/Directed/coverage/importer/byrefsubbyref1.il b/src/tests/JIT/Directed/coverage/importer/byrefsubbyref1.il index 1023f8e..eb9587c 100644 --- a/src/tests/JIT/Directed/coverage/importer/byrefsubbyref1.il +++ b/src/tests/JIT/Directed/coverage/importer/byrefsubbyref1.il @@ -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 diff --git a/src/tests/JIT/Regression/JitBlue/GitHub_18295/GitHub_18295.il b/src/tests/JIT/Regression/JitBlue/GitHub_18295/GitHub_18295.il index 8310732..05c5dd9 100644 --- a/src/tests/JIT/Regression/JitBlue/GitHub_18295/GitHub_18295.il +++ b/src/tests/JIT/Regression/JitBlue/GitHub_18295/GitHub_18295.il @@ -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 diff --git a/src/tests/JIT/opt/Inline/regression/mismatch32/mismatch32.il b/src/tests/JIT/opt/Inline/regression/mismatch32/mismatch32.il index b338de2..9cc64c3 100644 --- a/src/tests/JIT/opt/Inline/regression/mismatch32/mismatch32.il +++ b/src/tests/JIT/opt/Inline/regression/mismatch32/mismatch32.il @@ -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 @@ -35,21 +30,38 @@ 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 } diff --git a/src/tests/JIT/opt/Inline/regression/mismatch64/mismatch64.il b/src/tests/JIT/opt/Inline/regression/mismatch64/mismatch64.il index 8fd72c2..53b2c0e 100644 --- a/src/tests/JIT/opt/Inline/regression/mismatch64/mismatch64.il +++ b/src/tests/JIT/opt/Inline/regression/mismatch64/mismatch64.il @@ -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 @@ -29,27 +24,44 @@ .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 }