From 6d20e9041a49ab368d8d06e8366aa24788dfd1ac Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Sun, 25 Jan 2015 01:19:10 +0000 Subject: [PATCH] DebugInfo: Use the preferred location rather than the start location for expression line info This causes things like assignment to refer to the '=' rather than the LHS when attributing the store instruction, for example. There were essentially 3 options for this: * The beginning of an expression (this was the behavior prior to this commit). This meant that stepping through subexpressions would bounce around from subexpressions back to the start of the outer expression, etc. (eg: x + y + z would go x, y, x, z, x (the repeated 'x's would be where the actual addition occurred)). * The end of an expression. This seems to be what GCC does /mostly/, and certainly this for function calls. This has the advantage that progress is always 'forwards' (never jumping backwards - except for independent subexpressions if they're evaluated in interesting orders, etc). "x + y + z" would go "x y z" with the additions occurring at y and z after the respective loads. The problem with this is that the user would still have to think fairly hard about precedence to realize which subexpression is being evaluated or which operator overload is being called in, say, an asan backtrace. * The preferred location or 'exprloc'. In this case you get sort of what you'd expect, though it's a bit confusing in its own way due to going 'backwards'. In this case the locations would be: "x y + z +" in lovely postfix arithmetic order. But this does mean that if the op+ were an operator overload, say, and in a backtrace, the backtrace will point to the exact '+' that's being called, not to the end of one of its operands. (actually the operator overload case doesn't work yet for other reasons, but that's being fixed - but this at least gets scalar/complex assignments and other plain operators right) git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@227027 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGDebugInfo.cpp | 9 +++++++++ lib/CodeGen/CGDebugInfo.h | 4 ++++ lib/CodeGen/CGExpr.cpp | 4 ++-- lib/CodeGen/CGExprAgg.cpp | 2 +- lib/CodeGen/CGExprCXX.cpp | 2 +- lib/CodeGen/CGExprComplex.cpp | 2 +- lib/CodeGen/CGExprScalar.cpp | 2 +- test/CodeGenCXX/debug-info-line.cpp | 20 ++++++++++++-------- 8 files changed, 31 insertions(+), 14 deletions(-) diff --git a/lib/CodeGen/CGDebugInfo.cpp b/lib/CodeGen/CGDebugInfo.cpp index 17a8f063c5..38a1184bd7 100644 --- a/lib/CodeGen/CGDebugInfo.cpp +++ b/lib/CodeGen/CGDebugInfo.cpp @@ -65,6 +65,10 @@ ArtificialLocation::ArtificialLocation(CodeGenFunction &CGF) ApplyDebugLocation::ApplyDebugLocation(CodeGenFunction &CGF, SourceLocation TemporaryLocation) : CGF(CGF) { + init(TemporaryLocation); +} + +void ApplyDebugLocation::init(SourceLocation TemporaryLocation) { if (auto *DI = CGF.getDebugInfo()) { OriginalLocation = CGF.Builder.getCurrentDebugLocation(); if (TemporaryLocation.isInvalid()) @@ -74,6 +78,11 @@ ApplyDebugLocation::ApplyDebugLocation(CodeGenFunction &CGF, } } +ApplyDebugLocation::ApplyDebugLocation(CodeGenFunction &CGF, const Expr *E) + : CGF(CGF) { + init(E->getExprLoc()); +} + ApplyDebugLocation::ApplyDebugLocation(CodeGenFunction &CGF, llvm::DebugLoc Loc) : CGF(CGF) { if (CGF.getDebugInfo()) { diff --git a/lib/CodeGen/CGDebugInfo.h b/lib/CodeGen/CGDebugInfo.h index bcce900018..8e00c1b048 100644 --- a/lib/CodeGen/CGDebugInfo.h +++ b/lib/CodeGen/CGDebugInfo.h @@ -444,6 +444,9 @@ private: }; class ApplyDebugLocation { +private: + void init(SourceLocation TemporaryLocation); + protected: llvm::DebugLoc OriginalLocation; CodeGenFunction &CGF; @@ -451,6 +454,7 @@ protected: public: ApplyDebugLocation(CodeGenFunction &CGF, SourceLocation TemporaryLocation = SourceLocation()); + ApplyDebugLocation(CodeGenFunction &CGF, const Expr *E); ApplyDebugLocation(CodeGenFunction &CGF, llvm::DebugLoc Loc); ~ApplyDebugLocation(); }; diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index 6fdd110480..3a45d77e2c 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -807,7 +807,7 @@ LValue CodeGenFunction::EmitCheckedLValue(const Expr *E, TypeCheckKind TCK) { /// length type, this is not possible. /// LValue CodeGenFunction::EmitLValue(const Expr *E) { - ApplyDebugLocation DL(*this, E->getLocStart()); + ApplyDebugLocation DL(*this, E); switch (E->getStmtClass()) { default: return EmitUnsupportedLValue(E, "l-value expression"); @@ -3060,7 +3060,7 @@ RValue CodeGenFunction::EmitRValueForField(LValue LV, RValue CodeGenFunction::EmitCallExpr(const CallExpr *E, ReturnValueSlot ReturnValue) { - ApplyDebugLocation DL(*this, E->getLocStart()); + ApplyDebugLocation DL(*this, E); // Builtins never have block type. if (E->getCallee()->getType()->isBlockPointerType()) diff --git a/lib/CodeGen/CGExprAgg.cpp b/lib/CodeGen/CGExprAgg.cpp index 4cba4781af..80b16dd5ba 100644 --- a/lib/CodeGen/CGExprAgg.cpp +++ b/lib/CodeGen/CGExprAgg.cpp @@ -99,7 +99,7 @@ public: //===--------------------------------------------------------------------===// void Visit(Expr *E) { - ApplyDebugLocation DL(CGF, E->getLocStart()); + ApplyDebugLocation DL(CGF, E); StmtVisitor::Visit(E); } diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp index 6d63b3ae9c..cead8a429d 100644 --- a/lib/CodeGen/CGExprCXX.cpp +++ b/lib/CodeGen/CGExprCXX.cpp @@ -1016,7 +1016,7 @@ static void EmitNewInitializer(CodeGenFunction &CGF, const CXXNewExpr *E, llvm::Value *NewPtr, llvm::Value *NumElements, llvm::Value *AllocSizeWithoutCookie) { - ApplyDebugLocation DL(CGF, E->getStartLoc()); + ApplyDebugLocation DL(CGF, E); if (E->isArray()) CGF.EmitNewArrayInitializer(E, ElementType, NewPtr, NumElements, AllocSizeWithoutCookie); diff --git a/lib/CodeGen/CGExprComplex.cpp b/lib/CodeGen/CGExprComplex.cpp index 2cbd42b7ee..b6adbf64f8 100644 --- a/lib/CodeGen/CGExprComplex.cpp +++ b/lib/CodeGen/CGExprComplex.cpp @@ -95,7 +95,7 @@ public: //===--------------------------------------------------------------------===// ComplexPairTy Visit(Expr *E) { - ApplyDebugLocation DL(CGF, E->getLocStart()); + ApplyDebugLocation DL(CGF, E); return StmtVisitor::Visit(E); } diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index a9cbf05da1..2e2821b623 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -196,7 +196,7 @@ public: //===--------------------------------------------------------------------===// Value *Visit(Expr *E) { - ApplyDebugLocation DL(CGF, E->getLocStart()); + ApplyDebugLocation DL(CGF, E); return StmtVisitor::Visit(E); } diff --git a/test/CodeGenCXX/debug-info-line.cpp b/test/CodeGenCXX/debug-info-line.cpp index 87ea576cca..340222eba5 100644 --- a/test/CodeGenCXX/debug-info-line.cpp +++ b/test/CodeGenCXX/debug-info-line.cpp @@ -10,11 +10,11 @@ extern "C" __complex float *complex_sink(); // CHECK-LABEL: define void f1() { -#line 100 - * // The store for the assignment should be attributed to the start of the - // assignment expression here, regardless of the location of subexpressions. - sink() = src(); + *sink() // CHECK: store {{.*}}, !dbg [[DBG_F1:!.*]] +#line 100 + = // + src(); } struct foo { @@ -38,16 +38,20 @@ foo::foo() // CHECK-LABEL: define {{.*}}f2{{.*}} void f2() { + // CHECK: store float {{.*}} !dbg [[DBG_F2:!.*]] + *complex_sink() #line 300 - * // CHECK: store float {{.*}} !dbg [[DBG_F2:!.*]] - complex_sink() = complex_src(); + = // + complex_src(); } // CHECK-LABEL: define void f3() { + // CHECK: store float {{.*}} !dbg [[DBG_F3:!.*]] + *complex_sink() #line 400 - * // CHECK: store float {{.*}} !dbg [[DBG_F3:!.*]] - complex_sink() += complex_src(); + += // + complex_src(); } // CHECK-LABEL: define -- 2.40.0