From 3ed5b8f439117dcec2fd6bb4ec5bebf0923c4536 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Tue, 3 Feb 2015 18:40:42 +0000 Subject: [PATCH] Merge ArtificialLocation into ApplyDebugLocation and make a clear distinction between the different use-cases. With the previous default behavior we would occasionally emit empty debug locations in situations where they actually were strictly required (= on invoke insns). We now have a choice between defaulting to an empty location or an artificial location. Specifically, this fixes a bug caused by a missing debug location when emitting C++ EH cleanup blocks from within an artificial function, such as an ObjC destroy helper function. rdar://problem/19670595 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@228003 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGBlocks.cpp | 12 ++++---- lib/CodeGen/CGCleanup.cpp | 2 +- lib/CodeGen/CGDebugInfo.cpp | 32 ++++++++++++-------- lib/CodeGen/CGDebugInfo.h | 40 ++++++++++++------------- lib/CodeGen/CGDecl.cpp | 2 +- lib/CodeGen/CGDeclCXX.cpp | 8 ++--- lib/CodeGen/CGException.cpp | 2 +- lib/CodeGen/CGExprScalar.cpp | 2 +- lib/CodeGen/CGStmt.cpp | 8 ++--- lib/CodeGen/CGStmtOpenMP.cpp | 4 +-- test/CodeGenObjCXX/nested-ehlocation.mm | 24 +++++++++++++++ 11 files changed, 82 insertions(+), 54 deletions(-) create mode 100644 test/CodeGenObjCXX/nested-ehlocation.mm diff --git a/lib/CodeGen/CGBlocks.cpp b/lib/CodeGen/CGBlocks.cpp index b98460a9dd..ffc6e57b07 100644 --- a/lib/CodeGen/CGBlocks.cpp +++ b/lib/CodeGen/CGBlocks.cpp @@ -1178,7 +1178,7 @@ CodeGenFunction::GenerateBlockFunction(GlobalDecl GD, Alloca->setAlignment(Align); // Set the DebugLocation to empty, so the store is recognized as a // frame setup instruction by llvm::DwarfDebug::beginFunction(). - ApplyDebugLocation NL(*this); + ApplyDebugLocation NL(*this, ApplyDebugLocation::MarkAsPrologue); Builder.CreateAlignedStore(BlockPointer, Alloca, Align); BlockPointerDbgLoc = Alloca; } @@ -1328,10 +1328,10 @@ CodeGenFunction::GenerateCopyHelperFunction(const CGBlockInfo &blockInfo) { nullptr, SC_Static, false, false); - // Create a scope with an artificial location for the body of this function. - ApplyDebugLocation NL(*this); + ApplyDebugLocation NL(*this, ApplyDebugLocation::MarkAsPrologue); StartFunction(FD, C.VoidTy, Fn, FI, args); - ArtificialLocation AL(*this); + // Create a scope with an artificial location for the body of this function. + ApplyDebugLocation AL(*this, ApplyDebugLocation::Artificial); llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo(); @@ -1500,9 +1500,9 @@ CodeGenFunction::GenerateDestroyHelperFunction(const CGBlockInfo &blockInfo) { nullptr, SC_Static, false, false); // Create a scope with an artificial location for the body of this function. - ApplyDebugLocation NL(*this); + ApplyDebugLocation NL(*this, ApplyDebugLocation::MarkAsPrologue); StartFunction(FD, C.VoidTy, Fn, FI, args); - ArtificialLocation AL(*this); + ApplyDebugLocation AL(*this, ApplyDebugLocation::Artificial); llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo(); diff --git a/lib/CodeGen/CGCleanup.cpp b/lib/CodeGen/CGCleanup.cpp index 18ed3e543d..a75a7af453 100644 --- a/lib/CodeGen/CGCleanup.cpp +++ b/lib/CodeGen/CGCleanup.cpp @@ -861,7 +861,7 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) { // Emit the EH cleanup if required. if (RequiresEHCleanup) { - ApplyDebugLocation AutoRestoreLocation(*this, CurEHLocation); + ApplyDebugLocation AL(*this, ApplyDebugLocation::Artificial, CurEHLocation); CGBuilderTy::InsertPoint SavedIP = Builder.saveAndClearIP(); diff --git a/lib/CodeGen/CGDebugInfo.cpp b/lib/CodeGen/CGDebugInfo.cpp index 38a1184bd7..3fb839369d 100644 --- a/lib/CodeGen/CGDebugInfo.cpp +++ b/lib/CodeGen/CGDebugInfo.cpp @@ -52,28 +52,34 @@ CGDebugInfo::~CGDebugInfo() { "Region stack mismatch, stack not empty!"); } -ArtificialLocation::ArtificialLocation(CodeGenFunction &CGF) - : ApplyDebugLocation(CGF) { - if (auto *DI = CGF.getDebugInfo()) { - // Construct a location that has a valid scope, but no line info. - assert(!DI->LexicalBlockStack.empty()); - llvm::DIDescriptor Scope(DI->LexicalBlockStack.back()); - CGF.Builder.SetCurrentDebugLocation(llvm::DebugLoc::get(0, 0, Scope)); - } +ApplyDebugLocation::ApplyDebugLocation(CodeGenFunction &CGF, + SourceLocation TemporaryLocation) + : CGF(CGF) { + assert(!TemporaryLocation.isInvalid() && "invalid location"); + init(TemporaryLocation); } ApplyDebugLocation::ApplyDebugLocation(CodeGenFunction &CGF, + bool MarkAsPrologue, SourceLocation TemporaryLocation) : CGF(CGF) { - init(TemporaryLocation); + init(TemporaryLocation, MarkAsPrologue); } -void ApplyDebugLocation::init(SourceLocation TemporaryLocation) { +void ApplyDebugLocation::init(SourceLocation TemporaryLocation, + bool MarkAsPrologue) { if (auto *DI = CGF.getDebugInfo()) { OriginalLocation = CGF.Builder.getCurrentDebugLocation(); - if (TemporaryLocation.isInvalid()) - CGF.Builder.SetCurrentDebugLocation(llvm::DebugLoc()); - else + if (TemporaryLocation.isInvalid()) { + if (MarkAsPrologue) + CGF.Builder.SetCurrentDebugLocation(llvm::DebugLoc()); + else { + // Construct a location that has a valid scope, but no line info. + assert(!DI->LexicalBlockStack.empty()); + llvm::DIDescriptor Scope(DI->LexicalBlockStack.back()); + CGF.Builder.SetCurrentDebugLocation(llvm::DebugLoc::get(0, 0, Scope)); + } + } else DI->EmitLocation(CGF.Builder, TemporaryLocation); } } diff --git a/lib/CodeGen/CGDebugInfo.h b/lib/CodeGen/CGDebugInfo.h index e3d672465c..2f0c3f51d4 100644 --- a/lib/CodeGen/CGDebugInfo.h +++ b/lib/CodeGen/CGDebugInfo.h @@ -47,7 +47,7 @@ namespace CodeGen { /// and is responsible for emitting to llvm globals or pass directly to /// the backend. class CGDebugInfo { - friend class ArtificialLocation; + friend class ApplyDebugLocation; friend class SaveAndRestoreLocation; CodeGenModule &CGM; const CodeGenOptions::DebugInfoKind DebugKind; @@ -447,38 +447,36 @@ private: /// location or preferred location of the specified Expr. class ApplyDebugLocation { private: - void init(SourceLocation TemporaryLocation); + void init(SourceLocation TemporaryLocation, bool MarkAsPrologue = false); protected: llvm::DebugLoc OriginalLocation; CodeGenFunction &CGF; public: - /// If TemporaryLocation is invalid, the IRBuilder will be set to not attach - /// debug locations, thus marking the instructions as prologue. - ApplyDebugLocation(CodeGenFunction &CGF, + enum { Artificial = false, MarkAsPrologue = true, NoLocation = true }; + + /// \brief Set the location to the (valid) TemporaryLocation. + ApplyDebugLocation(CodeGenFunction &CGF, SourceLocation TemporaryLocation); + /// \brief Apply TemporaryLocation if it is valid, or apply a default + /// location: If MarkAsPrologue is true, the IRBuilder will be set to not + /// attach debug locations, thus marking the instructions as + /// prologue. Otherwise this switches to an artificial debug location that has + /// a valid scope, but no line information. + /// + /// Artificial locations are useful when emitting compiler-generated helper + /// functions that have no source location associated with them. The DWARF + /// specification allows the compiler to use the special line number 0 to + /// indicate code that can not be attributed to any source location. Note that + /// passing an empty SourceLocation to CGDebugInfo::setLocation() will result + /// in the last valid location being reused. + ApplyDebugLocation(CodeGenFunction &CGF, bool MarkAsPrologue, SourceLocation TemporaryLocation = SourceLocation()); ApplyDebugLocation(CodeGenFunction &CGF, const Expr *E); ApplyDebugLocation(CodeGenFunction &CGF, llvm::DebugLoc Loc); ~ApplyDebugLocation(); }; -/// \brief An RAII object that temporarily switches to -/// an artificial debug location that has a valid scope, but no line -/// information. This is useful when emitting compiler-generated -/// helper functions that have no source location associated with -/// them. The DWARF specification allows the compiler to use the -/// special line number 0 to indicate code that can not be attributed -/// to any source location. -/// -/// This is necessary because passing an empty SourceLocation to -/// CGDebugInfo::setLocation() will result in the last valid location -/// being reused. -class ArtificialLocation : public ApplyDebugLocation { -public: - ArtificialLocation(CodeGenFunction &CGF); -}; - } // namespace CodeGen } // namespace clang diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp index 766d2aa6ff..bada6ea651 100644 --- a/lib/CodeGen/CGDecl.cpp +++ b/lib/CodeGen/CGDecl.cpp @@ -1090,7 +1090,7 @@ void CodeGenFunction::EmitAutoVarInit(const AutoVarEmission &emission) { if (emission.wasEmittedAsGlobal()) return; const VarDecl &D = *emission.Variable; - ApplyDebugLocation DL(*this, D.getLocation()); + ApplyDebugLocation DL(*this, ApplyDebugLocation::Artificial, D.getLocation()); QualType type = D.getType(); // If this local has an initializer, emit it now. diff --git a/lib/CodeGen/CGDeclCXX.cpp b/lib/CodeGen/CGDeclCXX.cpp index 3b379b7d25..6303db6000 100644 --- a/lib/CodeGen/CGDeclCXX.cpp +++ b/lib/CodeGen/CGDeclCXX.cpp @@ -469,11 +469,11 @@ CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn, ArrayRef Decls, llvm::GlobalVariable *Guard) { { - ApplyDebugLocation NL(*this); + ApplyDebugLocation NL(*this, ApplyDebugLocation::MarkAsPrologue); StartFunction(GlobalDecl(), getContext().VoidTy, Fn, getTypes().arrangeNullaryFunction(), FunctionArgList()); // Emit an artificial location for this function. - ArtificialLocation AL(*this); + ApplyDebugLocation AL(*this, ApplyDebugLocation::Artificial); llvm::BasicBlock *ExitBlock = nullptr; if (Guard) { @@ -520,11 +520,11 @@ void CodeGenFunction::GenerateCXXGlobalDtorsFunc(llvm::Function *Fn, const std::vector > &DtorsAndObjects) { { - ApplyDebugLocation NL(*this); + ApplyDebugLocation NL(*this, ApplyDebugLocation::MarkAsPrologue); StartFunction(GlobalDecl(), getContext().VoidTy, Fn, getTypes().arrangeNullaryFunction(), FunctionArgList()); // Emit an artificial location for this function. - ArtificialLocation AL(*this); + ApplyDebugLocation AL(*this, ApplyDebugLocation::Artificial); // Emit the dtors, in reverse order from construction. for (unsigned i = 0, e = DtorsAndObjects.size(); i != e; ++i) { diff --git a/lib/CodeGen/CGException.cpp b/lib/CodeGen/CGException.cpp index 9f886ebb13..406afb2739 100644 --- a/lib/CodeGen/CGException.cpp +++ b/lib/CodeGen/CGException.cpp @@ -770,7 +770,7 @@ llvm::BasicBlock *CodeGenFunction::EmitLandingPad() { // Save the current IR generation state. CGBuilderTy::InsertPoint savedIP = Builder.saveAndClearIP(); - ApplyDebugLocation AutoRestoreLocation(*this, CurEHLocation); + ApplyDebugLocation AL(*this, ApplyDebugLocation::Artificial, CurEHLocation); const EHPersonality &personality = EHPersonality::get(CGM); diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index 2e2821b623..503f9b1719 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -3043,7 +3043,7 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { // Emit an unconditional branch from this block to ContBlock. { // There is no need to emit line number for unconditional branch. - ApplyDebugLocation DL(CGF); + ApplyDebugLocation NL(CGF, ApplyDebugLocation::NoLocation); CGF.EmitBlock(ContBlock); } // Insert an entry into the phi node for the edge with the value of RHSCond. diff --git a/lib/CodeGen/CGStmt.cpp b/lib/CodeGen/CGStmt.cpp index e3bdf8661e..be2ddf6733 100644 --- a/lib/CodeGen/CGStmt.cpp +++ b/lib/CodeGen/CGStmt.cpp @@ -564,8 +564,8 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) { // Emit the 'else' code if present. if (const Stmt *Else = S.getElse()) { { - // There is no need to emit line number for unconditional branch. - ApplyDebugLocation DL(*this); + // There is no need to emit line number for an unconditional branch. + ApplyDebugLocation NL(*this, ApplyDebugLocation::NoLocation); EmitBlock(ElseBlock); } { @@ -573,8 +573,8 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) { EmitStmt(Else); } { - // There is no need to emit line number for unconditional branch. - ApplyDebugLocation DL(*this); + // There is no need to emit line number for an unconditional branch. + ApplyDebugLocation NL(*this, ApplyDebugLocation::NoLocation); EmitBranch(ContBlock); } } diff --git a/lib/CodeGen/CGStmtOpenMP.cpp b/lib/CodeGen/CGStmtOpenMP.cpp index 60958d071c..395f6fa69a 100644 --- a/lib/CodeGen/CGStmtOpenMP.cpp +++ b/lib/CodeGen/CGStmtOpenMP.cpp @@ -86,13 +86,13 @@ static void EmitOMPIfClause(CodeGenFunction &CGF, const Expr *Cond, // Emit the 'else' code if present. { // There is no need to emit line number for unconditional branch. - ApplyDebugLocation DL(CGF); + ApplyDebugLocation NL(CGF, ApplyDebugLocation::NoLocation); CGF.EmitBlock(ElseBlock); } CodeGen(/*ThenBlock*/ false); { // There is no need to emit line number for unconditional branch. - ApplyDebugLocation DL(CGF); + ApplyDebugLocation NL(CGF, ApplyDebugLocation::NoLocation); CGF.EmitBranch(ContBlock); } // Emit the continuation block for code after the if. diff --git a/test/CodeGenObjCXX/nested-ehlocation.mm b/test/CodeGenObjCXX/nested-ehlocation.mm new file mode 100644 index 0000000000..de3e359754 --- /dev/null +++ b/test/CodeGenObjCXX/nested-ehlocation.mm @@ -0,0 +1,24 @@ +// RUN: %clang_cc1 -triple x86_64-apple-macosx -emit-llvm -g -stdlib=libc++ -fblocks -fexceptions -x objective-c++ -o - %s | FileCheck %s + +// Verify that all invoke instructions have a debug location. +// Literally: There are no unwind lines that don't end with ", (!dbg 123)". +// CHECK-NOT: {{to label %.* unwind label [^,]+$}} + +void block(void (^)(void)); +extern void foo(); +struct A { + ~A(void) { foo(); } + void bar() const {} +}; +void baz(void const *const) {} +struct B : A {}; +void test() { + A a; + B b; + block(^(void) { + baz(&b); + block(^() { + a.bar(); + }); + }); +} -- 2.40.0