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:29:44 PST 2015
> 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?
-- 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