Merge pull request #7585 from CarolEidt/Fix5699

x86: adjust stack level at throw blocks

5 files changed, 48 insertions, 22 deletions

diff --git a/src/jit/codegen.h b/src/jit/codegen.h
index e58701e716..e042efae0e 100755
--- a/src/jit/codegen.h
+++ b/src/jit/codegen.h
@@ -250,6 +250,8 @@ protected:
 
     void genAdjustSP(ssize_t delta);
 
+    void genAdjustStackLevel(BasicBlock* block);
+
     void genExitCode(BasicBlock* block);
 
     //-------------------------------------------------------------------------
diff --git a/src/jit/codegencommon.cpp b/src/jit/codegencommon.cpp
index a9e43e8f0e..cb2013d8b7 100755
--- a/src/jit/codegencommon.cpp
+++ b/src/jit/codegencommon.cpp
@@ -1632,6 +1632,44 @@ void CodeGen::genAdjustSP(ssize_t delta)
         inst_RV_IV(INS_add, REG_SPBASE, delta, EA_PTRSIZE);
 }
 
+//------------------------------------------------------------------------
+// genAdjustStackLevel: Adjust the stack level, if required, for a throw helper block
+//
+// Arguments:
+//    block - The BasicBlock for which we are about to generate code.
+//
+// Assumptions:
+//    Must be called just prior to generating code for 'block'.
+//
+// Notes:
+//    This only makes an adjustment if !FEATURE_FIXED_OUT_ARGS, if there is no frame pointer,
+//    and if 'block' is a throw helper block with a non-zero stack level.
+
+void CodeGen::genAdjustStackLevel(BasicBlock* block)
+{
+#if !FEATURE_FIXED_OUT_ARGS
+    // Check for inserted throw blocks and adjust genStackLevel.
+
+    if (!isFramePointerUsed() && compiler->fgIsThrowHlpBlk(block))
+    {
+        noway_assert(block->bbFlags & BBF_JMP_TARGET);
+
+        genStackLevel = compiler->fgThrowHlpBlkStkLevel(block) * sizeof(int);
+
+        if (genStackLevel != 0)
+        {
+#ifdef _TARGET_X86_
+            getEmitter()->emitMarkStackLvl(genStackLevel);
+            inst_RV_IV(INS_add, REG_SPBASE, genStackLevel, EA_PTRSIZE);
+            genStackLevel = 0;
+#else  // _TARGET_X86_
+            NYI("Need emitMarkStackLvl()");
+#endif // _TARGET_X86_
+        }
+    }
+#endif // !FEATURE_FIXED_OUT_ARGS
+}
+
 #ifdef _TARGET_ARM_
 // return size
 // alignmentWB is out param
diff --git a/src/jit/codegenlegacy.cpp b/src/jit/codegenlegacy.cpp
index 4b225a318b..9ce445fddb 100644
--- a/src/jit/codegenlegacy.cpp
+++ b/src/jit/codegenlegacy.cpp
@@ -12762,27 +12762,7 @@ void CodeGen::genCodeForBBlist()
         genResetFPstkLevel();
 #endif // FEATURE_STACK_FP_X87
 
-#if !FEATURE_FIXED_OUT_ARGS
-        /* Check for inserted throw blocks and adjust genStackLevel */
-
-        if (!isFramePointerUsed() && compiler->fgIsThrowHlpBlk(block))
-        {
-            noway_assert(block->bbFlags & BBF_JMP_TARGET);
-
-            genStackLevel = compiler->fgThrowHlpBlkStkLevel(block) * sizeof(int);
-
-            if (genStackLevel)
-            {
-#ifdef _TARGET_X86_
-                getEmitter()->emitMarkStackLvl(genStackLevel);
-                inst_RV_IV(INS_add, REG_SPBASE, genStackLevel, EA_PTRSIZE);
-                genStackLevel = 0;
-#else  // _TARGET_X86_
-                NYI("Need emitMarkStackLvl()");
-#endif // _TARGET_X86_
-            }
-        }
-#endif // !FEATURE_FIXED_OUT_ARGS
+        genAdjustStackLevel(block);
 
         savedStkLvl = genStackLevel;
 
diff --git a/src/jit/codegenlinear.cpp b/src/jit/codegenlinear.cpp
index 121b4ffb65..9d5fb18a2b 100644
--- a/src/jit/codegenlinear.cpp
+++ b/src/jit/codegenlinear.cpp
@@ -296,7 +296,7 @@ void CodeGen::genCodeForBBlist()
         /* Both stacks are always empty on entry to a basic block */
 
         genStackLevel = 0;
-
+        genAdjustStackLevel(block);
         savedStkLvl = genStackLevel;
 
         /* Tell everyone which basic block we're working on */
diff --git a/src/jit/sideeffects.h b/src/jit/sideeffects.h
index 33fac16f05..e14b2925ed 100644
--- a/src/jit/sideeffects.h
+++ b/src/jit/sideeffects.h
@@ -136,6 +136,12 @@ public:
 // SideEffectSet:
 //    Represents a set of side effects for the purposes of analyzing code
 //    motion.
+//    Note that for non-fixed-size frames without a frame pointer (currently
+//    x86-only), we don't track the modification of the stack level that occurs
+//    with a GT_PUTARG_STK as a side-effect. If we ever support general code
+//    reordering, that would have to be taken into account. As it happens,
+//    we currently do not reorder any other side-effecting nodes relative to
+//    these.
 //
 class SideEffectSet final
 {