> On Feb 4, 2015, at 11:47 AM, David Blaikie <[email protected]> wrote: > > Author: dblaikie > Date: Wed Feb 4 13:47:54 2015 > New Revision: 228181 > > URL: http://llvm.org/viewvc/llvm-project?rev=228181&view=rev > Log: > DebugInfo: Attribute cleanup code to the end of the scope, not the end of the > function. > > Now if you break on a dtor and go 'up' in your debugger (or you get an > asan failure in a dtor) during an exception unwind, you'll have more > context. Instead of all dtors appearing to be called from the '}' of the > function, they'll be attributed to the end of the scope of the variable, > the same as the non-exceptional dtor call. > > This doesn't /quite/ remove all uses of CurEHLocation (which might be > nice to remove, for a few reasons) - it's still used to choose the > location for some other work in the landing pad. It'd be nice to > attribute that code to the same location as the exception calls within > the block and to remove CurEHLocation. > > Modified: > cfe/trunk/lib/CodeGen/CGCleanup.cpp > cfe/trunk/lib/CodeGen/CGDebugInfo.cpp > cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > cfe/trunk/lib/CodeGen/CodeGenFunction.h > cfe/trunk/test/CodeGenCXX/debug-info-line.cpp > cfe/trunk/test/CodeGenCXX/linetable-cleanup.cpp > cfe/trunk/test/CodeGenObjC/arc-linetable.m > > Modified: cfe/trunk/lib/CodeGen/CGCleanup.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCleanup.cpp?rev=228181&r1=228180&r2=228181&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGCleanup.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGCleanup.cpp Wed Feb 4 13:47:54 2015 > @@ -861,8 +861,6 @@ void CodeGenFunction::PopCleanupBlock(bo > > // Emit the EH cleanup if required. > if (RequiresEHCleanup) { > - auto AL = ApplyDebugLocation::CreateDefaultArtificial(*this, > CurEHLocation); > -
You can’t just remove this here: If CurEHLocation is a nullptr ((as it would be in an artificial function), or after removing this - the current location) we still need to set the location to *something* valid or we will create an unattributed call which will after inlining trigger the famous "scope does not describe function” assertion. > CGBuilderTy::InsertPoint SavedIP = Builder.saveAndClearIP(); > > EmitBlock(EHEntry); > > Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=228181&r1=228180&r2=228181&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Feb 4 13:47:54 2015 > @@ -55,7 +55,6 @@ CGDebugInfo::~CGDebugInfo() { > ApplyDebugLocation::ApplyDebugLocation(CodeGenFunction &CGF, > SourceLocation TemporaryLocation) > : CGF(CGF) { > - assert(!TemporaryLocation.isInvalid() && "invalid location”); Please don’t. I’d like to keep a clear separation between the uses of ApplyDebugLocation were a location is mandatory (for example, when we will be emitting a call/invoke) and the ones were it is optional. > init(TemporaryLocation); > } > > > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=228181&r1=228180&r2=228181&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Feb 4 13:47:54 2015 > @@ -241,8 +241,6 @@ void CodeGenFunction::FinishFunction(Sou > // edges will be *really* confused. > bool EmitRetDbgLoc = true; > if (EHStack.stable_begin() != PrologueCleanupDepth) { > - PopCleanupBlocks(PrologueCleanupDepth); > - > // Make sure the line table doesn't jump back into the body for > // the ret after it's been at EndLoc. > EmitRetDbgLoc = false; > @@ -250,6 +248,8 @@ void CodeGenFunction::FinishFunction(Sou > if (CGDebugInfo *DI = getDebugInfo()) > if (OnlySimpleReturnStmts) > DI->EmitLocation(Builder, EndLoc); > + > + PopCleanupBlocks(PrologueCleanupDepth); > } > > // Emit function epilog (to return). > > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=228181&r1=228180&r2=228181&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original) > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Wed Feb 4 13:47:54 2015 > @@ -567,7 +567,10 @@ public: > > // If we should perform a cleanup, force them now. Note that > // this ends the cleanup scope before rescoping any labels. > - if (PerformCleanup) ForceCleanup(); > + if (PerformCleanup) { > + ApplyDebugLocation DL(CGF, Range.getEnd()); > + ForceCleanup(); > + } This also looks super-dangerous for the same reason. What if the last location was empty? -- adrian > } > > /// \brief Force the emission of cleanups now, instead of waiting > > Modified: cfe/trunk/test/CodeGenCXX/debug-info-line.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-line.cpp?rev=228181&r1=228180&r2=228181&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenCXX/debug-info-line.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/debug-info-line.cpp Wed Feb 4 13:47:54 2015 > @@ -259,6 +259,21 @@ void f21() { > f21_b(); > } > > +// CHECK-LABEL: define > +struct f22_dtor { > + ~f22_dtor(); > +}; > +void f22() { > + { > + f22_dtor f; > + src(); > +// CHECK: call {{.*}}src > +// CHECK: call {{.*}}, !dbg [[DBG_F22:![0-9]*]] > +// CHECK: call {{.*}}, !dbg [[DBG_F22]] > +#line 2400 > + } > +} > + > // CHECK: [[DBG_F1]] = !MDLocation(line: 100, > // CHECK: [[DBG_FOO_VALUE]] = !MDLocation(line: 200, > // CHECK: [[DBG_FOO_REF]] = !MDLocation(line: 202, > > Modified: cfe/trunk/test/CodeGenCXX/linetable-cleanup.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/linetable-cleanup.cpp?rev=228181&r1=228180&r2=228181&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenCXX/linetable-cleanup.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/linetable-cleanup.cpp Wed Feb 4 13:47:54 2015 > @@ -4,8 +4,8 @@ > // simple return expressions. > > // CHECK: define {{.*}}foo > -// CHECK: call void @_ZN1CD1Ev(%class.C* {{.*}}), !dbg ![[CLEANUP:[0-9]+]] > -// CHECK: ret i32 0, !dbg ![[RET:[0-9]+]] > +// CHECK: call void @_ZN1CD1Ev(%class.C* {{.*}}), !dbg ![[RET:[0-9]+]] > +// CHECK: ret i32 0, !dbg ![[RET]] > > // CHECK: define {{.*}}bar > // CHECK: ret void, !dbg ![[RETBAR:[0-9]+]] > @@ -23,9 +23,8 @@ int foo() > { > C c; > c.i = 42; > - // This breakpoint should be at/before the cleanup code. > - // CHECK: ![[CLEANUP]] = !MDLocation(line: [[@LINE+1]], scope: !{{.*}}) > return 0; > + // This breakpoint should be at/before the cleanup code. > // CHECK: ![[RET]] = !MDLocation(line: [[@LINE+1]], scope: !{{.*}}) > } > > > Modified: cfe/trunk/test/CodeGenObjC/arc-linetable.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/arc-linetable.m?rev=228181&r1=228180&r2=228181&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenObjC/arc-linetable.m (original) > +++ cfe/trunk/test/CodeGenObjC/arc-linetable.m Wed Feb 4 13:47:54 2015 > @@ -4,8 +4,8 @@ > > // CHECK: define {{.*}}testNoSideEffect > // CHECK: call void @objc_storeStrong{{.*}} > -// CHECK: call void @objc_storeStrong{{.*}} !dbg ![[ARC1:[0-9]+]] > -// CHECK: ret {{.*}} !dbg ![[RET1:[0-9]+]] > +// CHECK: call void @objc_storeStrong{{.*}} !dbg ![[RET1:[0-9]+]] > +// CHECK: ret {{.*}} !dbg ![[RET1]] > > // CHECK: define {{.*}}testNoCleanup > // CHECK: ret {{.*}} !dbg ![[RET2:[0-9]+]] > @@ -21,8 +21,8 @@ > > // CHECK: define {{.*}}testVoid > // CHECK: call void @objc_storeStrong{{.*}} > -// CHECK: call void @objc_storeStrong{{.*}} !dbg ![[ARC5:[0-9]+]] > -// CHECK: ret {{.*}} !dbg ![[RET5:[0-9]+]] > +// CHECK: call void @objc_storeStrong{{.*}} !dbg ![[RET5:[0-9]+]] > +// CHECK: ret {{.*}} !dbg ![[RET5]] > > // CHECK: define {{.*}}testVoidNoReturn > // CHECK: @objc_msgSend{{.*}} !dbg ![[MSG6:[0-9]+]] > @@ -57,9 +57,8 @@ typedef signed char BOOL; > // CHECK: ![[TESTNOSIDEEFFECT:.*]] = {{.*}}[ DW_TAG_subprogram ] [line > [[@LINE+1]]] [local] [def] [-[AppDelegate testNoSideEffect:]] > - (int)testNoSideEffect:(NSString *)foo { > int x = 1; > - // CHECK: ![[ARC1]] = !MDLocation(line: [[@LINE+1]], scope: > ![[TESTNOSIDEEFFECT]]) > return 1; // Return expression > - // CHECK: ![[RET1]] = !MDLocation(line: [[@LINE+1]], scope: !{{.*}}) > + // CHECK: ![[RET1]] = !MDLocation(line: [[@LINE+1]], scope: > ![[TESTNOSIDEEFFECT]]) > } // Cleanup + Ret > > - (int)testNoCleanup { > @@ -82,7 +81,6 @@ typedef signed char BOOL; > } > > - (void)testVoid:(NSString *)foo { > - // CHECK: ![[ARC5]] = !MDLocation(line: [[@LINE+1]], scope: !{{.*}}) > return; > // CHECK: ![[RET5]] = !MDLocation(line: [[@LINE+1]], scope: !{{.*}}) > } > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
