[llvm] bb4a04e - [Verifier][WinEH] Check funclet tokens on intrinsic calls that may lower to function calls

[Stefan Gränitz via llvm-commits]

Author: Stefan Gränitz
Date: 2023-01-27T18:05:13+01:00
New Revision: bb4a04e794aadadc411e7e42a661c6d836d9e8a0

URL: https://github.com/llvm/llvm-project/commit/bb4a04e794aadadc411e7e42a661c6d836d9e8a0
DIFF: https://github.com/llvm/llvm-project/commit/bb4a04e794aadadc411e7e42a661c6d836d9e8a0.diff

LOG: [Verifier][WinEH] Check funclet tokens on intrinsic calls that may lower to function calls

WinEHPrepare requires funclet operand bundles ("tokens") on function calls from EH funclets to prevent them from getting removed as "implausible" calls. This includes calls to intrinsic functions that lower to function calls in the course of IR transformations (e.g. ObjC ARC runtime calls).

We can not detect such cases in WinEHPrepare itself, because at this point they mixed up with valid implausible calls. These must be removed to guarantee that the EH backend can assign unique colors and EH state numbers to all blocks.

This patch allows the IR Verifier to detect missing and dangling funclet tokens. Non-conforming IR becomes illegal and miscompilations are detected early. In order to find funclet pad instructions for funclets that extend over multiple blocks, we have to calculate EH funclet colors. As coloring can be expensive, it runs on-demand and results are cached per function.

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D138123

Added: 
    llvm/test/Verifier/operand-bundles-wineh.ll

Modified: 
    llvm/lib/IR/Verifier.cpp
    llvm/test/Transforms/ObjCARC/invoke-2.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 5d86723c50346..c1bb196bd49a0 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -72,6 +72,7 @@
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/IR/EHPersonalities.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GCStrategy.h"
 #include "llvm/IR/GlobalAlias.h"
@@ -328,6 +329,10 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
   // terminators that indicate the unwind, used to detect cycles therein.
   MapVector<Instruction *, Instruction *> SiblingFuncletInfo;
 
+  /// Cache which blocks are in which funclet, if an EH funclet personality is
+  /// in use. Otherwise empty.
+  DenseMap<BasicBlock *, ColorVector> BlockEHFuncletColors;
+
   /// Cache of constants visited in search of ConstantExprs.
   SmallPtrSet<const Constant *, 32> ConstantExprVisited;
 
@@ -2652,6 +2657,9 @@ void Verifier::visitFunction(const Function &F) {
             F.getParent(), Per, Per->getParent());
   }
 
+  // EH funclet coloring can be expensive, recompute on-demand
+  BlockEHFuncletColors.clear();
+
   if (F.isMaterializable()) {
     // Function has a body somewhere we can't see.
     Check(MDs.empty(), "unmaterialized function cannot have metadata", &F,
@@ -5817,6 +5825,37 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
     break;
   }
   };
+
+  // Verify that there aren't any unmediated control transfers between funclets.
+  if (IntrinsicInst::mayLowerToFunctionCall(ID)) {
+    Function *F = Call.getParent()->getParent();
+    if (F->hasPersonalityFn() &&
+        isScopedEHPersonality(classifyEHPersonality(F->getPersonalityFn()))) {
+      // Run EH funclet coloring on-demand and cache results for other intrinsic
+      // calls in this function
+      if (BlockEHFuncletColors.empty())
+        BlockEHFuncletColors = colorEHFunclets(*F);
+
+      // Check for catch-/cleanup-pad in first funclet block
+      bool InEHFunclet = false;
+      BasicBlock *CallBB = Call.getParent();
+      const ColorVector &CV = BlockEHFuncletColors.find(CallBB)->second;
+      assert(CV.size() > 0 && "Uncolored block");
+      for (BasicBlock *ColorFirstBB : CV)
+        if (dyn_cast_or_null<FuncletPadInst>(ColorFirstBB->getFirstNonPHI()))
+          InEHFunclet = true;
+
+      // Check for funclet operand bundle
+      bool HasToken = false;
+      for (unsigned I = 0, E = Call.getNumOperandBundles(); I != E; ++I)
+        if (Call.getOperandBundleAt(I).getTagID() == LLVMContext::OB_funclet)
+          HasToken = true;
+
+      // This would cause silent code truncation in WinEHPrepare
+      if (InEHFunclet)
+        Check(HasToken, "Missing funclet token on intrinsic call", &Call);
+    }
+  }
 }
 
 /// Carefully grab the subprogram from a local scope.

diff  --git a/llvm/test/Transforms/ObjCARC/invoke-2.ll b/llvm/test/Transforms/ObjCARC/invoke-2.ll
index eda66feb8df66..f1038048ad44c 100644
--- a/llvm/test/Transforms/ObjCARC/invoke-2.ll
+++ b/llvm/test/Transforms/ObjCARC/invoke-2.ll
@@ -39,7 +39,7 @@ catch:                                            ; preds = %catch.dispatch
   %4 = catchpad within %2 [ptr null, i32 0, ptr null]
   %exn.adjusted = tail call ptr @llvm.objc.begin_catch(ptr undef)
   tail call void @llvm.objc.end_catch(), !clang.arc.no_objc_arc_exceptions !0
-  br label %eh.cont
+  catchret from %4 to label %eh.cont
 }
 
 ; CHECK-LABEL: @f

diff  --git a/llvm/test/Verifier/operand-bundles-wineh.ll b/llvm/test/Verifier/operand-bundles-wineh.ll
new file mode 100644
index 0000000000000..0e67b547d7d20
--- /dev/null
+++ b/llvm/test/Verifier/operand-bundles-wineh.ll
@@ -0,0 +1,28 @@
+; RUN: not opt -passes=verify < %s 2>&1 | FileCheck %s
+
+define void @report_missing() personality ptr @__CxxFrameHandler3 {
+entry:
+  invoke void @may_throw() to label %eh.cont unwind label %catch.dispatch
+
+catch.dispatch:
+  %0 = catchswitch within none [label %catch] unwind to caller
+
+catch:
+  %1 = catchpad within %0 [ptr null, i32 0, ptr null]
+  br label %catch.cont
+
+catch.cont:
+; CHECK: Missing funclet token on intrinsic call
+  %2 = call ptr @llvm.objc.retain(ptr null)
+  catchret from %1 to label %eh.cont
+
+eh.cont:
+  ret void
+}
+
+declare void @may_throw()
+declare i32 @__CxxFrameHandler3(...)
+
+declare ptr @llvm.objc.retain(ptr) #0
+
+attributes #0 = { nounwind }