Skip to content

Commit

Permalink
Add an implicit argument coercion check. (#43386)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
Sergey Andreenko authored Nov 6, 2020
1 parent d51d380 commit d7c4602
Show file tree
Hide file tree
Showing 10 changed files with 219 additions and 81 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/src/System.Private.CoreLib/src/System/StubHelpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ internal static class InterfaceMarshaler
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);
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/src/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4121,6 +4121,8 @@ class Compiler

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);

/*
Expand Down
118 changes: 115 additions & 3 deletions src/coreclr/src/jit/importer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.)
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/vm/corelib.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/vm/metasig.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
32 changes: 16 additions & 16 deletions src/tests/JIT/Directed/coverage/importer/Desktop/byrefsubbyref1.il
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
34 changes: 17 additions & 17 deletions src/tests/JIT/Directed/coverage/importer/byrefsubbyref1.il
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,23 @@
.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
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
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
Expand All @@ -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()
Expand All @@ -53,57 +53,57 @@ 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)

// 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
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
Expand Down
2 changes: 1 addition & 1 deletion src/tests/JIT/Regression/JitBlue/GitHub_18295/GitHub_18295.il
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit d7c4602

Please sign in to comment.