From 62e05a0d18968704c6b0a48b584305a8f747cdcf Mon Sep 17 00:00:00 2001 From: Pat Gavlin Date: Wed, 22 Feb 2017 11:05:25 -0800 Subject: [PATCH] Do not report FP restores in x86 epilogs. 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 | 7 ++++++ src/coreclr/src/jit/emit.cpp | 43 ++++++++++++++++++++++------------- src/coreclr/src/jit/emit.h | 13 +++++++---- src/coreclr/src/jit/emitxarch.cpp | 7 ++---- 4 files changed, 45 insertions(+), 25 deletions(-) diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp index 65cc6de..68b5820 100644 --- a/src/coreclr/src/jit/codegencommon.cpp +++ b/src/coreclr/src/jit/codegencommon.cpp @@ -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()) diff --git a/src/coreclr/src/jit/emit.cpp b/src/coreclr/src/jit/emit.cpp index 70a4905..8f92826 100644 --- a/src/coreclr/src/jit/emit.cpp +++ b/src/coreclr/src/jit/emit.cpp @@ -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 diff --git a/src/coreclr/src/jit/emit.h b/src/coreclr/src/jit/emit.h index f57cc0a..186c60f 100644 --- a/src/coreclr/src/jit/emit.h +++ b/src/coreclr/src/jit/emit.h @@ -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(); diff --git a/src/coreclr/src/jit/emitxarch.cpp b/src/coreclr/src/jit/emitxarch.cpp index 796af19..ad4000e 100644 --- a/src/coreclr/src/jit/emitxarch.cpp +++ b/src/coreclr/src/jit/emitxarch.cpp @@ -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; -- 2.7.4