Skip to content

Commit

Permalink
Properly account for padding in PUTARG_SPLIT with GT_FIELD_LIST (#57279)
Browse files Browse the repository at this point in the history
We were writing the stack part to the outgoing stack area by a loop
incrementing the offset by the written size in each iteration. However,
it is possible due to padding and optimization that the written size is
smaller than the delta to the next field. Instead, use the offset of
each field minus the offset of the first stack field to find the offset
in the outgoing arg area.

Fix #57064
  • Loading branch information
jakobbotsch authored Aug 14, 2021
1 parent 1f26d24 commit b2fc310
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 11 deletions.
30 changes: 19 additions & 11 deletions src/coreclr/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
emit->emitIns_S_R(INS_str, storeAttr, srcReg, varNumOut, argOffsetOut);
argOffsetOut += EA_SIZE_IN_BYTES(storeAttr);
}
assert(argOffsetOut <= argOffsetMax); // We can't write beyound the outgoing area area
assert(argOffsetOut <= argOffsetMax); // We can't write beyond the outgoing arg area
return;
}

Expand Down Expand Up @@ -810,7 +810,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
#endif // TARGET_ARM
}
argOffsetOut += EA_SIZE_IN_BYTES(storeAttr);
assert(argOffsetOut <= argOffsetMax); // We can't write beyound the outgoing area area
assert(argOffsetOut <= argOffsetMax); // We can't write beyond the outgoing arg area
}
else // We have some kind of a struct argument
{
Expand Down Expand Up @@ -1005,7 +1005,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
emit->emitIns_S_S_R_R(INS_stp, emitTypeSize(type0), emitTypeSize(type1), loReg, hiReg, varNumOut,
argOffsetOut);
argOffsetOut += (2 * TARGET_POINTER_SIZE); // We stored 16-bytes of the struct
assert(argOffsetOut <= argOffsetMax); // We can't write beyound the outgoing area area
assert(argOffsetOut <= argOffsetMax); // We can't write beyond the outgoing arg area

remainingSize -= (2 * TARGET_POINTER_SIZE); // We loaded 16-bytes of the struct
structOffset += (2 * TARGET_POINTER_SIZE);
Expand Down Expand Up @@ -1036,7 +1036,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
// Emit str instruction to store the register into the outgoing argument area
emit->emitIns_S_R(INS_str, emitTypeSize(type), loReg, varNumOut, argOffsetOut);
argOffsetOut += TARGET_POINTER_SIZE; // We stored 4-bytes of the struct
assert(argOffsetOut <= argOffsetMax); // We can't write beyound the outgoing area area
assert(argOffsetOut <= argOffsetMax); // We can't write beyond the outgoing arg area

remainingSize -= TARGET_POINTER_SIZE; // We loaded 4-bytes of the struct
structOffset += TARGET_POINTER_SIZE;
Expand Down Expand Up @@ -1100,7 +1100,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
instruction storeIns = ins_Store(type);
emit->emitIns_S_R(storeIns, attr, loReg, varNumOut, argOffsetOut);
argOffsetOut += moveSize;
assert(argOffsetOut <= argOffsetMax); // We can't write beyound the outgoing area area
assert(argOffsetOut <= argOffsetMax); // We can't write beyond the outgoing arg area

structOffset += moveSize;
nextIndex++;
Expand Down Expand Up @@ -1154,13 +1154,14 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)
emitter* emit = GetEmitter();
unsigned varNumOut = compiler->lvaOutgoingArgSpaceVar;
unsigned argOffsetMax = compiler->lvaOutgoingArgSpaceSize;
unsigned argOffsetOut = treeNode->getArgOffset();

if (source->OperGet() == GT_FIELD_LIST)
{
// Evaluate each of the GT_FIELD_LIST items into their register
// and store their register into the outgoing argument area
unsigned regIndex = 0;
unsigned regIndex = 0;
unsigned firstOnStackOffs = UINT_MAX;

for (GenTreeFieldList::Use& use : source->AsFieldList()->Uses())
{
GenTree* nextArgNode = use.GetNode();
Expand All @@ -1169,14 +1170,20 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)

if (regIndex >= treeNode->gtNumRegs)
{
if (firstOnStackOffs == UINT_MAX)
{
firstOnStackOffs = use.GetOffset();
}
var_types type = nextArgNode->TypeGet();
emitAttr attr = emitTypeSize(type);

unsigned offset = treeNode->getArgOffset() + use.GetOffset() - firstOnStackOffs;
// We can't write beyond the outgoing arg area
assert(offset + EA_SIZE_IN_BYTES(attr) <= argOffsetMax);

// Emit store instructions to store the registers produced by the GT_FIELD_LIST into the outgoing
// argument area
emit->emitIns_S_R(ins_Store(type), attr, fieldReg, varNumOut, argOffsetOut);
argOffsetOut += EA_SIZE_IN_BYTES(attr);
assert(argOffsetOut <= argOffsetMax); // We can't write beyound the outgoing area area
emit->emitIns_S_R(ins_Store(type), attr, fieldReg, varNumOut, offset);
}
else
{
Expand Down Expand Up @@ -1287,6 +1294,7 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)
unsigned nextIndex = treeNode->gtNumRegs;
unsigned structOffset = nextIndex * TARGET_POINTER_SIZE;
int remainingSize = treeNode->GetStackByteSize();
unsigned argOffsetOut = treeNode->getArgOffset();

// remainingSize is always multiple of TARGET_POINTER_SIZE
assert(remainingSize % TARGET_POINTER_SIZE == 0);
Expand All @@ -1311,7 +1319,7 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)
// Emit str instruction to store the register into the outgoing argument area
emit->emitIns_S_R(INS_str, emitTypeSize(type), baseReg, varNumOut, argOffsetOut);
argOffsetOut += TARGET_POINTER_SIZE; // We stored 4-bytes of the struct
assert(argOffsetOut <= argOffsetMax); // We can't write beyound the outgoing area area
assert(argOffsetOut <= argOffsetMax); // We can't write beyond the outgoing arg area
remainingSize -= TARGET_POINTER_SIZE; // We loaded 4-bytes of the struct
structOffset += TARGET_POINTER_SIZE;
nextIndex += 1;
Expand Down
35 changes: 35 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_57064/Runtime_57064.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.CompilerServices;

struct S
{
public uint F0;
public ushort F1;
public uint F2;
}

public class Runtime_57064
{
public static int Main()
{
S val = Create();
val.F0 = 0xF0;
val.F1 = 0xF1;
val.F2 = 0xF2;
// This call splits S between registers and stack on ARM32.
// The issue was that we were writing S.F2 at stack+2
// instead of stack+4 when val was promoted.
return Split(null, false, null, val) == 0xF2 ? 100 : -1;
}

[MethodImpl(MethodImplOptions.NoInlining)]
static uint Split(ushort[] arg0, bool arg1, bool[] arg2, S arg3)
{
return arg3.F2;
}

[MethodImpl(MethodImplOptions.NoInlining)]
static S Create() => default;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
</PropertyGroup>
<PropertyGroup>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit b2fc310

Please sign in to comment.