diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index f5de40823259e4..bbaba6051a0f33 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -1306,14 +1306,8 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode) } else // addrNode is used { - assert(addrNode != nullptr); - // TODO-Cleanup: `Lowering::NewPutArg` marks only `LCL_VAR_ADDR` as contained nowadays, - // but we use `genConsumeAddress` as a precaution, use `genConsumeReg()` instead. - assert(!addrNode->isContained()); - // Generate code to load the address that we need into a register - genConsumeAddress(addrNode); - addrReg = addrNode->GetRegNum(); + addrReg = genConsumeReg(addrNode); // If addrReg equal to baseReg, we use the last target register as alternative baseReg. // Because the candidate mask for the internal baseReg does not include any of the target register, @@ -1327,21 +1321,40 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode) ClassLayout* layout = source->AsObj()->GetLayout(); // Put on stack first - unsigned nextIndex = treeNode->gtNumRegs; - unsigned structOffset = nextIndex * TARGET_POINTER_SIZE; - int remainingSize = treeNode->GetStackByteSize(); + unsigned structOffset = treeNode->gtNumRegs * TARGET_POINTER_SIZE; + unsigned remainingSize = layout->GetSize() - structOffset; unsigned argOffsetOut = treeNode->getArgOffset(); - // remainingSize is always multiple of TARGET_POINTER_SIZE - assert(remainingSize % TARGET_POINTER_SIZE == 0); + assert((remainingSize > 0) && (roundUp(remainingSize, TARGET_POINTER_SIZE) == treeNode->GetStackByteSize())); while (remainingSize > 0) { - var_types type = layout->GetGCPtrType(nextIndex); + var_types type; + if (remainingSize >= TARGET_POINTER_SIZE) + { + type = layout->GetGCPtrType(structOffset / TARGET_POINTER_SIZE); + } + else if (remainingSize >= 4) + { + type = TYP_INT; + } + else if (remainingSize >= 2) + { + type = TYP_USHORT; + } + else + { + assert(remainingSize == 1); + type = TYP_UBYTE; + } + + emitAttr attr = emitActualTypeSize(type); + unsigned moveSize = genTypeSize(type); + instruction loadIns = ins_Load(type); if (varNode != nullptr) { - // Load from our varNumImp source - emit->emitIns_R_S(INS_ldr, emitTypeSize(type), baseReg, srcVarNum, structOffset); + // Load from our local source + emit->emitIns_R_S(loadIns, attr, baseReg, srcVarNum, structOffset); } else { @@ -1349,16 +1362,16 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode) assert(baseReg != addrReg); // Load from our address expression source - emit->emitIns_R_R_I(INS_ldr, emitTypeSize(type), baseReg, addrReg, structOffset); + emit->emitIns_R_R_I(loadIns, attr, baseReg, addrReg, structOffset); } - // 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 beyond the outgoing arg area - remainingSize -= TARGET_POINTER_SIZE; // We loaded 4-bytes of the struct - structOffset += TARGET_POINTER_SIZE; - nextIndex += 1; + // Emit the instruction to store the register into the outgoing argument area + emit->emitIns_S_R(ins_Store(type), attr, baseReg, varNumOut, argOffsetOut); + argOffsetOut += moveSize; + assert(argOffsetOut <= argOffsetMax); + + remainingSize -= moveSize; + structOffset += moveSize; } // We set up the registers in order, so that we assign the last target register `baseReg` is no longer in use, @@ -1371,7 +1384,7 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode) if (varNode != nullptr) { - // Load from our varNumImp source + // Load from our local source emit->emitIns_R_S(INS_ldr, emitTypeSize(type), targetReg, srcVarNum, structOffset); } else diff --git a/src/tests/JIT/Directed/StructABI/MisSizedStructs_ArmSplit.cs b/src/tests/JIT/Directed/StructABI/MisSizedStructs_ArmSplit.cs new file mode 100644 index 00000000000000..2f83aa0685b7da --- /dev/null +++ b/src/tests/JIT/Directed/StructABI/MisSizedStructs_ArmSplit.cs @@ -0,0 +1,124 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; + +public unsafe class MisSizedStructs_ArmSplit +{ + public const byte ByteValue = 0xC1; + + public static int Main() + { + if (ProblemWithOutOfBoundsLoads(out int result)) + { + return result; + } + + return 100; + } + + // Test that we do not load of bounds for split arguments on ARM. + // + static bool ProblemWithOutOfBoundsLoads(out int result) + { + result = 100; + + // TODO: enable for x64 once https://github.com/dotnet/runtime/issues/65937 has been fixed. + if (!OperatingSystem.IsLinux() || (RuntimeInformation.ProcessArchitecture == Architecture.X64)) + { + return false; + } + + const int PROT_NONE = 0x0; + const int PROT_READ = 0x1; + const int PROT_WRITE = 0x2; + const int MAP_PRIVATE = 0x02; + const int MAP_ANONYMOUS = 0x20; + const int PAGE_SIZE = 0x1000; + + byte* pages = (byte*)mmap(null, 2 * PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + + if (pages == (byte*)-1) + { + Console.WriteLine("Failed to allocate two pages, errno is {0}, giving up on the test", Marshal.GetLastSystemError()); + return false; + } + + if (mprotect(pages + PAGE_SIZE, PAGE_SIZE, PROT_NONE) != 0) + { + Console.WriteLine("Failed to protect the second page, errno is {0}, giving up on the test", Marshal.GetLastSystemError()); + munmap(pages, 2 * PAGE_SIZE); + return false; + } + + pages[PAGE_SIZE - 1] = ByteValue; + + if (CallForSplitStructWithSixteenBytes(0, *(StructWithSixteenBytes*)(pages + PAGE_SIZE - sizeof(StructWithSixteenBytes))) != ByteValue) + { + result = 200; + return true; + } + if (CallForSplitStructWithSeventeenBytes(0, *(StructWithSeventeenBytes*)(pages + PAGE_SIZE - sizeof(StructWithSeventeenBytes))) != ByteValue) + { + result = 201; + return true; + } + if (CallForSplitStructWithEighteenBytes(0, *(StructWithEighteenBytes*)(pages + PAGE_SIZE - sizeof(StructWithEighteenBytes))) != ByteValue) + { + result = 202; + return true; + } + if (CallForSplitStructWithNineteenBytes(0, *(StructWithNineteenBytes*)(pages + PAGE_SIZE - sizeof(StructWithNineteenBytes))) != ByteValue) + { + result = 203; + return true; + } + + munmap(pages, 2 * PAGE_SIZE); + + return false; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static byte CallForSplitStructWithSixteenBytes(long arg0, StructWithSixteenBytes splitArg) => splitArg.Bytes[15]; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static byte CallForSplitStructWithSeventeenBytes(long arg0, StructWithSeventeenBytes splitArg) => splitArg.Bytes[16]; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static byte CallForSplitStructWithEighteenBytes(long arg0, StructWithEighteenBytes splitArg) => splitArg.Bytes[17]; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static byte CallForSplitStructWithNineteenBytes(long arg0, StructWithNineteenBytes splitArg) => splitArg.Bytes[18]; + + [DllImport("libc")] + private static extern void* mmap(void* addr, nuint length, int prot, int flags, int fd, nuint offset); + + [DllImport("libc")] + private static extern int mprotect(void* addr, nuint len, int prot); + + [DllImport("libc")] + private static extern int munmap(void* addr, nuint length); + + struct StructWithSixteenBytes + { + public fixed byte Bytes[16]; + } + + struct StructWithSeventeenBytes + { + public fixed byte Bytes[17]; + } + + struct StructWithEighteenBytes + { + public fixed byte Bytes[18]; + } + + struct StructWithNineteenBytes + { + public fixed byte Bytes[19]; + } +} diff --git a/src/tests/JIT/Directed/StructABI/MisSizedStructs_ArmSplit.csproj b/src/tests/JIT/Directed/StructABI/MisSizedStructs_ArmSplit.csproj new file mode 100644 index 00000000000000..de62e29fa3e187 --- /dev/null +++ b/src/tests/JIT/Directed/StructABI/MisSizedStructs_ArmSplit.csproj @@ -0,0 +1,13 @@ + + + Exe + true + + + PdbOnly + True + + + + +