Fix win-x64 outgoing argument size calculation for PInvoke prolog hel…

…per (#60050)

The win-x64 ABI requires allocating 32 bytes of outgoing argument
stack space for every call, which can be used by callees. In the case
where there are no calls in a function, there is no need to allocate
that space. The JIT computes the required argument space during
`fgSimpleLowering` by processing the IR and looking for call instructions,
but also accounting for a few special cases where calls are not visible
in the IR. One case was missing, leading to a zero-sized outgoing argument
space even though a call was still generated. This can cause corruption in the
caller stack frame. The case is where there are no explicit calls but the
JIT is going to generate a PInvoke frame initialization helper call. This
shouldn't happen, however, it occurs because the JIT eliminates a PInvoke
that is statically dead code, but still generates the prolog setup code.

A general solution would be to eliminate the PInvoke setup code if there
are no remaining PInvokes in the function. The simple change made here is
to ensure there is sufficient outgoing argument space for the PInvoke helper
call if that will be generated.

This was found by seeing test crashes due to stack corruption when
building the CLR with the VC++ compiler `-O2` optimization switch, which
stored data in the incoming argument stack space of the PInvoke helper function
(with PR #59235).

There are 5 SPMI diffs: 2 in coreclr tests (the ones that failed at runtime),
2 in libraries tests, and 1 in System.Net.Http.WinHttpChannelBinding:.ctor(SafeWinHttpHandle):this

src/coreclr/jit/flowgraph.cpp

@@ -2930,7 +2930,7 @@ void Compiler::fgFindOperOrder()
 
 //------------------------------------------------------------------------
 // fgSimpleLowering: do full walk of all IR, lowering selected operations
-// and computing lvaOutgoingArgumentAreaSize.
+// and computing lvaOutgoingArgSpaceSize.
 //
 // Notes:
 //    Lowers GT_ARR_LENGTH, GT_ARR_BOUNDS_CHECK, and GT_SIMD_CHK.
@@ -2941,7 +2941,7 @@ void Compiler::fgFindOperOrder()
 //    Outgoing arg area size is computed here because we want to run it
 //    after optimization (in case calls are removed) and need to look at
 //    all possible calls in the method.
-
+//
 void Compiler::fgSimpleLowering()
 {
 #if FEATURE_FIXED_OUT_ARGS
@@ -3023,9 +3023,9 @@ void Compiler::fgSimpleLowering()
 
                         if (thisCallOutAreaSize > outgoingArgSpaceSize)
                         {
+                            JITDUMP("Bumping outgoingArgSpaceSize from %u to %u for call [%06d]\n",
+                                    outgoingArgSpaceSize, thisCallOutAreaSize, dspTreeID(tree));
                             outgoingArgSpaceSize = thisCallOutAreaSize;
-                            JITDUMP("Bumping outgoingArgSpaceSize to %u for call [%06d]\n", outgoingArgSpaceSize,
-                                    dspTreeID(tree));
                         }
                         else
                         {
@@ -3054,18 +3054,24 @@ void Compiler::fgSimpleLowering()
     // Finish computing the outgoing args area size
     //
     // Need to make sure the MIN_ARG_AREA_FOR_CALL space is added to the frame if:
-    // 1. there are calls to THROW_HEPLPER methods.
+    // 1. there are calls to THROW_HELPER methods.
     // 2. we are generating profiling Enter/Leave/TailCall hooks. This will ensure
     //    that even methods without any calls will have outgoing arg area space allocated.
+    // 3. We will be generating calls to PInvoke helpers. TODO: This shouldn't be required because
+    //    if there are any calls to PInvoke methods, there should be a call that we processed
+    //    above. However, we still generate calls to PInvoke prolog helpers even if we have dead code
+    //    eliminated all the calls.
     //
     // An example for these two cases is Windows Amd64, where the ABI requires to have 4 slots for
     // the outgoing arg space if the method makes any calls.
     if (outgoingArgSpaceSize < MIN_ARG_AREA_FOR_CALL)
     {
-        if (compUsesThrowHelper || compIsProfilerHookNeeded())
+        if (compUsesThrowHelper || compIsProfilerHookNeeded() ||
+            (compMethodRequiresPInvokeFrame() && !opts.ShouldUsePInvokeHelpers()))
         {
             outgoingArgSpaceSize = MIN_ARG_AREA_FOR_CALL;
-            JITDUMP("Bumping outgoingArgSpaceSize to %u for throw helper or profile hook", outgoingArgSpaceSize);
+            JITDUMP("Bumping outgoingArgSpaceSize to %u for throw helper or profile hook or PInvoke helper",
+                    outgoingArgSpaceSize);
         }
     }
 

src/coreclr/jit/importer.cpp

@@ -7435,7 +7435,7 @@ void Compiler::impCheckForPInvokeCall(
         }
     }
 
-    JITLOG((LL_INFO1000000, "\nInline a CALLI PINVOKE call from method %s", info.compFullName));
+    JITLOG((LL_INFO1000000, "\nInline a CALLI PINVOKE call from method %s\n", info.compFullName));
 
     call->gtFlags |= GTF_CALL_UNMANAGED;
     call->unmgdCallConv = unmanagedCallConv;

src/coreclr/jit/lower.cpp

@@ -5948,7 +5948,7 @@ GenTree* Lowering::LowerArrElem(GenTree* node)
 
 PhaseStatus Lowering::DoPhase()
 {
-    // If we have any PInvoke calls, insert the one-time prolog code. We'll inserted the epilog code in the
+    // If we have any PInvoke calls, insert the one-time prolog code. We'll insert the epilog code in the
     // appropriate spots later. NOTE: there is a minor optimization opportunity here, as we still create p/invoke
     // data structures and setup/teardown even if we've eliminated all p/invoke calls due to dead code elimination.
     if (comp->compMethodRequiresPInvokeFrame())