ARM64: Fix Vararg for PInvoke

Fixes https://github.com/dotnet/coreclr/issues/4474
1. Fix entry point to `VarargPInvokeStub_RetBuffArg` instead of
`VarargPInvokeStub` to pass `VASigCookieReg` via `x1` when hasRetBuffArg is
true.
2. The previous fix for varargs SP offset is not correct when both varargs
and callee save regs exist. Fix the SP offset computation from caller's SP.
3. Fix epilog which didn't take varargs into account.

diff --git a/src/jit/codegencommon.cpp b/src/jit/codegencommon.cpp
index 7daaff4..331cfbd 100644 (file)
--- a/src/jit/codegencommon.cpp
+++ b/src/jit/codegencommon.cpp
@@ -6359,8 +6359,9 @@ void            CodeGen::genPopCalleeSavedRegistersAndFreeLclFrame(bool jmpEpilo
             }
 
             regsToRestoreMask &= ~(RBM_FP | RBM_LR); // We'll restore FP/LR at the end, and post-index SP.
-            calleeSaveSPOffset = totalFrameSize - genCountBits(regsToRestoreMask) * REGSIZE_BYTES;
 
+            // Compute callee save SP offset which is at the top of local frame while the FP/LR is saved at the bottom of stack.
+            calleeSaveSPOffset = compiler->compLclFrameSize + 2 * REGSIZE_BYTES;
         }
         else if (totalFrameSize <= 512)
         {
@@ -6374,7 +6375,9 @@ void            CodeGen::genPopCalleeSavedRegistersAndFreeLclFrame(bool jmpEpilo
             }
 
             regsToRestoreMask &= ~(RBM_FP | RBM_LR); // We'll restore FP/LR at the end, and post-index SP.
-            calleeSaveSPOffset = totalFrameSize - genCountBits(regsToRestoreMask) * REGSIZE_BYTES;
+
+            // Compute callee save SP offset which is at the top of local frame while the FP/LR is saved at the bottom of stack.
+            calleeSaveSPOffset = compiler->compLclFrameSize + 2 * REGSIZE_BYTES;
         }
         else
         {
@@ -6385,9 +6388,6 @@ void            CodeGen::genPopCalleeSavedRegistersAndFreeLclFrame(bool jmpEpilo
             assert((calleeSaveSPDeltaUnaligned % 8) == 0); // It better at least be 8 byte aligned.
             calleeSaveSPDelta = AlignUp((UINT)calleeSaveSPDeltaUnaligned, STACK_ALIGN);
 
-            calleeSaveSPOffset = calleeSaveSPDelta - calleeSaveSPDeltaUnaligned;
-            assert((calleeSaveSPOffset == 0) || (calleeSaveSPOffset == REGSIZE_BYTES)); // At most one alignment slot between SP and where we store the callee-saved registers.
-
             regsToRestoreMask &= ~(RBM_FP | RBM_LR); // We'll restore FP/LR at the end, and (hopefully) post-index SP.
 
             int remainingFrameSz = totalFrameSize - calleeSaveSPDelta;
@@ -6442,7 +6442,12 @@ void            CodeGen::genPopCalleeSavedRegistersAndFreeLclFrame(bool jmpEpilo
                 genEpilogRestoreRegPair(REG_FP, REG_LR, compiler->lvaOutgoingArgSpaceSize, remainingFrameSz, REG_IP0, nullptr);
             }
         }
-        assert(calleeSaveSPOffset >= 0);
+
+        // Unlike frameType=1 or frameType=2 that restore SP at the end,
+        // frameType=3 already adjusted SP above to delete local frame.
+        // There is at most one alignment slot between SP and where we store the callee-saved registers.
+        calleeSaveSPOffset = calleeSaveSPDelta - calleeSaveSPDeltaUnaligned;
+        assert((calleeSaveSPOffset == 0) || (calleeSaveSPOffset == REGSIZE_BYTES));
     }
     else
     {

diff --git a/src/jit/lclvars.cpp b/src/jit/lclvars.cpp
index a4dbe97..40947fc 100644 (file)
--- a/src/jit/lclvars.cpp
+++ b/src/jit/lclvars.cpp
@@ -4732,25 +4732,27 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals()
     // If the frame pointer is used, then we'll save FP/LR at the bottom of the stack.
     // Otherwise, we won't store FP, and we'll store LR at the top, with the other callee-save
     // registers (if any).
+
     int initialStkOffs = 0;
+    if (info.compIsVarArgs)
+    {
+        // For varargs we always save all of the integer register arguments
+        // so that they are contiguous with the incoming stack arguments.
+        initialStkOffs = MAX_REG_ARG * REGSIZE_BYTES;
+        stkOffs -= initialStkOffs;
+    }
+
     if (isFramePointerUsed())
     {
         // Subtract off FP and LR.
         assert(compCalleeRegsPushed >= 2);
-        initialStkOffs = (compCalleeRegsPushed - 2) * REGSIZE_BYTES;
+        stkOffs -= (compCalleeRegsPushed - 2) * REGSIZE_BYTES;
     }
     else
     {
-        initialStkOffs = compCalleeRegsPushed * REGSIZE_BYTES;
+        stkOffs -= compCalleeRegsPushed * REGSIZE_BYTES;
     }
 
-    if (info.compIsVarArgs)
-    {
-        initialStkOffs += MAX_REG_ARG * REGSIZE_BYTES;
-    }
-
-    stkOffs -= initialStkOffs;
-
 #else // !_TARGET_ARM64_
     stkOffs -= compCalleeRegsPushed * REGSIZE_BYTES;
 #endif // !_TARGET_ARM64_

diff --git a/src/vm/prestub.cpp b/src/vm/prestub.cpp
index c82378f..a35b421 100644 (file)
--- a/src/vm/prestub.cpp
+++ b/src/vm/prestub.cpp
@@ -1682,7 +1682,7 @@ PCODE TheVarargNDirectStub(BOOL hasRetBuffArg)
 {
     LIMITED_METHOD_CONTRACT;
 
-#if defined(_TARGET_ARM_) || defined(_TARGET_AMD64_)
+#if !defined(_TARGET_X86_)
     if (hasRetBuffArg)
     {
         return GetEEFuncEntryPoint(VarargPInvokeStub_RetBuffArg);

diff --git a/tests/arm64/Tests.lst b/tests/arm64/Tests.lst
index 20390b7..fd3d449 100644 (file)
--- a/tests/arm64/Tests.lst
+++ b/tests/arm64/Tests.lst
@@ -11638,21 +11638,21 @@ RelativePath=JIT\jit64\mcc\interop\mcc_i00\mcc_i00.cmd
 WorkingDir=JIT\jit64\mcc\interop\mcc_i00
 Expected=0
 MaxAllowedDurationSeconds=600
-Categories=Pri0;EXPECTED_FAIL;NATIVE_INTEROP
+Categories=Pri0;JIT;EXPECTED_PASS
 HostStyle=0
 [mcc_i01.cmd_1693]
 RelativePath=JIT\jit64\mcc\interop\mcc_i01\mcc_i01.cmd
 WorkingDir=JIT\jit64\mcc\interop\mcc_i01
 Expected=0
 MaxAllowedDurationSeconds=600
-Categories=Pri0;EXPECTED_FAIL;NATIVE_INTEROP
+Categories=Pri0;JIT;EXPECTED_PASS
 HostStyle=0
 [mcc_i02.cmd_1694]
 RelativePath=JIT\jit64\mcc\interop\mcc_i02\mcc_i02.cmd
 WorkingDir=JIT\jit64\mcc\interop\mcc_i02
 Expected=0
 MaxAllowedDurationSeconds=600
-Categories=Pri0;EXPECTED_FAIL;NATIVE_INTEROP
+Categories=Pri0;JIT;EXPECTED_PASS
 HostStyle=0
 [mcc_i03.cmd_1695]
 RelativePath=JIT\jit64\mcc\interop\mcc_i03\mcc_i03.cmd