Merge pull request #10782 from parjong/fix/x86_funclet_unwinding

[x86/Linux] Use GCInfo for funclet unwinding

Author: BruceForstall

src/gcdump/i386/gcdumpx86.cpp

@@ -452,7 +452,10 @@ size_t              GCDump::DumpGCTable(PTR_CBYTE      table,
                     /* non-ptr arg push */
 
                     curOffs += (val & 0x07);
+#ifndef UNIX_X86_ABI
+                    // For x86/Linux, non-ptr arg pushes can be reported even for EBP frames
                     _ASSERTE(!header.ebpFrame);
+#endif // UNIX_X86_ABI
                     argCnt++;
 
                     DumpEncoding(bp, table-bp); bp = table;

src/jit/emit.cpp

@@ -4403,6 +4403,12 @@ unsigned emitter::emitEndCodeGen(Compiler* comp,
     emitFullyInt   = fullyInt;
     emitFullGCinfo = fullPtrMap;
 
+#ifndef UNIX_X86_ABI
+    emitFullArgInfo = !emitHasFramePtr;
+#else
+    emitFullArgInfo = fullPtrMap;
+#endif
+
 #if EMITTER_STATS
     GCrefsTable.record(emitGCrFrameOffsCnt);
     emitSizeTable.record(static_cast<unsigned>(emitSizeMethod));
@@ -6815,7 +6821,7 @@ void emitter::emitStackPushLargeStk(BYTE* addr, GCtype gcType, unsigned count)
         *u2.emitArgTrackTop++ = (BYTE)gcType;
         assert(u2.emitArgTrackTop <= u2.emitArgTrackTab + emitMaxStackDepth);
 
-        if (!emitHasFramePtr || needsGC(gcType))
+        if (emitFullArgInfo || needsGC(gcType))
         {
             if (emitFullGCinfo)
             {
@@ -6887,7 +6893,7 @@ void emitter::emitStackPopLargeStk(BYTE* addr, bool isCall, unsigned char callIn
 
         // This is an "interesting" argument
 
-        if (!emitHasFramePtr || needsGC(gcType))
+        if (emitFullArgInfo || needsGC(gcType))
         {
             argRecCnt += 1;
         }
@@ -7035,7 +7041,7 @@ void emitter::emitStackKillArgs(BYTE* addr, unsigned count, unsigned char callIn
 
         /* We're about to kill the corresponding (pointer) arg records */
 
-        if (emitHasFramePtr)
+        if (!emitFullArgInfo)
         {
             u2.emitGcArgTrackCnt -= gcCnt.Value();
         }

src/jit/emit.h

@@ -2018,8 +2018,9 @@ class emitter
     /*         The following logic keeps track of live GC ref values        */
     /************************************************************************/
 
-    bool emitFullGCinfo; // full GC pointer maps?
-    bool emitFullyInt;   // fully interruptible code?
+    bool emitFullArgInfo; // full arg info (including non-ptr arg)?
+    bool emitFullGCinfo;  // full GC pointer maps?
+    bool emitFullyInt;    // fully interruptible code?
 
 #if EMIT_TRACK_STACK_DEPTH
     unsigned emitCntStackDepth; // 0 in prolog/epilog, One DWORD elsewhere

src/jit/gcencode.cpp

@@ -2424,7 +2424,9 @@ size_t GCInfo::gcMakeRegPtrTable(BYTE* dest, int mask, const InfoHdr& header, un
 
                     assert((codeDelta & 0x7) == codeDelta);
                     *dest++ = 0xB0 | (BYTE)codeDelta;
+#ifndef UNIX_X86_ABI
                     assert(!compiler->isFramePointerUsed());
+#endif
 
                     /* Remember the new 'last' offset */
 

src/jit/morph.cpp

@@ -16202,7 +16202,9 @@ void Compiler::fgSetOptions()
     // to use a frame pointer because of EH. But until all the code uses
     // the same test, leave info.compXcptnsCount here.
     if (info.compXcptnsCount > 0)
+    {
         codeGen->setFramePointerRequiredEH(true);
+    }
 
 #else // !_TARGET_X86_
 
@@ -16213,6 +16215,15 @@ void Compiler::fgSetOptions()
 
 #endif // _TARGET_X86_
 
+#ifdef UNIX_X86_ABI
+    if (info.compXcptnsCount > 0)
+    {
+        assert(!codeGen->isGCTypeFixed());
+        // Enforce fully interruptible codegen for funclet unwinding
+        genInterruptible = true;
+    }
+#endif // UNIX_X86_ABI
+
     fgCheckArgCnt();
 
     if (info.compCallUnmanaged)

src/vm/eetwain.cpp

@@ -2413,6 +2413,18 @@ unsigned scanArgRegTableI(PTR_CBYTE     table,
     _ASSERTE(*castto(table, unsigned short *)++ == 0xBABE);
 #endif
 
+    bool      hasPartialArgInfo;
+
+#ifndef UNIX_X86_ABI
+    hasPartialArgInfo = info->ebpFrame;
+#else
+    // For x86/Linux, interruptible code always has full arg info
+    //
+    // This should be aligned with emitFullArgInfo setting at
+    // emitter::emitEndCodeGen (in JIT)
+    hasPartialArgInfo = false;
+#endif
+
   /*
       Encoding table for methods that are fully interruptible
 
@@ -2611,9 +2623,9 @@ unsigned scanArgRegTableI(PTR_CBYTE     table,
 
                         argOfs--;
                     }
-                    else if (info->ebpFrame)
+                    else if (hasPartialArgInfo)
                         argCnt--;
-                    else /* !ebpFrame && not a ref */
+                    else /* full arg info && not a ref */
                         argOfs--;
 
                     /* Continue with the next lower bit */
@@ -2622,11 +2634,11 @@ unsigned scanArgRegTableI(PTR_CBYTE     table,
                 }
                 while (argOfs);
 
-                _ASSERTE((info->ebpFrame != 0) ||
+                _ASSERTE(!hasPartialArgInfo    ||
                          isZero(argHigh)       ||
                         (argHigh == CONSTRUCT_ptrArgTP(1, (argCnt-1))));
 
-                if (info->ebpFrame)
+                if (hasPartialArgInfo)
                 {
                     while (!intersect(argHigh, ptrArgs) && (!isZero(argHigh)))
                         argHigh >>= 1;
@@ -2643,10 +2655,10 @@ unsigned scanArgRegTableI(PTR_CBYTE     table,
                 }
                 else
                 {
-                    /* For ESP-frames, all pushes are reported, and so
+                    /* Full arg info reports all pushes, and thus
                        argOffs has to be consistent with argCnt */
 
-                    _ASSERTE(info->ebpFrame || argCnt == argOfs);
+                    _ASSERTE(hasPartialArgInfo || argCnt == argOfs);
 
                     /* store arg count */
 
@@ -2692,7 +2704,7 @@ unsigned scanArgRegTableI(PTR_CBYTE     table,
             }
             else {
                 /* non-ptr arg push */
-                _ASSERTE(!(info->ebpFrame));
+                _ASSERTE(!hasPartialArgInfo);
                 ptrOffs += (val & 0x07);
                 if (ptrOffs > curOffs) {
                     iptr = isThis = false;
@@ -2753,9 +2765,9 @@ unsigned scanArgRegTableI(PTR_CBYTE     table,
                 }
             }
 
-            // For ebp-frames, need to find the next higest pointer for argHigh
+            // For partial arg info, need to find the next higest pointer for argHigh
 
-            if (info->ebpFrame)
+            if (hasPartialArgInfo)
             {
                 for(argHigh = ptrArgTP(0); !isZero(argMask); argMask >>= 1)
                 {
@@ -2794,7 +2806,7 @@ unsigned scanArgRegTableI(PTR_CBYTE     table,
     info->thisPtrResult  = thisPtrReg;
     _ASSERTE(thisPtrReg == REGI_NA || (regNumToMask(thisPtrReg) & info->regMaskResult));
 
-    if (info->ebpFrame)
+    if (hasPartialArgInfo)
     {
         return 0;
     }
@@ -3789,7 +3801,9 @@ bool UnwindEbpDoubleAlignFrame(
         PREGDISPLAY     pContext,
         EECodeInfo     *pCodeInfo,
         hdrInfo        *info,
+        PTR_CBYTE       table,
         PTR_CBYTE       methodStart,
+        DWORD           curOffs,
         unsigned        flags,
         StackwalkCacheUnwindInfo  *pUnwindInfo) // out-only, perf improvement
 {
@@ -3815,8 +3829,9 @@ bool UnwindEbpDoubleAlignFrame(
         // TODO Currently we assume that ESP of funclet frames is always fixed but actually it could change.
         if (pCodeInfo->IsFunclet())
         {
+            baseSP = curESP;
             // Set baseSP as initial SP
-            baseSP = pContext->pCurrentContext->ResumeEsp;
+            baseSP += GetPushedArgSize(info, table, curOffs);
             // 16-byte stack alignment padding (allocated in genFuncletProlog)
             baseSP += 12;
 
@@ -4014,7 +4029,7 @@ bool UnwindStackFrame(PREGDISPLAY     pContext,
          *  Now we know that have an EBP frame
          */
 
-        if (!UnwindEbpDoubleAlignFrame(pContext, pCodeInfo, info, methodStart, flags, pUnwindInfo))
+        if (!UnwindEbpDoubleAlignFrame(pContext, pCodeInfo, info, table, methodStart, curOffs, flags, pUnwindInfo))
             return false;
     }