[WinEH] Require token linkage in EH pad/ret signatures

Summary:
WinEHPrepare is going to require that cleanuppad and catchpad produce values
of token type which are consumed by any cleanupret or catchret exiting the
pad.  This change updates the signatures of those operators to require/enforce
that the type produced by the pads is token type and that the rets have an
appropriate argument.

The catchpad argument of a `CatchReturnInst` must be a `CatchPadInst` (and
similarly for `CleanupReturnInst`/`CleanupPadInst`).  To accommodate that
restriction, this change adds a notion of an operator constraint to both
LLParser and BitcodeReader, allowing appropriate sentinels to be constructed
for forward references and appropriate error messages to be emitted for
illegal inputs.

Also add a verifier rule (noted in LangRef) that a catchpad with a catchpad
predecessor must have no other predecessors; this ensures that WinEHPrepare
will see the expected linear relationship between sibling catches on the
same try.

Lastly, remove some superfluous/vestigial casts from instruction operand
setters operating on BasicBlocks.

Reviewers: rnk, majnemer

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D12108

llvm-svn: 245797

1 files changed, 100 insertions, 54 deletions

diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index b379e59236d..7ede794bc76 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -41,6 +41,14 @@ enum {
   SWITCH_INST_MAGIC = 0x4B5 // May 2012 => 1205 => Hex
 };
 
+/// Indicates which operator an operand allows (for the few operands that may
+/// only reference a certain operator).
+enum OperatorConstraint {
+  OC_None = 0,  // No constraint
+  OC_CatchPad,  // Must be CatchPadInst
+  OC_CleanupPad // Must be CleanupPadInst
+};
+
 class BitcodeReaderValueList {
   std::vector<WeakVH> ValuePtrs;
 
@@ -84,9 +92,10 @@ public:
   }
 
   Constant *getConstantFwdRef(unsigned Idx, Type *Ty);
-  Value *getValueFwdRef(unsigned Idx, Type *Ty);
+  Value *getValueFwdRef(unsigned Idx, Type *Ty,
+                        OperatorConstraint OC = OC_None);
 
-  void assignValue(Value *V, unsigned Idx);
+  bool assignValue(Value *V, unsigned Idx);
 
   /// Once all constants are read, this method bulk resolves any forward
   /// references.
@@ -262,10 +271,11 @@ private:
   StructType *createIdentifiedStructType(LLVMContext &Context);
 
   Type *getTypeByID(unsigned ID);
-  Value *getFnValueByID(unsigned ID, Type *Ty) {
+  Value *getFnValueByID(unsigned ID, Type *Ty,
+                        OperatorConstraint OC = OC_None) {
     if (Ty && Ty->isMetadataTy())
       return MetadataAsValue::get(Ty->getContext(), getFnMetadataByID(ID));
-    return ValueList.getValueFwdRef(ID, Ty);
+    return ValueList.getValueFwdRef(ID, Ty, OC);
   }
   Metadata *getFnMetadataByID(unsigned ID) {
     return MDValueList.getValueFwdRef(ID);
@@ -308,8 +318,9 @@ private:
   /// past the number of slots used by the value in the record. Return true if
   /// there is an error.
   bool popValue(SmallVectorImpl<uint64_t> &Record, unsigned &Slot,
-                unsigned InstNum, Type *Ty, Value *&ResVal) {
-    if (getValue(Record, Slot, InstNum, Ty, ResVal))
+                unsigned InstNum, Type *Ty, Value *&ResVal,
+                OperatorConstraint OC = OC_None) {
+    if (getValue(Record, Slot, InstNum, Ty, ResVal, OC))
       return true;
     // All values currently take a single record slot.
     ++Slot;
@@ -318,32 +329,34 @@ private:
 
   /// Like popValue, but does not increment the Slot number.
   bool getValue(SmallVectorImpl<uint64_t> &Record, unsigned Slot,
-                unsigned InstNum, Type *Ty, Value *&ResVal) {
-    ResVal = getValue(Record, Slot, InstNum, Ty);
+                unsigned InstNum, Type *Ty, Value *&ResVal,
+                OperatorConstraint OC = OC_None) {
+    ResVal = getValue(Record, Slot, InstNum, Ty, OC);
     return ResVal == nullptr;
   }
 
   /// Version of getValue that returns ResVal directly, or 0 if there is an
   /// error.
   Value *getValue(SmallVectorImpl<uint64_t> &Record, unsigned Slot,
-                  unsigned InstNum, Type *Ty) {
+                  unsigned InstNum, Type *Ty, OperatorConstraint OC = OC_None) {
     if (Slot == Record.size()) return nullptr;
     unsigned ValNo = (unsigned)Record[Slot];
     // Adjust the ValNo, if it was encoded relative to the InstNum.
     if (UseRelativeIDs)
       ValNo = InstNum - ValNo;
-    return getFnValueByID(ValNo, Ty);
+    return getFnValueByID(ValNo, Ty, OC);
   }
 
   /// Like getValue, but decodes signed VBRs.
   Value *getValueSigned(SmallVectorImpl<uint64_t> &Record, unsigned Slot,
-                        unsigned InstNum, Type *Ty) {
+                        unsigned InstNum, Type *Ty,
+                        OperatorConstraint OC = OC_None) {
     if (Slot == Record.size()) return nullptr;
     unsigned ValNo = (unsigned)decodeSignRotatedValue(Record[Slot]);
     // Adjust the ValNo, if it was encoded relative to the InstNum.
     if (UseRelativeIDs)
       ValNo = InstNum - ValNo;
-    return getFnValueByID(ValNo, Ty);
+    return getFnValueByID(ValNo, Ty, OC);
   }
 
   /// Converts alignment exponent (i.e. power of two (or zero)) to the
@@ -753,10 +766,10 @@ struct OperandTraits<ConstantPlaceHolder> :
 DEFINE_TRANSPARENT_OPERAND_ACCESSORS(ConstantPlaceHolder, Value)
 }
 
-void BitcodeReaderValueList::assignValue(Value *V, unsigned Idx) {
+bool BitcodeReaderValueList::assignValue(Value *V, unsigned Idx) {
   if (Idx == size()) {
     push_back(V);
-    return;
+    return false;
   }
 
   if (Idx >= size())
@@ -765,7 +778,7 @@ void BitcodeReaderValueList::assignValue(Value *V, unsigned Idx) {
   WeakVH &OldV = ValuePtrs[Idx];
   if (!OldV) {
     OldV = V;
-    return;
+    return false;
   }
 
   // Handle constants and non-constants (e.g. instrs) differently for
@@ -776,9 +789,26 @@ void BitcodeReaderValueList::assignValue(Value *V, unsigned Idx) {
   } else {
     // If there was a forward reference to this value, replace it.
     Value *PrevVal = OldV;
+    // Check operator constraints.  We only put cleanuppads or catchpads in
+    // the forward value map if the value is constrained to match.
+    if (CatchPadInst *CatchPad = dyn_cast<CatchPadInst>(PrevVal)) {
+      if (!isa<CatchPadInst>(V))
+        return true;
+      // Delete the dummy basic block that was created with the sentinel
+      // catchpad.
+      BasicBlock *DummyBlock = CatchPad->getUnwindDest();
+      assert(DummyBlock == CatchPad->getNormalDest());
+      CatchPad->dropAllReferences();
+      delete DummyBlock;
+    } else if (isa<CleanupPadInst>(PrevVal)) {
+      if (!isa<CleanupPadInst>(V))
+        return true;
+    }
     OldV->replaceAllUsesWith(V);
     delete PrevVal;
   }
+
+  return false;
 }
 
 
@@ -799,7 +829,8 @@ Constant *BitcodeReaderValueList::getConstantFwdRef(unsigned Idx,
   return C;
 }
 
-Value *BitcodeReaderValueList::getValueFwdRef(unsigned Idx, Type *Ty) {
+Value *BitcodeReaderValueList::getValueFwdRef(unsigned Idx, Type *Ty,
+                                              OperatorConstraint OC) {
   // Bail out for a clearly invalid value. This would make us call resize(0)
   if (Idx == UINT_MAX)
     return nullptr;
@@ -811,14 +842,39 @@ Value *BitcodeReaderValueList::getValueFwdRef(unsigned Idx, Type *Ty) {
     // If the types don't match, it's invalid.
     if (Ty && Ty != V->getType())
       return nullptr;
-    return V;
+    if (!OC)
+      return V;
+    // Use dyn_cast to enforce operator constraints
+    switch (OC) {
+    case OC_CatchPad:
+      return dyn_cast<CatchPadInst>(V);
+    case OC_CleanupPad:
+      return dyn_cast<CleanupPadInst>(V);
+    default:
+      llvm_unreachable("Unexpected operator constraint");
+    }
   }
 
   // No type specified, must be invalid reference.
   if (!Ty) return nullptr;
 
   // Create and return a placeholder, which will later be RAUW'd.
-  Value *V = new Argument(Ty);
+  Value *V;
+  switch (OC) {
+  case OC_None:
+    V = new Argument(Ty);
+    break;
+  case OC_CatchPad: {
+    BasicBlock *BB = BasicBlock::Create(Context);
+    V = CatchPadInst::Create(BB, BB, {});
+    break;
+  }
+  default:
+    assert(OC == OC_CleanupPad && "unexpected operator constraint");
+    V = CleanupPadInst::Create(Context, {});
+    break;
+  }
+
   ValuePtrs[Idx] = V;
   return V;
 }
@@ -2610,7 +2666,8 @@ std::error_code BitcodeReader::parseConstants() {
     }
     }
 
-    ValueList.assignValue(V, NextCstNo);
+    if (ValueList.assignValue(V, NextCstNo))
+      return error("Invalid forward reference");
     ++NextCstNo;
   }
 }
@@ -3819,56 +3876,47 @@ std::error_code BitcodeReader::parseFunctionBody(Function *F) {
       }
       break;
     }
-    // CLEANUPRET: [] or [ty,val] or [bb#] or [ty,val,bb#]
+    // CLEANUPRET: [val] or [val,bb#]
     case bitc::FUNC_CODE_INST_CLEANUPRET: {
-      if (Record.size() < 2)
+      if (Record.size() != 1 && Record.size() != 2)
         return error("Invalid record");
       unsigned Idx = 0;
-      bool HasReturnValue = !!Record[Idx++];
-      bool HasUnwindDest = !!Record[Idx++];
-      Value *RetVal = nullptr;
-      BasicBlock *UnwindDest = nullptr;
-
-      if (HasReturnValue && getValueTypePair(Record, Idx, NextValueNo, RetVal))
+      Value *CleanupPad = getValue(Record, Idx++, NextValueNo,
+                                   Type::getTokenTy(Context), OC_CleanupPad);
+      if (!CleanupPad)
         return error("Invalid record");
-      if (HasUnwindDest) {
-        if (Idx == Record.size())
-          return error("Invalid record");
+      BasicBlock *UnwindDest = nullptr;
+      if (Record.size() == 2) {
         UnwindDest = getBasicBlock(Record[Idx++]);
         if (!UnwindDest)
           return error("Invalid record");
       }
 
-      if (Record.size() != Idx)
-        return error("Invalid record");
-
-      I = CleanupReturnInst::Create(Context, RetVal, UnwindDest);
+      I = CleanupReturnInst::Create(cast<CleanupPadInst>(CleanupPad),
+                                    UnwindDest);
       InstructionList.push_back(I);
       break;
     }
-    case bitc::FUNC_CODE_INST_CATCHRET: { // CATCHRET: [bb#]
-      if (Record.size() != 1 && Record.size() != 3)
+    case bitc::FUNC_CODE_INST_CATCHRET: { // CATCHRET: [val,bb#]
+      if (Record.size() != 2)
         return error("Invalid record");
       unsigned Idx = 0;
+      Value *CatchPad = getValue(Record, Idx++, NextValueNo,
+                                 Type::getTokenTy(Context), OC_CatchPad);
+      if (!CatchPad)
+        return error("Invalid record");
       BasicBlock *BB = getBasicBlock(Record[Idx++]);
       if (!BB)
         return error("Invalid record");
-      Value *RetVal = nullptr;
-      if (Record.size() == 3 &&
-          getValueTypePair(Record, Idx, NextValueNo, RetVal))
-        return error("Invalid record");
 
-      I = CatchReturnInst::Create(BB, RetVal);
+      I = CatchReturnInst::Create(cast<CatchPadInst>(CatchPad), BB);
       InstructionList.push_back(I);
       break;
     }
-    case bitc::FUNC_CODE_INST_CATCHPAD: { // CATCHPAD: [ty,bb#,bb#,num,(ty,val)*]
-      if (Record.size() < 4)
+    case bitc::FUNC_CODE_INST_CATCHPAD: { // CATCHPAD: [bb#,bb#,num,(ty,val)*]
+      if (Record.size() < 3)
         return error("Invalid record");
       unsigned Idx = 0;
-      Type *Ty = getTypeByID(Record[Idx++]);
-      if (!Ty)
-        return error("Invalid record");
       BasicBlock *NormalBB = getBasicBlock(Record[Idx++]);
       if (!NormalBB)
         return error("Invalid record");
@@ -3886,7 +3934,7 @@ std::error_code BitcodeReader::parseFunctionBody(Function *F) {
       if (Record.size() != Idx)
         return error("Invalid record");
 
-      I = CatchPadInst::Create(Ty, NormalBB, UnwindBB, Args);
+      I = CatchPadInst::Create(NormalBB, UnwindBB, Args);
       InstructionList.push_back(I);
       break;
     }
@@ -3918,13 +3966,10 @@ std::error_code BitcodeReader::parseFunctionBody(Function *F) {
       InstructionList.push_back(I);
       break;
     }
-    case bitc::FUNC_CODE_INST_CLEANUPPAD: { // CLEANUPPAD: [ty, num,(ty,val)*]
-      if (Record.size() < 2)
+    case bitc::FUNC_CODE_INST_CLEANUPPAD: { // CLEANUPPAD: [num,(ty,val)*]
+      if (Record.size() < 1)
         return error("Invalid record");
       unsigned Idx = 0;
-      Type *Ty = getTypeByID(Record[Idx++]);
-      if (!Ty)
-        return error("Invalid record");
       unsigned NumArgOperands = Record[Idx++];
       SmallVector<Value *, 2> Args;
       for (unsigned Op = 0; Op != NumArgOperands; ++Op) {
@@ -3936,7 +3981,7 @@ std::error_code BitcodeReader::parseFunctionBody(Function *F) {
       if (Record.size() != Idx)
         return error("Invalid record");
 
-      I = CleanupPadInst::Create(Ty, Args);
+      I = CleanupPadInst::Create(Context, Args);
       InstructionList.push_back(I);
       break;
     }
@@ -4541,7 +4586,8 @@ std::error_code BitcodeReader::parseFunctionBody(Function *F) {
 
     // Non-void values get registered in the value table for future use.
     if (I && !I->getType()->isVoidTy())
-      ValueList.assignValue(I, NextValueNo++);
+      if (ValueList.assignValue(I, NextValueNo++))
+        return error("Invalid forward reference");
   }
 
 OutOfRecordLoop: