[clr-interp] Fix argument alignment of 16 byte or greater aligned structures in the interpreter

src/coreclr/interpreter/compiler.cpp

@@ -1823,6 +1823,13 @@ int32_t InterpCompiler::GetInterpTypeStackSize(CORINFO_CLASS_HANDLE clsHnd, Inte
         // All vars are stored at 8 byte aligned offsets
         if (align < INTERP_STACK_SLOT_SIZE)
             align = INTERP_STACK_SLOT_SIZE;
+
+        // We do not align beyond the stack alignment 
+        // (This is relevant for structs with very high alignment requirements, 
+        // where we align within struct layout, but the structs are not actually
+        // aligned on the stack)
+        if (align > INTERP_STACK_ALIGNMENT)
+            align = INTERP_STACK_ALIGNMENT;
     }
     else
     {
@@ -3445,7 +3452,7 @@ void InterpCompiler::EmitPushUnboxAny(const CORINFO_GENERICHANDLE_RESULT& arg1,
     PushStackType(resultStackType, clsHndStack);
     int resultVar = m_pStackPointer[-1].var;
 
-    PushStackType(StackTypeI, NULL);
+    PushStackType(StackTypeByRef, NULL);
     int32_t intermediateVar = m_pStackPointer[-1].var;
     m_pStackPointer--;
 

src/coreclr/interpreter/compileropt.cpp

@@ -12,6 +12,20 @@ int32_t InterpCompiler::AllocVarOffset(int var, int32_t *pPos)
     offset = *pPos;
     size = m_pVars[var].size;
 
+    size_t align = INTERP_STACK_SLOT_SIZE;
+
+    if (size > INTERP_STACK_SLOT_SIZE)
+    {
+        assert(m_pVars[var].interpType == InterpTypeVT);
+        align = m_compHnd->getClassAlignmentRequirement(m_pVars[var].clsHnd);
+        if (align < INTERP_STACK_SLOT_SIZE)
+            align = INTERP_STACK_SLOT_SIZE;
+        if (align > INTERP_STACK_ALIGNMENT)
+            align = INTERP_STACK_ALIGNMENT;
+    }
+
+    offset = (int32_t)ALIGN_UP_TO(offset, align);
+
     m_pVars[var].offset = offset;
 
     *pPos = ALIGN_UP_TO(offset + size, INTERP_STACK_SLOT_SIZE);

src/coreclr/vm/amd64/AsmHelpers.asm

@@ -879,6 +879,12 @@ LEAF_ENTRY Store_XMM3, _TEXT
         jmp qword ptr [r11]
 LEAF_END Store_XMM3, _TEXT
 
+LEAF_ENTRY InjectInterpStackAlign, _TEXT
+        add r10, 8
+        add r11, 8
+        jmp qword ptr [r11]
+LEAF_END InjectInterpStackAlign, _TEXT
+
 ; Copy arguments from the interpreter stack to the processor stack.
 ; The CPU stack slots are aligned to pointer size.
 LEAF_ENTRY Load_Stack, _TEXT

src/coreclr/vm/amd64/asmhelpers.S

@@ -1148,6 +1148,12 @@ LEAF_END Store_XMM7, _TEXT
 // Routines for passing value type arguments by reference in general purpose registers RDI, RSI, RDX, RCX, R8, R9
 // from the interpreter to native code
 
+LEAF_ENTRY InjectInterpStackAlign, _TEXT
+        add r10, 8
+        add r11, 8
+        jmp qword ptr [r11]
+LEAF_END InjectInterpStackAlign, _TEXT
+
 // Copy arguments from the the interpreter stack to the CPU stack.
 // The CPU stack slots are aligned to pointer size.
 LEAF_ENTRY Load_Stack, _TEXT

src/coreclr/vm/arm64/asmhelpers.S

@@ -1312,6 +1312,12 @@ ALTERNATE_ENTRY Store_D7
     EPILOG_BRANCH_REG x11
 LEAF_END Store_D1_D2_D3_D4_D5_D6_D7
 
+LEAF_ENTRY InjectInterpStackAlign
+    add x9, x9, #8
+    ldr x11, [x10], #8
+    EPILOG_BRANCH_REG x11
+LEAF_END InjectInterpStackAlign
+
 // Copy arguments from the interpreter stack to the processor stack
 // The CPU stack slots are aligned to pointer size.
 LEAF_ENTRY Load_Stack

src/coreclr/vm/arm64/asmhelpers.asm

@@ -1627,6 +1627,12 @@ RefCopyDone$argReg
         EPILOG_BRANCH_REG x11
     LEAF_END Store_D1_D2_D3_D4_D5_D6_D7
 
+    LEAF_ENTRY InjectInterpStackAlign
+        add x9, x9, #8
+        ldr x11, [x10], #8
+        EPILOG_BRANCH_REG x11
+    LEAF_END InjectInterpStackAlign
+
     ; Routines for passing value type arguments by reference in general purpose registers X0..X7
     ; from the interpreter to native code
     ; Copy arguments from the interpreter stack to the processor stack

src/coreclr/vm/callstubgenerator.cpp

@@ -6,6 +6,7 @@
 #include "callstubgenerator.h"
 #include "ecall.h"
 
+extern "C" void InjectInterpStackAlign();
 extern "C" void Load_Stack();
 extern "C" void Store_Stack();
 
@@ -1350,7 +1351,7 @@ CallStubHeader *CallStubGenerator::GenerateCallStubForSig(MetaSig &sig)
 {
     STANDARD_VM_CONTRACT;
 
-    // Allocate space for the routines. The size of the array is conservatively set to twice the number of arguments
+    // Allocate space for the routines. The size of the array is conservatively set to three times the number of arguments
     // plus one slot for the target pointer and reallocated to the real size at the end.
     size_t tempStorageSize = ComputeTempStorageSize(sig);
     PCODE *pRoutines = (PCODE*)alloca(tempStorageSize);
@@ -1408,6 +1409,7 @@ void CallStubGenerator::ComputeCallStub(MetaSig &sig, PCODE *pRoutines)
 {
 
     ArgIterator argIt(&sig);
+    int32_t interpreterStackOffset = 0;
 
     m_r1 = NoRange; // indicates that there is no active range of general purpose registers
     m_r2 = 0;
@@ -1431,6 +1433,7 @@ void CallStubGenerator::ComputeCallStub(MetaSig &sig, PCODE *pRoutines)
         // we need to "inject" it here.
         // CLR ABI specifies that unlike the native Windows x64 calling convention, it is passed in the first argument register.
         m_r1 = 0;
+        interpreterStackOffset += INTERP_STACK_SLOT_SIZE;
     }
 
     if (argIt.HasParamType())
@@ -1442,6 +1445,7 @@ void CallStubGenerator::ComputeCallStub(MetaSig &sig, PCODE *pRoutines)
         ArgLocDesc paramArgLocDesc;
         argIt.GetParamTypeLoc(&paramArgLocDesc);
         ProcessArgument(NULL, paramArgLocDesc, pRoutines);
+        interpreterStackOffset += INTERP_STACK_SLOT_SIZE;
     }
 
     int ofs;
@@ -1453,6 +1457,53 @@ void CallStubGenerator::ComputeCallStub(MetaSig &sig, PCODE *pRoutines)
         ArgLocDesc argLocDesc;
         argIt.GetArgLoc(ofs, &argLocDesc);
 
+        // Each argument takes at least one slot on the interpreter stack
+        int interpStackSlotSize = INTERP_STACK_SLOT_SIZE;
+
+        // Each entry on the interpreter stack is always aligned to at least 8 bytes, but some arguments are 16 byte aligned
+        TypeHandle thArgTypeHandle;
+        if (argIt.GetArgType(&thArgTypeHandle) == ELEMENT_TYPE_VALUETYPE)
+        {
+            unsigned align = CEEInfo::getClassAlignmentRequirementStatic(thArgTypeHandle);
+            if (align < INTERP_STACK_SLOT_SIZE)
+            {
+                align = INTERP_STACK_SLOT_SIZE;
+            }
+            else if (align > INTERP_STACK_ALIGNMENT)
+            {
+                align = INTERP_STACK_ALIGNMENT;
+            }
+            assert(align == 8 || align == 16); // At the moment, we can only have an 8 or 16 byte alignment requirement here
+            while (interpreterStackOffset != ALIGN_UP(interpreterStackOffset, align))
+            {
+                if (m_r1 != NoRange)
+                {
+                    pRoutines[m_routineIndex++] = GetGPRegRangeRoutine(m_r1, m_r2);
+                    m_r1 = NoRange;
+                }
+                else if (m_x1 != NoRange)
+                {
+                    pRoutines[m_routineIndex++] = GetFPRegRangeRoutine(m_x1, m_x2);
+                    m_x1 = NoRange;
+                }
+                else if (m_s1 != NoRange)
+                {
+                    pRoutines[m_routineIndex++] = GetStackRoutine();
+                    pRoutines[m_routineIndex++] = ((int64_t)(m_s2 - m_s1 + 1) << 32) | m_s1;
+                    m_s1 = NoRange;
+                }
+
+                interpreterStackOffset += INTERP_STACK_SLOT_SIZE;
+                pRoutines[m_routineIndex++] = (PCODE)InjectInterpStackAlign;
+#if LOG_COMPUTE_CALL_STUB
+                printf("Inject stack align argument\n");
+#endif
+            }
+
+            interpStackSlotSize = ALIGN_UP(thArgTypeHandle.GetSize(), align);
+        }
+        interpreterStackOffset += interpStackSlotSize;
+
 #ifdef UNIX_AMD64_ABI
         if (argIt.GetArgLocDescForStructInRegs() != NULL)
         {

src/coreclr/vm/callstubgenerator.h

@@ -145,8 +145,8 @@ class CallStubGenerator
         int numArgs = sig.NumFixedArgs() + (sig.HasThis() ? 1 : 0);
 
         // The size of the temporary storage is the size of the CallStubHeader plus the size of the routines array.
-        // The size of the routines array is twice the number of arguments plus one slot for the target method pointer.
-        return sizeof(CallStubHeader) + ((numArgs + 1) * 2 + 1) * sizeof(PCODE);
+        // The size of the routines array is three the number of arguments plus one slot for the target method pointer.
+        return sizeof(CallStubHeader) + ((numArgs + 1) * 3 + 1) * sizeof(PCODE);
     }
     void ComputeCallStub(MetaSig &sig, PCODE *pRoutines);
 };

src/tests/Interop/PInvoke/Int128/Int128Test.cs

@@ -115,7 +115,6 @@ public static void TestInt128FieldLayout()
         StructWithInt128 rhs = new StructWithInt128(new Int128(13, 14));
 
         Int128Native.AddStructWithInt128_ByRef(ref lhs, ref rhs);
-        // Interpreter-FIXME: Incorrect result. Tracked by https://github.com/dotnet/runtime/issues/118618
         Assert.Equal(new StructWithInt128(new Int128(24, 26)), lhs);
 
         Int128 value2;