Do not report FP restores in x86 epilogs.
authorPat Gavlin <pagavlin@microsoft.com>
Wed, 22 Feb 2017 19:05:25 +0000 (11:05 -0800)
committerPat Gavlin <pagavlin@microsoft.com>
Thu, 23 Feb 2017 01:35:37 +0000 (17:35 -0800)
The x86 unwinder neither needs nor expects to see these restores (which
right now consist solely of a `vzeroupper` instruction that is emitted
to eliminate AVX <-> SSE transition penalties), which was causing unwind
failures under GC stress. This change stops reporting these restores as
part of a function epilog.

Fixes dotnet/coreclr#9452.

Commit migrated from https://github.com/dotnet/coreclr/commit/ce70f4c2acae22fd8eaee1588c3f54596e521c8a

src/coreclr/src/jit/codegencommon.cpp
src/coreclr/src/jit/emit.cpp
src/coreclr/src/jit/emit.h
src/coreclr/src/jit/emitxarch.cpp

index 65cc6de..68b5820 100644 (file)
@@ -9450,6 +9450,13 @@ void CodeGen::genFnEpilog(BasicBlock* block)
     genRestoreCalleeSavedFltRegs(compiler->compLclFrameSize);
 #endif // !FEATURE_STACK_FP_X87
 
+#ifdef JIT32_GCENCODER
+    // On IA32, we start the OS-reported portion of the epilog after restoring any callee-saved
+    // floating-point registers. This avoids the need to update the x86 unwinder and retains binary
+    // compatibility between later versions of the JIT and earlier versions of the runtime.
+    getEmitter()->emitStartEpilog();
+#endif
+
     /* Compute the size in bytes we've pushed/popped */
 
     if (!doubleAlignOrFramePointerUsed())
index 70a4905..8f92826 100644 (file)
@@ -1027,7 +1027,7 @@ void emitter::emitBegFN(bool hasFramePtr
     emitPlaceholderList = emitPlaceholderLast = nullptr;
 
 #ifdef JIT32_GCENCODER
-    emitEpilogList = emitEpilogLast = NULL;
+    emitEpilogList = emitEpilogLast = nullptr;
 #endif // JIT32_GCENCODER
 
     /* We don't have any jumps */
@@ -1215,14 +1215,12 @@ size_t emitter::emitGenEpilogLst(size_t (*fp)(void*, unsigned), void* cp)
     EpilogList* el;
     size_t      sz;
 
-    for (el = emitEpilogList, sz = 0; el; el = el->elNext)
+    for (el = emitEpilogList, sz = 0; el != nullptr; el = el->elNext)
     {
-        assert(el->elIG->igFlags & IGF_EPILOG);
+        assert(el->elLoc.GetIG()->igFlags & IGF_EPILOG);
 
-        UNATIVE_OFFSET ofs =
-            el->elIG->igOffs; // The epilog starts at the beginning of the IG, so the IG offset is correct
-
-        sz += fp(cp, ofs);
+        // The epilog starts at the location recorded in the epilog list.
+        sz += fp(cp, el->elLoc.CodeOffset(this));
     }
 
     return sz;
@@ -1957,22 +1955,20 @@ void emitter::emitBegFnEpilog(insGroup* igPh)
 
 #ifdef JIT32_GCENCODER
 
-    EpilogList* el = new (emitComp, CMK_GC) EpilogList;
-    el->elNext     = NULL;
-    el->elIG       = emitCurIG;
+    EpilogList* el = new (emitComp, CMK_GC) EpilogList();
 
-    if (emitEpilogLast)
+    if (emitEpilogLast != nullptr)
+    {
         emitEpilogLast->elNext = el;
+    }
     else
+    {
         emitEpilogList = el;
+    }
 
     emitEpilogLast = el;
 
 #endif // JIT32_GCENCODER
-
-    /* Remember current position so that we can compute total epilog size */
-
-    emitEpilogBegLoc.CaptureLocation(this);
 }
 
 /*****************************************************************************
@@ -1984,8 +1980,11 @@ void emitter::emitEndFnEpilog()
 {
     emitEndPrologEpilog();
 
+#ifdef JIT32_GCENCODER
+    assert(emitEpilogLast != nullptr);
+
     UNATIVE_OFFSET newSize;
-    UNATIVE_OFFSET epilogBegCodeOffset = emitEpilogBegLoc.CodeOffset(this);
+    UNATIVE_OFFSET epilogBegCodeOffset = emitEpilogLast->elLoc.CodeOffset(this);
 #ifdef _TARGET_XARCH_
     UNATIVE_OFFSET epilogExitSeqStartCodeOffset = emitExitSeqBegLoc.CodeOffset(this);
 #else
@@ -2021,6 +2020,7 @@ void emitter::emitEndFnEpilog()
     }
 
 #endif // _TARGET_X86_
+#endif // JIT32_GCENCODER
 }
 
 #if FEATURE_EH_FUNCLETS
@@ -2067,8 +2067,19 @@ void emitter::emitEndFuncletEpilog()
 
 #endif // FEATURE_EH_FUNCLETS
 
+
 #ifdef JIT32_GCENCODER
 
+//
+// emitter::emitStartEpilog:
+//   Mark the current position so that we can later compute the total epilog size.
+//
+void emitter::emitStartEpilog()
+{
+    assert(emitEpilogLast != nullptr);
+    emitEpilogLast->elLoc.CaptureLocation(this);
+}
+
 /*****************************************************************************
  *
  *  Return non-zero if the current method only has one epilog, which is
index f57cc0a..186c60f 100644 (file)
@@ -1518,14 +1518,21 @@ protected:
     // IG of the epilog, and use it to find the epilog offset at the end of code generation.
     struct EpilogList
     {
-        EpilogList* elNext;
-        insGroup*   elIG;
+        EpilogList*  elNext;
+        emitLocation elLoc;
+
+        EpilogList()
+            : elNext(nullptr), elLoc()
+        {
+        }
     };
 
     EpilogList* emitEpilogList; // per method epilog list - head
     EpilogList* emitEpilogLast; // per method epilog list - tail
 
 public:
+    void emitStartEpilog();
+
     bool emitHasEpilogEnd();
 
     size_t emitGenEpilogLst(size_t (*fp)(void*, unsigned), void* cp);
@@ -1535,8 +1542,6 @@ public:
     void emitBegPrologEpilog(insGroup* igPh);
     void emitEndPrologEpilog();
 
-    emitLocation emitEpilogBegLoc;
-
     void emitBegFnEpilog(insGroup* igPh);
     void emitEndFnEpilog();
 
index 796af19..ad4000e 100644 (file)
@@ -2321,11 +2321,8 @@ void emitter::emitIns(instruction ins)
     }
 
 #ifndef LEGACY_BACKEND
-    // Account for 2-byte VEX prefix in case of vzeroupper
-    if (ins == INS_vzeroupper)
-    {
-        sz += 2;
-    }
+    // vzeroupper includes its 2-byte VEX prefix in its MR code.
+    assert((ins != INS_vzeroupper) || (sz == 3));
 #endif
 
     insFormat fmt = IF_NONE;