Caller return buffer argument can point to GC heap while DispatchTailCalls expects the return value argument to point to the stack. We should use a temporary stack allocated return buffer to hold the value during the dispatcher call and copy the value back to the caller return buffer after that.
// Add return value arg.
GenTree* retValArg;
- GenTree* retVal = nullptr;
- unsigned int newRetLcl = BAD_VAR_NUM;
+ GenTree* retVal = nullptr;
+ unsigned int newRetLcl = BAD_VAR_NUM;
+ GenTree* copyToRetBufNode = nullptr;
- // Use existing retbuf if there is one.
if (origCall->HasRetBufArg())
{
JITDUMP("Transferring retbuf\n");
GenTree* retBufArg = origCall->gtCallArgs->GetNode();
- assert((info.compRetBuffArg != BAD_VAR_NUM) && retBufArg->OperIsLocal() &&
- (retBufArg->AsLclVarCommon()->GetLclNum() == info.compRetBuffArg));
- retValArg = retBufArg;
+ assert(info.compRetBuffArg != BAD_VAR_NUM);
+ assert(retBufArg->OperIsLocal());
+ assert(retBufArg->AsLclVarCommon()->GetLclNum() == info.compRetBuffArg);
+
+ if (info.compRetBuffDefStack)
+ {
+ // Use existing retbuf.
+ retValArg = retBufArg;
+ }
+ else
+ {
+ // Caller return buffer argument retBufArg can point to GC heap while the dispatcher expects
+ // the return value argument retValArg to point to the stack.
+ // We use a temporary stack allocated return buffer to hold the value during the dispatcher call
+ // and copy the value back to the caller return buffer after that.
+ unsigned int tmpRetBufNum = lvaGrabTemp(true DEBUGARG("substitute local for return buffer"));
+
+ constexpr bool unsafeValueClsCheck = false;
+ lvaSetStruct(tmpRetBufNum, origCall->gtRetClsHnd, unsafeValueClsCheck);
+ lvaSetVarAddrExposed(tmpRetBufNum);
+
+ var_types tmpRetBufType = lvaGetDesc(tmpRetBufNum)->TypeGet();
+
+ retValArg = gtNewOperNode(GT_ADDR, TYP_I_IMPL, gtNewLclvNode(tmpRetBufNum, tmpRetBufType));
+
+ var_types callerRetBufType = lvaGetDesc(info.compRetBuffArg)->TypeGet();
+
+ GenTree* dstAddr = gtNewLclvNode(info.compRetBuffArg, callerRetBufType);
+ GenTree* dst = gtNewObjNode(info.compMethodInfo->args.retTypeClass, dstAddr);
+ GenTree* src = gtNewLclvNode(tmpRetBufNum, tmpRetBufType);
+
+ constexpr bool isVolatile = false;
+ constexpr bool isCopyBlock = true;
+ copyToRetBufNode = gtNewBlkOpNode(dst, src, isVolatile, isCopyBlock);
+ }
+
if (origCall->gtType != TYP_VOID)
{
- retVal = gtClone(retValArg);
+ retVal = gtClone(retBufArg);
}
}
else if (origCall->gtType != TYP_VOID)
GenTree* retAddrSlot = gtNewOperNode(GT_ADDR, TYP_I_IMPL, gtNewLclvNode(lvaRetAddrVar, TYP_I_IMPL));
callDispatcherNode->gtCallArgs = gtPrependNewCallArg(retAddrSlot, callDispatcherNode->gtCallArgs);
+ GenTree* finalTree = callDispatcherNode;
+
+ if (copyToRetBufNode != nullptr)
+ {
+ finalTree = gtNewOperNode(GT_COMMA, TYP_VOID, callDispatcherNode, copyToRetBufNode);
+ }
+
if (origCall->gtType == TYP_VOID)
{
- return callDispatcherNode;
+ return finalTree;
}
assert(retVal != nullptr);
- GenTree* comma = gtNewOperNode(GT_COMMA, origCall->TypeGet(), callDispatcherNode, retVal);
+ finalTree = gtNewOperNode(GT_COMMA, origCall->TypeGet(), finalTree, retVal);
+
// The JIT seems to want to CSE this comma and messes up multi-reg ret
// values in the process. Just avoid CSE'ing this tree entirely in that
// case.
if (origCall->HasMultiRegRetVal())
{
- comma->gtFlags |= GTF_DONT_CSE;
+ finalTree->gtFlags |= GTF_DONT_CSE;
}
- return comma;
+ return finalTree;
}
//------------------------------------------------------------------------
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39576, https://github.com/dotnet/runtime/issues/39579 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39576, https://github.com/dotnet/runtime/issues/39579 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39576, https://github.com/dotnet/runtime/issues/39579 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39576, https://github.com/dotnet/runtime/issues/39579 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39576, https://github.com/dotnet/runtime/issues/39579 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39576, https://github.com/dotnet/runtime/issues/39579 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39581 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39581 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39576, https://github.com/dotnet/runtime/issues/39579 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39576, https://github.com/dotnet/runtime/issues/39579 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39576, https://github.com/dotnet/runtime/issues/39579 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <!-- Disabled under GCStress due to https://github.com/dotnet/runtime/issues/39576, https://github.com/dotnet/runtime/issues/39579 -->
- <GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
<PropertyGroup>
<DebugType>Embedded</DebugType>
--- /dev/null
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+//
+// The test exposes the issue when:
+//
+// 1) methods Callee and Caller have the following structure:
+//
+// S Callee() { produce and return an instance of S }
+//
+// S Caller() { return Callee(); }
+//
+// 2) S is a value type with non-GC fields and has such size that it
+// must be passed via return buffer;
+//
+// 3) call to Callee is transformed to tail call via helper:
+//
+// S result;
+// DispatchTailCalls(&IL_STUB_CallTailCallTarget, (IntPtr)&result, _AddressOfReturnAddress());
+// return result;
+//
+// 4) Caller is invoked via Reflection.
+//
+// During morph phase the JIT discovers that both Caller and Callee have the return buffer arguments and
+// falsely assumes that the value can be directly passed from Caller to Callee.
+// Here is a problem: DispatchTailCalls helper expects the RetValue argument (in this case, return buffer argument)
+// to always point to the stack. However, during a reflection call the return buffer for S value type can be placed on the GC heap (see condition 2 above).
+// As a consequence, when GC is called at DispatchTailCalls the object corresponding to the return value buffer can be moved,
+// but the pointers passed to DispatchTailCalls will not be updated that leads to Callee using the non-updated location when writing its return value.
+//
+// Note: you will find below that Caller uses localloc - this is done in order to prevent a call to Caller to be transformed into fast tail call.
+// Note: COMPlus_GCStress=3 or COMPlus_GCStress=C are required in order to reliably expose this issue.
+
+.assembly extern System.Runtime
+{
+}
+
+.assembly Runtime_39581
+{
+}
+
+.class private sequential ansi sealed beforefieldinit int32x8
+ extends [System.Runtime]System.ValueType
+{
+ .field public int32 a
+ .field public int32 b
+ .field public int32 c
+ .field public int32 d
+ .field public int32 e
+ .field public int32 f
+ .field public int32 g
+ .field public int32 h
+}
+
+.class private auto ansi beforefieldinit Runtime_39581.Program
+ extends [System.Runtime]System.Object
+{
+ .method private hidebysig static valuetype int32x8 Callee(uint8* arg0) cil managed noinlining
+ {
+ .maxstack 2
+ .locals init (valuetype int32x8 V_0)
+ IL_0000: ldloca.s V_0
+ IL_0002: initobj int32x8
+ IL_0008: ldloca.s V_0
+ IL_000a: ldc.i4.0
+ IL_000b: stfld int32 int32x8::a
+ IL_0010: ldloca.s V_0
+ IL_0012: ldc.i4.1
+ IL_0013: stfld int32 int32x8::b
+ IL_0018: ldloca.s V_0
+ IL_001a: ldc.i4.2
+ IL_001b: stfld int32 int32x8::c
+ IL_0020: ldloca.s V_0
+ IL_0022: ldc.i4.3
+ IL_0023: stfld int32 int32x8::d
+ IL_0028: ldloca.s V_0
+ IL_002a: ldc.i4.4
+ IL_002b: stfld int32 int32x8::e
+ IL_0030: ldloca.s V_0
+ IL_0032: ldc.i4.5
+ IL_0033: stfld int32 int32x8::f
+ IL_0038: ldloca.s V_0
+ IL_003a: ldc.i4.6
+ IL_003b: stfld int32 int32x8::g
+ IL_0040: ldloca.s V_0
+ IL_0042: ldc.i4.7
+ IL_0043: stfld int32 int32x8::h
+ IL_0048: ldloc.0
+ IL_0049: ret
+ }
+
+ .method public hidebysig static valuetype int32x8 Caller(int32 arg0) cil managed noinlining
+ {
+ .maxstack 1
+ IL_0000: ldarg.0
+ IL_0001: conv.u
+ IL_0002: localloc
+ IL_0004: tail.
+ IL_0006: call valuetype int32x8 Runtime_39581.Program::Callee(uint8*)
+ IL_000b: ret
+ }
+
+ .method private hidebysig static int32 Main(string[] args) cil managed
+ {
+ .entrypoint
+ .maxstack 6
+ .locals init (valuetype int32x8 V_0)
+ IL_0000: ldtoken Runtime_39581.Program
+ IL_0005: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
+ IL_000a: ldstr "Caller"
+ IL_000f: ldc.i4.1
+ IL_0010: newarr [System.Runtime]System.Type
+ IL_0015: dup
+ IL_0016: ldc.i4.0
+ IL_0017: ldtoken [System.Runtime]System.Int32
+ IL_001c: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
+ IL_0021: stelem.ref
+ IL_0022: call instance class [System.Runtime]System.Reflection.MethodInfo [System.Runtime]System.Type::GetMethod(string, class [System.Runtime]System.Type[])
+ IL_0027: ldnull
+ IL_0028: ldc.i4.1
+ IL_0029: newarr [System.Runtime]System.Object
+ IL_002e: dup
+ IL_002f: ldc.i4.0
+ IL_0030: ldc.i4.1
+ IL_0031: box [System.Runtime]System.Int32
+ IL_0036: stelem.ref
+ IL_0037: callvirt instance object [System.Runtime]System.Reflection.MethodBase::Invoke(object, object[])
+ IL_003c: unbox.any int32x8
+ IL_0041: stloc.0
+ IL_0042: ldloc.0
+ IL_0043: ldfld int32 int32x8::a
+ IL_0048: brtrue.s IL_008c
+
+ IL_004a: ldloc.0
+ IL_004b: ldfld int32 int32x8::b
+ IL_0050: ldc.i4.1
+ IL_0051: bne.un.s IL_008c
+
+ IL_0053: ldloc.0
+ IL_0054: ldfld int32 int32x8::c
+ IL_0059: ldc.i4.2
+ IL_005a: bne.un.s IL_008c
+
+ IL_005c: ldloc.0
+ IL_005d: ldfld int32 int32x8::d
+ IL_0062: ldc.i4.3
+ IL_0063: bne.un.s IL_008c
+
+ IL_0065: ldloc.0
+ IL_0066: ldfld int32 int32x8::e
+ IL_006b: ldc.i4.4
+ IL_006c: bne.un.s IL_008c
+
+ IL_006e: ldloc.0
+ IL_006f: ldfld int32 int32x8::f
+ IL_0074: ldc.i4.5
+ IL_0075: bne.un.s IL_008c
+
+ IL_0077: ldloc.0
+ IL_0078: ldfld int32 int32x8::g
+ IL_007d: ldc.i4.6
+ IL_007e: bne.un.s IL_008c
+
+ IL_0080: ldloc.0
+ IL_0081: ldfld int32 int32x8::h
+ IL_0086: ldc.i4.7
+ IL_0087: bne.un.s IL_008c
+
+ IL_0089: ldc.i4.s 100
+ IL_008b: ret
+
+ IL_008c: ldc.i4.0
+ IL_008d: ret
+ }
+}
--- /dev/null
+<Project Sdk="Microsoft.NET.Sdk.IL">
+ <PropertyGroup>
+ <OutputType>Exe</OutputType>
+ </PropertyGroup>
+ <PropertyGroup>
+ <DebugType>None</DebugType>
+ <Optimize>True</Optimize>
+ <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+ </PropertyGroup>
+ <ItemGroup>
+ <Compile Include="$(MSBuildProjectName).il" />
+ </ItemGroup>
+</Project>
projects / platform / upstream / dotnet / runtime.git / commitdiff