From 2431442e6760031935001d82b6ebb4cf2812a57e Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Fri, 10 Jul 2015 07:01:03 +0000 Subject: [PATCH] Address Joseph's review comments. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@241890 91177308-0d34-0410-b5e6-96231b3b80d8 --- docs/LangRef.rst | 20 +++++---- include/llvm/IR/Instructions.h | 4 +- lib/AsmParser/LLParser.cpp | 4 +- lib/IR/Instruction.cpp | 2 + lib/IR/Verifier.cpp | 75 +++++++++++++++++++++++++++++++++- 5 files changed, 92 insertions(+), 13 deletions(-) diff --git a/docs/LangRef.rst b/docs/LangRef.rst index 7db5a277708..86a5a135b28 100644 --- a/docs/LangRef.rst +++ b/docs/LangRef.rst @@ -4730,6 +4730,7 @@ The terminator instructions are: ':ref:`ret `', ':ref:`resume `', ':ref:`catchblock `', ':ref:`catchendblock `', ':ref:`catchret `', +':ref:`cleanupret `', ':ref:`terminateblock `', and ':ref:`unreachable `'. @@ -5143,6 +5144,8 @@ The ``catchblock`` instruction has several restrictions: an exceptional instruction. - A catch block must have a '``catchblock``' instruction as its first non-PHI instruction. +- A catch block's ``exception`` edge must refer to a catch block or a + catch-end block. - There can be only one '``catchblock``' instruction within the catch block. - A basic block that is not a catch block may not include a @@ -5252,15 +5255,17 @@ Semantics: The '``catchret``' instruction ends the existing (in-flight) exception whose unwinding was interrupted with a -:ref:`catchblock ` instruction and transfers control to -``normal``. +:ref:`catchblock ` instruction. +The :ref:`personality function ` gets a chance to execute +arbitrary code to, for example, run a C++ destructor. +Control then transfers to ``normal``. Example: """""""" .. code-block:: llvm - catchret unwind label %continue + catchret label %continue .. _i_cleanupret: @@ -5293,8 +5298,8 @@ Semantics: """""""""" The '``cleanupret``' instruction indicates to the -:ref:`personality function ` that the -:ref:`cleanupblock ` it transfered control to has ended. +:ref:`personality function ` that one +:ref:`cleanupblock ` it transferred control to has ended. It transfers control to ``continue`` or unwinds out of the function. Example: @@ -5327,7 +5332,7 @@ is a terminate block --- one where a personality routine may decide to terminate the program. The ``args`` correspond to whatever information the personality routine requires to know if this is an appropriate place to terminate the -program. Control is tranfered to the ``exception`` label if the +program. Control is transferred to the ``exception`` label if the personality routine decides not to terminate the program for the in-flight exception. @@ -8339,8 +8344,7 @@ transfer control to run cleanup actions. The ``args`` correspond to whatever additional information the :ref:`personality function ` requires to execute the cleanup. -:ref:`personality function ` upon re-entry to the -function. The ``resultval`` has the type ``resultty``. +The ``resultval`` has the type ``resultty``. Arguments: """""""""" diff --git a/include/llvm/IR/Instructions.h b/include/llvm/IR/Instructions.h index 82c9a69838f..60ead82e0e2 100644 --- a/include/llvm/IR/Instructions.h +++ b/include/llvm/IR/Instructions.h @@ -3805,12 +3805,12 @@ public: void setUnwindDest(BasicBlock *B) { Op<-1>() = reinterpret_cast(B); } BasicBlock *getSuccessor(unsigned i) const { - assert(i < 2 && "Successor # out of range for invoke!"); + assert(i < 2 && "Successor # out of range for catchblock!"); return i == 0 ? getNormalDest() : getUnwindDest(); } void setSuccessor(unsigned idx, BasicBlock *NewSucc) { - assert(idx < 2 && "Successor # out of range for invoke!"); + assert(idx < 2 && "Successor # out of range for catchblock!"); *(&Op<-2>() + idx) = reinterpret_cast(NewSucc); } diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp index 4e28410abdc..05373bf05d9 100644 --- a/lib/AsmParser/LLParser.cpp +++ b/lib/AsmParser/LLParser.cpp @@ -4994,7 +4994,7 @@ bool LLParser::ParseTerminateBlock(Instruction *&Inst, PerFunctionState &PFS) { BasicBlock *UnwindBB = nullptr; if (Lex.getKind() == lltok::kw_to) { Lex.Lex(); - if (ParseToken(lltok::kw_caller, "expected 'caller' in cleanupret")) + if (ParseToken(lltok::kw_caller, "expected 'caller' in terminateblock")) return true; } else { if (ParseTypeAndBasicBlock(UnwindBB, PFS)) { @@ -5022,7 +5022,7 @@ bool LLParser::ParseCleanupBlock(Instruction *&Inst, PerFunctionState &PFS) { /// ParseCatchEndBlock /// ::= 'catchendblock' unwind ('to' 'caller' | TypeAndValue) bool LLParser::ParseCatchEndBlock(Instruction *&Inst, PerFunctionState &PFS) { - if (ParseToken(lltok::kw_unwind, "expected 'unwind' in cleanupret")) + if (ParseToken(lltok::kw_unwind, "expected 'unwind' in catchendblock")) return true; BasicBlock *UnwindBB = nullptr; diff --git a/lib/IR/Instruction.cpp b/lib/IR/Instruction.cpp index 124bf089321..2b09e551b79 100644 --- a/lib/IR/Instruction.cpp +++ b/lib/IR/Instruction.cpp @@ -469,6 +469,8 @@ bool Instruction::mayThrow() const { return CRI->unwindsToCaller(); if (const auto *CEBI = dyn_cast(this)) return CEBI->unwindsToCaller(); + if (const auto *TBI = dyn_cast(this)) + return TBI->unwindsToCaller(); return isa(this); } diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index cc7d662d759..6def326a618 100644 --- a/lib/IR/Verifier.cpp +++ b/lib/IR/Verifier.cpp @@ -184,6 +184,9 @@ class Verifier : public InstVisitor, VerifierSupport { /// \brief Track unresolved string-based type references. SmallDenseMap UnresolvedTypeRefs; + /// \brief The result value from the personality function. + Type *PersonalityFnResultTy; + /// \brief Whether we've seen a call to @llvm.localescape in this function /// already. bool SawFrameEscape; @@ -194,7 +197,8 @@ class Verifier : public InstVisitor, VerifierSupport { public: explicit Verifier(raw_ostream &OS) - : VerifierSupport(OS), Context(nullptr), SawFrameEscape(false) {} + : VerifierSupport(OS), Context(nullptr), PersonalityFnResultTy(nullptr), + SawFrameEscape(false) {} bool verify(const Function &F) { M = F.getParent(); @@ -228,6 +232,7 @@ public: // FIXME: We strip const here because the inst visitor strips const. visit(const_cast(F)); InstsInThisBlock.clear(); + PersonalityFnResultTy = nullptr; SawFrameEscape = false; return !Broken; @@ -383,6 +388,7 @@ private: void visitCatchBlockInst(CatchBlockInst &CBI); void visitCatchEndBlockInst(CatchEndBlockInst &CEBI); void visitCleanupBlockInst(CleanupBlockInst &CBI); + void visitCleanupReturnInst(CleanupReturnInst &CRI); void visitTerminateBlockInst(TerminateBlockInst &TBI); void VerifyCallSite(CallSite CS); @@ -2799,6 +2805,14 @@ void Verifier::visitLandingPadInst(LandingPadInst &LPI) { &LPI); } + if (!PersonalityFnResultTy) + PersonalityFnResultTy = LPI.getType(); + else + Assert(PersonalityFnResultTy == LPI.getType(), + "The personality routine should have a consistent result type " + "inside a function.", + &LPI); + Function *F = LPI.getParent()->getParent(); Assert(F->hasPersonalityFn(), "LandingPadInst needs to be in a function with a personality.", &LPI); @@ -2827,6 +2841,14 @@ void Verifier::visitLandingPadInst(LandingPadInst &LPI) { void Verifier::visitCatchBlockInst(CatchBlockInst &CBI) { BasicBlock *BB = CBI.getParent(); + if (!PersonalityFnResultTy) + PersonalityFnResultTy = CBI.getType(); + else + Assert(PersonalityFnResultTy == CBI.getType(), + "The personality routine should have a consistent result type " + "inside a function.", + &CBI); + Function *F = BB->getParent(); Assert(F->hasPersonalityFn(), "CatchBlockInst needs to be in a function with a personality.", &CBI); @@ -2837,6 +2859,12 @@ void Verifier::visitCatchBlockInst(CatchBlockInst &CBI) { "CatchBlockInst not the first non-PHI instruction in the block.", &CBI); + BasicBlock *UnwindDest = CBI.getUnwindDest(); + Instruction *I = UnwindDest->getFirstNonPHI(); + Assert(I->isEHBlock() && !isa(I), + "CatchBlockInst must unwind to an EH block which is not a landingpad.", + &CBI); + visitTerminatorInst(CBI); } @@ -2854,12 +2882,37 @@ void Verifier::visitCatchEndBlockInst(CatchEndBlockInst &CEBI) { "CatchEndBlockInst not the first non-PHI instruction in the block.", &CEBI); + unsigned CatchBlocksSeen = 0; + for (BasicBlock *PredBB : predecessors(BB)) + if (isa(PredBB->getTerminator())) + ++CatchBlocksSeen; + + Assert(CatchBlocksSeen <= 1, "CatchEndBlockInst must have no more than one " + "CatchBlockInst predecessor.", + &CEBI); + + if (BasicBlock *UnwindDest = CEBI.getUnwindDest()) { + Instruction *I = UnwindDest->getFirstNonPHI(); + Assert( + I->isEHBlock() && !isa(I), + "CatchEndBlock must unwind to an EH block which is not a landingpad.", + &CEBI); + } + visitTerminatorInst(CEBI); } void Verifier::visitCleanupBlockInst(CleanupBlockInst &CBI) { BasicBlock *BB = CBI.getParent(); + if (!PersonalityFnResultTy) + PersonalityFnResultTy = CBI.getType(); + else + Assert(PersonalityFnResultTy == CBI.getType(), + "The personality routine should have a consistent result type " + "inside a function.", + &CBI); + Function *F = BB->getParent(); Assert(F->hasPersonalityFn(), "CleanupBlockInst needs to be in a function with a personality.", &CBI); @@ -2873,6 +2926,18 @@ void Verifier::visitCleanupBlockInst(CleanupBlockInst &CBI) { visitInstruction(CBI); } +void Verifier::visitCleanupReturnInst(CleanupReturnInst &CRI) { + if (BasicBlock *UnwindDest = CRI.getUnwindDest()) { + Instruction *I = UnwindDest->getFirstNonPHI(); + Assert(I->isEHBlock() && !isa(I), + "CleanupReturnInst must unwind to an EH block which is not a " + "landingpad.", + &CRI); + } + + visitTerminatorInst(CRI); +} + void Verifier::visitTerminateBlockInst(TerminateBlockInst &TBI) { BasicBlock *BB = TBI.getParent(); @@ -2887,6 +2952,14 @@ void Verifier::visitTerminateBlockInst(TerminateBlockInst &TBI) { "TerminateBlockInst not the first non-PHI instruction in the block.", &TBI); + if (BasicBlock *UnwindDest = TBI.getUnwindDest()) { + Instruction *I = UnwindDest->getFirstNonPHI(); + Assert(I->isEHBlock() && !isa(I), + "TerminateBlockInst must unwind to an EH block which is not a " + "landingpad.", + &TBI); + } + visitTerminatorInst(TBI); } -- 2.34.1