From 39dbeee8c64e0c845da1f6b3a8ef81707806a672 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 5 May 2022 19:44:03 +0200 Subject: [PATCH] Fix passing too small args for PUTARG_STK on macOS arm64 ABI (#68113) Backport of #68108 --- src/coreclr/jit/codegenarmarch.cpp | 29 +++++++++---------- .../JitBlue/Runtime_66720/Runtime_66720.cs | 29 +++++++++++++++++++ .../Runtime_66720/Runtime_66720.csproj | 9 ++++++ 3 files changed, 52 insertions(+), 15 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_66720/Runtime_66720.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_66720/Runtime_66720.csproj diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 1989d1d2036a4..1ce113357096c 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -681,12 +681,6 @@ void CodeGen::genIntrinsic(GenTree* treeNode) void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode) { assert(treeNode->OperIs(GT_PUTARG_STK)); - GenTree* source = treeNode->gtOp1; -#if !defined(OSX_ARM64_ABI) - var_types targetType = genActualType(source->TypeGet()); -#else - var_types targetType = source->TypeGet(); -#endif emitter* emit = GetEmitter(); // This is the varNum for our store operations, @@ -730,11 +724,13 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode) argOffsetMax = compiler->lvaOutgoingArgSpaceSize; } - bool isStruct = (targetType == TYP_STRUCT) || (source->OperGet() == GT_FIELD_LIST); + GenTree* source = treeNode->gtGetOp1(); + + bool isStruct = source->TypeIs(TYP_STRUCT) || (source->OperGet() == GT_FIELD_LIST); if (!isStruct) // a normal non-Struct argument { - if (varTypeIsSIMD(targetType)) + if (varTypeIsSIMD(source->TypeGet())) { assert(!source->isContained()); @@ -753,7 +749,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode) else #endif // OSX_ARM64_ABI { - emitAttr storeAttr = emitTypeSize(targetType); + emitAttr storeAttr = emitTypeSize(source->TypeGet()); emit->emitIns_S_R(INS_str, storeAttr, srcReg, varNumOut, argOffsetOut); argOffsetOut += EA_SIZE_IN_BYTES(storeAttr); } @@ -761,14 +757,17 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode) return; } -#if defined(OSX_ARM64_ABI) + var_types slotType = genActualType(source); +#ifdef OSX_ARM64_ABI + // Small typed args do not get their own full stack slots, so make + // sure we do not overwrite adjacent arguments. switch (treeNode->GetStackByteSize()) { case 1: - targetType = TYP_BYTE; + slotType = TYP_BYTE; break; case 2: - targetType = TYP_SHORT; + slotType = TYP_SHORT; break; default: assert(treeNode->GetStackByteSize() >= 4); @@ -776,8 +775,8 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode) } #endif - instruction storeIns = ins_Store(targetType); - emitAttr storeAttr = emitTypeSize(targetType); + instruction storeIns = ins_Store(slotType); + emitAttr storeAttr = emitTypeSize(slotType); // If it is contained then source must be the integer constant zero if (source->isContained()) @@ -797,7 +796,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode) genConsumeReg(source); emit->emitIns_S_R(storeIns, storeAttr, source->GetRegNum(), varNumOut, argOffsetOut); #ifdef TARGET_ARM - if (targetType == TYP_LONG) + if (source->TypeIs(TYP_LONG)) { // This case currently only occurs for double types that are passed as TYP_LONG; // actual long types would have been decomposed by now. diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_66720/Runtime_66720.cs b/src/tests/JIT/Regression/JitBlue/Runtime_66720/Runtime_66720.cs new file mode 100644 index 0000000000000..77303e01a8f8c --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_66720/Runtime_66720.cs @@ -0,0 +1,29 @@ +// 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; + +public class Runtime_66720 +{ + public static int Main() + { + return Test(0); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static int Test(in short zero) + { + // Fill arg stack slot with all ones + LastArg(0, 0, 0, 0, 0, 0, 0, 0, -1); + // Bug was that the last arg passed here would write only 16 bits + // instead of 32 bits + int last = LastArg(0, 0, 0, 0, 0, 0, 0, 0, zero); + return last == 0 ? 100 : -1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static int LastArg(int a, int b, int c, int d, int e, int f, int g, int h, int i) + { + return i; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_66720/Runtime_66720.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_66720/Runtime_66720.csproj new file mode 100644 index 0000000000000..f492aeac9d056 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_66720/Runtime_66720.csproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + + \ No newline at end of file