> On Feb 4, 2015, at 5:29 PM, Adrian Prantl <[email protected]> wrote: > > >> 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?
Wait no, sorry. This change looks fine :-) > > -- 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
