[WinEH] Tighten parentPad verifier checks

[JosephTremoulet]

Summary

A catchswitch cannot be a parent of a cleanuppad or another catchswitch.

[majnemer]

LGTM

lib/IR/Verifier.cpp

3022–3023

I guess we can simplify this by writing it as isa<ConstantTokenNone>(ParentPad) || isa<FuncletEHPadInst>(ParentPad).

3080–3081

Ditto.

[JosephTremoulet]

address feedback

lib/IR/Verifier.cpp

void Verifier::visitCleanupPadInst(CleanupPadInst &CPI) {
 
   // The cleanuppad instruction must be the first non-PHI instruction in the
   // block.
   Assert(BB->getFirstNonPHI() == &CPI,
          "CleanupPadInst not the first non-PHI instruction in the block.",
          &CPI);
 
   auto *ParentPad = CPI.getParentPad();
-  Assert(isa<CatchSwitchInst>(ParentPad) || isa<ConstantTokenNone>(ParentPad) ||
-             isa<CleanupPadInst>(ParentPad) || isa<CatchPadInst>(ParentPad),
+  Assert(isa<ConstantTokenNone>(ParentPad) || isa<CleanupPadInst>(ParentPad) ||
+             isa<CatchPadInst>(ParentPad),
          "CleanupPadInst has an invalid parent.", &CPI);
 
   User *FirstUser = nullptr;
   BasicBlock *FirstUnwindDest = nullptr;
   for (User *U : CPI.users()) {
     BasicBlock *UnwindDest;
     if (CleanupReturnInst *CRI = dyn_cast<CleanupReturnInst>(U)) {
       UnwindDest = CRI->getUnwindDest();
if (BasicBlock *UnwindDest = CatchSwitch.getUnwindDest()) {
     Instruction *I = UnwindDest->getFirstNonPHI();
     Assert(I->isEHPad() && !isa<LandingPadInst>(I),
            "CatchSwitchInst must unwind to an EH block which is not a "
            "landingpad.",
            &CatchSwitch);
   }
 
   auto *ParentPad = CatchSwitch.getParentPad();
-  Assert(isa<CatchSwitchInst>(ParentPad) || isa<ConstantTokenNone>(ParentPad) ||
-             isa<CleanupPadInst>(ParentPad) || isa<CatchPadInst>(ParentPad),
+  Assert(isa<ConstantTokenNone>(ParentPad) || isa<CleanupPadInst>(ParentPad) ||
+             isa<CatchPadInst>(ParentPad),
          "CatchSwitchInst has an invalid parent.", ParentPad);
 
   visitTerminatorInst(CatchSwitch);
 }
 
 void Verifier::visitCleanupReturnInst(CleanupReturnInst &CRI) {
   Assert(isa<CleanupPadInst>(CRI.getOperand(0)),
          "CleanupReturnInst needs to be provided a CleanupPad", &CRI,

test/Bitcode/compatibility.ll

 
 catchpad:
   %catch = catchpad within %cs []
   br label %body
   ; CHECK: %catch = catchpad within %cs []
   ; CHECK-NEXT: br label %body
 
 body:
-  invoke void @f.ccc() to label %continue unwind label %terminate
+  invoke void @f.ccc() to label %continue unwind label %terminate.inner
   catchret from %catch to label %return
   ; CHECK: catchret from %catch to label %return
 
 return:
   ret i32 0
 
+terminate.inner:
+  cleanuppad within %catch []
+  unreachable
+  ; CHECK: cleanuppad within %catch []
+  ; CHECK-NEXT: unreachable
+
 terminate:
-  cleanuppad within %cs []
+  cleanuppad within none []
   unreachable
-  ; CHECK: cleanuppad within %cs []
+  ; CHECK: cleanuppad within none []
   ; CHECK-NEXT: unreachable
 
 continue:
   ret i32 0
 }
 
 ; Instructions -- Binary Operations
 define void @instructions.binops(i8 %op1, i8 %op2) {

test/Verifier/invalid-eh.ll

 ; RUN: sed -e s/.T1:// %s | not llvm-as -disable-output 2>&1 | FileCheck --check-prefix=CHECK1 %s
 ; RUN: sed -e s/.T2:// %s | not llvm-as -disable-output 2>&1 | FileCheck --check-prefix=CHECK2 %s
 ; RUN: sed -e s/.T3:// %s | not llvm-as -disable-output 2>&1 | FileCheck --check-prefix=CHECK3 %s
 ; RUN: sed -e s/.T4:// %s | not llvm-as -disable-output 2>&1 | FileCheck --check-prefix=CHECK4 %s
+; RUN: sed -e s/.T5:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK5 %s
+; RUN: sed -e s/.T6:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK6 %s
+; RUN: sed -e s/.T7:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK7 %s
+
+declare void @g()
 
 ;T1: define void @f() {
 ;T1:   entry:
 ;T1:     catchret from undef to label %next
 ;T1:     ; CHECK1: CatchReturnInst needs to be provided a CatchPad
 ;T1:   next:
 ;T1:     unreachable
 ;T1: }

 ;T4:   entry:
 ;T4:     %cs = catchswitch within none [label %next] unwind to caller
 ;T4:   next:
 ;T4:     %x = catchpad within %cs []
 ;T4:     ; cleanupret first operand's operator must be cleanuppad
 ;T4:     cleanupret from %x unwind to caller
 ;T4:     ; CHECK4: CleanupReturnInst needs to be provided a CleanupPad
 ;T4: }
+
+;T5: define void @f() personality void ()* @g {
+;T5:   entry:
+;T5:     ret void
+;T5:   switch:
+;T5:     %cs = catchswitch within none [label %catch] unwind to caller
+;T5:   catch:
+;T5:     catchpad within %cs []
+;T5:     unreachable
+;T5:   bogus:
+;T5:     cleanuppad within %cs []
+;T5:     ; CHECK5: CleanupPadInst has an invalid parent
+;T5:     unreachable
+;T5: }
+
+;T6: define void @f() personality void ()* @g {
+;T6:   entry:
+;T6:     ret void
+;T6:   switch1:
+;T6:     %cs1 = catchswitch within none [label %catch1] unwind label %catch2
+;T6:     ; CHECK6: Block containg CatchPadInst must be jumped to only by its catchswitch
+;T6:   catch1:
+;T6:     catchpad within %cs1 []
+;T6:     unreachable
+;T6:   switch2:
+;T6:     %cs2 = catchswitch within none [label %catch2] unwind to caller
+;T6:   catch2:
+;T6:     catchpad within %cs2 []
+;T6:     unreachable
+;T6: }
+
+;T7: define void @f() personality void ()* @g {
+;T7:   entry:
+;T7:     ret void
+;T7:   switch1:
+;T7:     %cs1 = catchswitch within none [label %catch1] unwind to caller
+;T7:   catch1:
+;T7:     catchpad within %cs1 []
+;T7:     unreachable
+;T7:   switch2:
+;T7:     %cs2 = catchswitch within %cs1 [label %catch2] unwind to caller
+;T7:     ; CHECK7: CatchSwitchInst has an invalid parent
+;T7:   catch2:
+;T7:     catchpad within %cs2 []
+;T7:     unreachable
+;T7: }