Handle mis-sized structs in ARM/64 PUTARG_SPLIT codegen

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,38 +1321,57 @@ 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
             {
                 // check for case of destroying the addrRegister while we still need it
                 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

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];
+    }
+}

src/tests/JIT/Directed/StructABI/MisSizedStructs_ArmSplit.csproj

@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>PdbOnly</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>