From 00587db40167d2cc72384863b6f4f638d224202e Mon Sep 17 00:00:00 2001
From: Aaron Robinson <arobins@microsoft.com>
Date: Tue, 17 Mar 2020 18:48:32 -0700
Subject: [PATCH] Reverse P/Invoke methods do not support tailcalls. (#33677)

* Disable tailcalls in all scenarios involving a Reverse P/Invoke
---
 src/coreclr/src/jit/flowgraph.cpp  |  6 ++++++
 src/coreclr/src/jit/importer.cpp   | 12 ++++++++++++
 src/coreclr/src/jit/lower.cpp      |  1 +
 src/coreclr/src/jit/patchpoint.cpp |  6 ++++++
 4 files changed, 25 insertions(+)

diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp
index 8ba6d2f..b7ccbcb 100644
--- a/src/coreclr/src/jit/flowgraph.cpp
+++ b/src/coreclr/src/jit/flowgraph.cpp
@@ -4329,6 +4329,12 @@ bool Compiler::fgMayExplicitTailCall()
         return false;
     }
 
+    if (opts.IsReversePInvoke())
+    {
+        // Reverse P/Invoke
+        return false;
+    }
+
 #if !FEATURE_FIXED_OUT_ARGS
     if (info.compIsVarArgs)
     {
diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp
index 04079ba..331650c 100644
--- a/src/coreclr/src/jit/importer.cpp
+++ b/src/coreclr/src/jit/importer.cpp
@@ -7344,11 +7344,18 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
     // Also, popping arguments in a varargs function is more work and NYI
     // If we have a security object, we have to keep our frame around for callers
     // to see any imperative security.
+    // Reverse P/Invokes need a call to CORINFO_HELP_JIT_REVERSE_PINVOKE_EXIT
+    // at the end, so tailcalls should be disabled.
     if (info.compFlags & CORINFO_FLG_SYNCH)
     {
         canTailCall             = false;
         szCanTailCallFailReason = "Caller is synchronized";
     }
+    else if (opts.IsReversePInvoke())
+    {
+        canTailCall             = false;
+        szCanTailCallFailReason = "Caller is Reverse P/Invoke";
+    }
 #if !FEATURE_FIXED_OUT_ARGS
     else if (info.compIsVarArgs)
     {
@@ -11479,6 +11486,11 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                     BADCODE("Jmp not allowed in protected region");
                 }
 
+                if (opts.IsReversePInvoke())
+                {
+                    BADCODE("Jmp not allowed in reverse P/Invoke");
+                }
+
                 if (verCurrentState.esStackDepth != 0)
                 {
                     BADCODE("Stack must be empty after CEE_JMPs");
diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp
index 838ed6b..f4d5b28 100644
--- a/src/coreclr/src/jit/lower.cpp
+++ b/src/coreclr/src/jit/lower.cpp
@@ -1809,6 +1809,7 @@ void Lowering::LowerFastTailCall(GenTreeCall* call)
     // Most of these checks are already done by importer or fgMorphTailCall().
     // This serves as a double sanity check.
     assert((comp->info.compFlags & CORINFO_FLG_SYNCH) == 0); // tail calls from synchronized methods
+    assert(!comp->opts.IsReversePInvoke());                  // tail calls reverse pinvoke
     assert(!call->IsUnmanaged());                            // tail calls to unamanaged methods
     assert(!comp->compLocallocUsed);                         // tail call from methods that also do localloc
 
diff --git a/src/coreclr/src/jit/patchpoint.cpp b/src/coreclr/src/jit/patchpoint.cpp
index e9aca3b..8527bd9 100644
--- a/src/coreclr/src/jit/patchpoint.cpp
+++ b/src/coreclr/src/jit/patchpoint.cpp
@@ -231,6 +231,12 @@ void Compiler::fgTransformPatchpoints()
         return;
     }
 
+    if (opts.IsReversePInvoke())
+    {
+        JITDUMP(" -- unable to handle Reverse P/Invoke\n");
+        return;
+    }
+
     PatchpointTransformer ppTransformer(this);
     int                   count = ppTransformer.Run();
     JITDUMP("\n*************** After fgTransformPatchpoints() [%d patchpoints transformed]\n", count);
-- 
2.7.4