r228181 - DebugInfo: Attribute cleanup code to the end of the scope, not the end of the function.
Adrian Prantl
aprantl at apple.com
Wed Feb 4 17:31:04 PST 2015
> On Feb 4, 2015, at 5:29 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
>
>> On Feb 4, 2015, at 11:47 AM, David Blaikie <dblaikie at gmail.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list