From b2fc310998f1d25ed01102a47366f89b5c1c9e9c Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sat, 14 Aug 2021 11:10:06 +0200 Subject: [PATCH] Properly account for padding in PUTARG_SPLIT with GT_FIELD_LIST (#57279) 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 --- src/coreclr/jit/codegenarmarch.cpp | 30 ++++++++++++------- .../JitBlue/Runtime_57064/Runtime_57064.cs | 35 ++++++++++++++++++++++ .../JitBlue/Runtime_57064/Runtime_57064.csproj | 12 ++++++++ 3 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57064/Runtime_57064.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57064/Runtime_57064.csproj diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index ccdd576..1989d1d 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -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; } @@ -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 { @@ -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); @@ -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; @@ -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++; @@ -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(); @@ -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 { @@ -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); @@ -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; diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57064/Runtime_57064.cs b/src/tests/JIT/Regression/JitBlue/Runtime_57064/Runtime_57064.cs new file mode 100644 index 0000000..463fdd5 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57064/Runtime_57064.cs @@ -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; +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57064/Runtime_57064.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_57064/Runtime_57064.csproj new file mode 100644 index 0000000..f3e1cbd --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57064/Runtime_57064.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + + -- 2.7.4