diff --git a/docs/design/specs/Ecma-335-Augments.md b/docs/design/specs/Ecma-335-Augments.md index 9220361e8dc537..a26490f87b71c2 100644 --- a/docs/design/specs/Ecma-335-Augments.md +++ b/docs/design/specs/Ecma-335-Augments.md @@ -25,6 +25,7 @@ This is a list of additions and edits to be made in ECMA-335 specifications. It - [API documentation](#api-documentation) - [Debug Interchange Format](#debug-interchange-format) - [Extended layout](#extended-layout) +- [Implicit argument coercion rules](#implicit-argument-coercion-rules) ## Signatures @@ -1191,3 +1192,6 @@ In section II.23.1.15, the following row is added to the table: |-----|------|------| | `ExtendedLayout` | 0x00000018 | Layout is supplied by a `System.Runtime.InteropServices.ExtendedLayoutAttribute` custom attribute | +## Implict argument coercion rules + +Implicit argument coercion as defined in section III.1.6 does not match with existing practice in CLR runtimes. Notably, implicit argument coercion of an `int32` on the IL evaluation stack to a `native unsigned int` is a sign extending operation, not a zero-extending operation. \ No newline at end of file diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 95b75b13c2d51d..d23c5ec127aaf6 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2403,7 +2403,7 @@ void InterpCompiler::EmitStoreVar(int32_t var) AddIns(INTOP_LOAD_FRAMEVAR); PushInterpType(InterpTypeI, NULL); m_pLastNewIns->SetDVar(m_pStackPointer[-1].var); - EmitStind(interpType, m_pVars[var].clsHnd, m_pVars[var].offset, true /* reverseSVarOrder */); + EmitStind(interpType, m_pVars[var].clsHnd, m_pVars[var].offset, true /* reverseSVarOrder */, false /* enableImplicitArgConversionRules */); return; } @@ -3848,10 +3848,8 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re { #ifdef TARGET_64BIT case CORINFO_TYPE_NATIVEINT: - EmitConv(m_pStackPointer + iCurrentStackArg, StackTypeI8, INTOP_CONV_I8_I4); - break; case CORINFO_TYPE_NATIVEUINT: - EmitConv(m_pStackPointer + iCurrentStackArg, StackTypeI8, INTOP_CONV_U8_U4); + EmitConv(m_pStackPointer + iCurrentStackArg, StackTypeI8, INTOP_CONV_I8_I4); // This subtly differs from the published ECMA spec, and is what is defined in the Ecma-335-Augments.md document break; #endif // TARGET_64BIT case CORINFO_TYPE_BYTE: @@ -4306,11 +4304,11 @@ void InterpCompiler::EmitLdind(InterpType interpType, CORINFO_CLASS_HANDLE clsHn m_pLastNewIns->SetDVar(m_pStackPointer[-1].var); } -void InterpCompiler::EmitStind(InterpType interpType, CORINFO_CLASS_HANDLE clsHnd, int32_t offset, bool reverseSVarOrder) +void InterpCompiler::EmitStind(InterpType interpType, CORINFO_CLASS_HANDLE clsHnd, int32_t offset, bool reverseSVarOrder, bool enableImplicitArgConversionRules) { int stackIndexValue; int stackIndexAddress; - if (reverseSVarOrder) + if (!reverseSVarOrder) { stackIndexAddress = -2; stackIndexValue = -1; @@ -4329,6 +4327,10 @@ void InterpCompiler::EmitStind(InterpType interpType, CORINFO_CLASS_HANDLE clsHn { EmitConv(m_pStackPointer + stackIndexValue, StackTypeR8, INTOP_CONV_R8_R4); } + else if (enableImplicitArgConversionRules && interpType == InterpTypeI8 && m_pStackPointer[stackIndexValue].type == StackTypeI4) // This subtly differs from the published ECMA spec for section III.1.6, and is what is defined in the Ecma-335-Augments.md document + { + EmitConv(m_pStackPointer + stackIndexValue, StackTypeI8, INTOP_CONV_I8_I4); + } // stack contains address and then the value to be stored // or in the reverse order if the flag is set @@ -4352,7 +4354,7 @@ void InterpCompiler::EmitStind(InterpType interpType, CORINFO_CLASS_HANDLE clsHn m_pLastNewIns->data[0] = offset; - m_pLastNewIns->SetSVars2(m_pStackPointer[stackIndexValue].var, m_pStackPointer[stackIndexAddress].var); + m_pLastNewIns->SetSVars2(m_pStackPointer[stackIndexAddress].var, m_pStackPointer[stackIndexValue].var); m_pStackPointer -= 2; } @@ -4525,7 +4527,7 @@ void InterpCompiler::EmitStaticFieldAccess(InterpType interpFieldType, CORINFO_F if (isLoad) EmitLdind(interpFieldType, pFieldInfo->structType, 0); else - EmitStind(interpFieldType, pFieldInfo->structType, 0, true); + EmitStind(interpFieldType, pFieldInfo->structType, 0, true, true /* enableImplicitArgConversionRules */); break; } } @@ -5065,7 +5067,7 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) } else { - EmitStind(interpType, resolvedToken.hClass, 0, false); + EmitStind(interpType, resolvedToken.hClass, 0, false, false /* enableImplicitArgConversionRules */); } m_ip += 5; break; @@ -5078,7 +5080,7 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) ResolveToken(getU4LittleEndian(m_ip + 1), CORINFO_TOKENKIND_Class, &resolvedToken); InterpType interpType = GetInterpType(m_compHnd->asCorInfoType(resolvedToken.hClass)); EmitLdind(interpType, resolvedToken.hClass, 0); - EmitStind(interpType, resolvedToken.hClass, 0, false); + EmitStind(interpType, resolvedToken.hClass, 0, false, false /* enableImplicitArgConversionRules */); m_ip += 5; break; } @@ -6366,7 +6368,7 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) else { assert(fieldInfo.fieldAccessor == CORINFO_FIELD_INSTANCE); - EmitStind(interpFieldType, fieldInfo.structType, fieldInfo.offset, false); + EmitStind(interpFieldType, fieldInfo.structType, fieldInfo.offset, false, true /* enableImplicitArgConversionRules */); } m_ip += 5; @@ -6534,7 +6536,7 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) AddIns(INTOP_MEMBAR); volatile_ = false; } - EmitStind(interpType, NULL, 0, false); + EmitStind(interpType, NULL, 0, false, false /* enableImplicitArgConversionRules */); m_ip++; break; } diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index 3f8526af7b2b02..db4d667b789fd7 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -737,7 +737,7 @@ class InterpCompiler void EmitCalli(bool isTailCall, void* calliCookie, int callIFunctionPointerVar, CORINFO_SIG_INFO* callSiteSig); bool EmitNamedIntrinsicCall(NamedIntrinsic ni, bool nonVirtualCall, CORINFO_CLASS_HANDLE clsHnd, CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO sig); void EmitLdind(InterpType type, CORINFO_CLASS_HANDLE clsHnd, int32_t offset); - void EmitStind(InterpType type, CORINFO_CLASS_HANDLE clsHnd, int32_t offset, bool reverseSVarOrder); + void EmitStind(InterpType type, CORINFO_CLASS_HANDLE clsHnd, int32_t offset, bool reverseSVarOrder, bool enableImplicitArgConversionRules); void EmitLdelem(int32_t opcode, InterpType type); void EmitStelem(InterpType type); void EmitStaticFieldAddress(CORINFO_FIELD_INFO *pFieldInfo, CORINFO_RESOLVED_TOKEN *pResolvedToken); diff --git a/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion.il b/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion.il new file mode 100644 index 00000000000000..01a49c3cde7f0a --- /dev/null +++ b/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion.il @@ -0,0 +1,169 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern mscorlib { } +.assembly extern System.Console +{ + .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) + .ver 4:0:0:0 +} +.assembly ASSEMBLY_NAME +{ +} +.assembly extern xunit.core {} +.assembly extern Microsoft.DotNet.XUnitExtensions { .publickeytoken = (31 BF 38 56 AD 36 4E 35 ) } +.namespace JitTest_implicit_arg_coercion +{ + .class public auto ansi beforefieldinit TestClass + extends [mscorlib]System.Object + { + .field private static native int s_data + .field private static native uint s_udata + + .method public hidebysig static native int TakeNativeIntArg(native int) cil managed + { + ldarg.0 + ret + } + + .method public hidebysig static native uint TakeNativeUIntArg(native uint) cil managed + { + ldarg.0 + ret + } + + .method public hidebysig static int32 TakeU2Arg(uint16) cil managed + { + ldarg.0 + ret + } + + .method public hidebysig static int32 TakeU1Arg(uint8) cil managed + { + ldarg.0 + ret + } + + .method public hidebysig static int32 + Main() cil managed + { + .custom instance void [xunit.core]Xunit.FactAttribute::.ctor() = ( + 01 00 00 00 + ) + .custom instance void [Microsoft.DotNet.XUnitExtensions]Xunit.SkipOnMonoAttribute::.ctor(string, valuetype [Microsoft.DotNet.XUnitExtensions]Xunit.TestPlatforms) = { + string('Handling of implicit arg coercion rules III.1.6 differs between Mono and CoreCLR, and this tests the CoreCLR behavior, and what we have put in the augments') + int32(0xFFFFFFFF) // Any + } + + .entrypoint + // Code size 36 (0x24) + .maxstack 2 + ldc.i4.m1 + stsfld native int JitTest_implicit_arg_coercion.TestClass::s_data + ldc.i4.m1 + stsfld native uint JitTest_implicit_arg_coercion.TestClass::s_udata + + ldsfld native int JitTest_implicit_arg_coercion.TestClass::s_data + conv.u8 + box [mscorlib]System.UInt64 + call void [System.Console]System.Console::WriteLine(object) + + ldsfld native uint JitTest_implicit_arg_coercion.TestClass::s_udata + conv.u8 + box [mscorlib]System.UInt64 + call void [System.Console]System.Console::WriteLine(object) + + ldc.i4.m1 + conv.i + box [mscorlib]System.UInt64 + call void [System.Console]System.Console::WriteLine(object) + + ldc.i4.m1 + conv.u + box [mscorlib]System.UInt64 + call void [System.Console]System.Console::WriteLine(object) + + ldc.i4.m1 + conv.i + ldsfld native int JitTest_implicit_arg_coercion.TestClass::s_data + beq IL_PassNativeInt + ldstr "Failed conversion of i4 into i for stsfld" + call void [System.Console]System.Console::WriteLine(string) + ldc.i4.s 101 + ret + + IL_PassNativeInt: + ldc.i4.m1 + conv.i // The published ECMA 335 spec specifies that we should use the implicit argument coercion rules in section III. + ldsfld native uint JitTest_implicit_arg_coercion.TestClass::s_udata + beq IL_PassNativeUInt + ldstr "Failed conversion of i4 into u for stsfld" + call void [System.Console]System.Console::WriteLine(string) + ldc.i4.s 102 + ret + IL_PassNativeUInt: + + ldc.i4.m1 + call native int [ASSEMBLY_NAME]JitTest_implicit_arg_coercion.TestClass::TakeNativeIntArg(native int) + ldc.i4.m1 + conv.i + beq IL_PassNativeIntCall + ldstr "Failed conversion of i4 into i for argument call" + call void [System.Console]System.Console::WriteLine(string) + ldc.i4.s 103 + ret + IL_PassNativeIntCall: + + ldc.i4.m1 + call native uint [ASSEMBLY_NAME]JitTest_implicit_arg_coercion.TestClass::TakeNativeUIntArg(native uint) + ldc.i4.m1 + conv.i + beq IL_PassNativeUIntCall + ldstr "Failed conversion of i4 into u for argument call" + call void [System.Console]System.Console::WriteLine(string) + ldc.i4.s 104 + ret + IL_PassNativeUIntCall: + + ldc.i4.m1 + call int32 [ASSEMBLY_NAME]JitTest_implicit_arg_coercion.TestClass::TakeU2Arg(uint16) + ldc.i4 0xFFFF + beq IL_PassU2Call + ldstr "Failed conversion of i4 into u2 for argument call" + call void [System.Console]System.Console::WriteLine(string) + ldc.i4.s 105 + ret + IL_PassU2Call: + + ldc.i4.m1 + call int32 [ASSEMBLY_NAME]JitTest_implicit_arg_coercion.TestClass::TakeU1Arg(uint8) + ldc.i4 0xFF + beq IL_PassU1Call + ldstr "Failed conversion of i4 into u1 for argument call" + call void [System.Console]System.Console::WriteLine(string) + ldc.i4.s 105 + ret + IL_PassU1Call: + + ldstr "Passed => 100" + IL_001c: call void [System.Console]System.Console::WriteLine(string) + IL_0021: ldc.i4.s 100 + IL_0023: ret + } // end of method TestClass::Main + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [mscorlib]System.Object::.ctor() + IL_0006: ret + } // end of method TestClass::.ctor + + } // end of class TestClass + +} // end of namespace JitTest_implicit_arg_coercion + +//*********** DISASSEMBLY COMPLETE *********************** +// WARNING: Created Win32 resource file ldnull.res \ No newline at end of file diff --git a/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_d.il b/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_d.il new file mode 100644 index 00000000000000..e9d46a13cbd819 --- /dev/null +++ b/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_d.il @@ -0,0 +1 @@ +#define ASSEMBLY_NAME "implicit_arg_coercion_d" diff --git a/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_d.ilproj b/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_d.ilproj new file mode 100644 index 00000000000000..11ecd2a0d9d3e5 --- /dev/null +++ b/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_d.ilproj @@ -0,0 +1,12 @@ + + + 0 + + + Full + + + + + + diff --git a/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_r.il b/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_r.il new file mode 100644 index 00000000000000..93dc53cf370633 --- /dev/null +++ b/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_r.il @@ -0,0 +1 @@ +#define ASSEMBLY_NAME "implicit_arg_coercion_r" diff --git a/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_r.ilproj b/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_r.ilproj new file mode 100644 index 00000000000000..3bb4dcb91f3de9 --- /dev/null +++ b/src/tests/JIT/Methodical/casts/coverage/implicit_arg_coercion_r.ilproj @@ -0,0 +1,12 @@ + + + 0 + + + PdbOnly + + + + + +