r211722 - PR20038: DebugInfo: Call sites without DebugLocs for temporary dtors after a conditional

[David Blaikie]

Thanks for letting me know, Reid. The only call in the function is the
dtor anyway, so I just made the match more general so it didn't depend
on the mangled name in r211742. Hopefully that's nice & portable now.

- David

On Wed, Jun 25, 2014 at 4:12 PM, Reid Kleckner <rnk at google.com> wrote:
> This test needs a triple, it's failing locally for me due to the
> Itanium-style names (@_ZN1CD1Ev). -triple %itanium_abi_triple is the usual
> fix.
>
>
> On Wed, Jun 25, 2014 at 10:57 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> Author: dblaikie
>> Date: Wed Jun 25 12:57:34 2014
>> New Revision: 211722
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=211722&view=rev
>> Log:
>> PR20038: DebugInfo: Call sites without DebugLocs for temporary dtors after
>> a conditional
>>
>> With && at the top level of an expression, the last thing done when
>> emitting the expression was an unconditional jump to the cleanup block.
>> To reduce the amount of stepping, the DebugLoc is omitted from the
>> unconditional jump. This is done by clearing the IRBuilder's
>> "CurrentDebugLocation"*. If this is not set to some non-empty value
>> before the cleanup block is emitted, the cleanups don't get a location
>> either. If a call without a location is emitted in a function with debug
>> info, and that call is then inlined - bad things happen. (without a
>> location for the call site, the inliner would just leave the inlined
>> DebugLocs as they were - pointing to roots in the original function, not
>> inlined into the current function)
>>
>> Follow up commit to LLVM will ensure that breaking the invariants of the
>> DebugLoc chains by having chains that don't lead to the current function
>> will fail assertions, so we shouldn't accidentally slip any of these
>> cases in anymore. Those assertions may reveal further cases that need to
>> be fixed in clang, though I've tried to test heavily to avoid that.
>>
>> * See r128471, r128513 for the code that clears the
>>   CurrentDebugLocation. Simply removing this code or moving the code
>>   into IRBuilder to apply to all unconditional branches would regress
>>   desired behavior, unfortunately.
>>
>> Added:
>>     cfe/trunk/test/CodeGenCXX/PR20038.cpp
>> Modified:
>>     cfe/trunk/lib/CodeGen/CGExprScalar.cpp
>>
>> Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=211722&r1=211721&r2=211722&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Wed Jun 25 12:57:34 2014
>> @@ -358,7 +358,10 @@ public:
>>    Value *VisitExprWithCleanups(ExprWithCleanups *E) {
>>      CGF.enterFullExpression(E);
>>      CodeGenFunction::RunCleanupsScope Scope(CGF);
>> -    return Visit(E->getSubExpr());
>> +    auto *V = Visit(E->getSubExpr());
>> +    if (CGDebugInfo *DI = CGF.getDebugInfo())
>> +      DI->EmitLocation(Builder, E->getLocEnd(), false);
>> +    return V;
>>    }
>>    Value *VisitCXXNewExpr(const CXXNewExpr *E) {
>>      return CGF.EmitCXXNewExpr(E);
>>
>> Added: cfe/trunk/test/CodeGenCXX/PR20038.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/PR20038.cpp?rev=211722&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/PR20038.cpp (added)
>> +++ cfe/trunk/test/CodeGenCXX/PR20038.cpp Wed Jun 25 12:57:34 2014
>> @@ -0,0 +1,11 @@
>> +// RUN: %clang_cc1 -g -emit-llvm %s -o - | FileCheck %s
>> +
>> +struct C {
>> +  ~C();
>> +};
>> +extern bool b;
>> +// CHECK: call void @_ZN1CD1Ev({{.*}}), !dbg [[DTOR_CALL_LOC:![0-9]*]]
>> +// CHECK: [[FUN4:.*]] = {{.*}}; [ DW_TAG_subprogram ] {{.*}} [def] [fun4]
>> +// CHECK: [[DTOR_CALL_LOC]] = metadata !{i32 [[@LINE+2]], i32 0, metadata
>> [[FUN4_BLOCK:.*]], null}
>> +// CHECK: [[FUN4_BLOCK]] = metadata !{{{[^,]*}}, {{[^,]*}}, metadata
>> [[FUN4]],
>> +void fun4() { b && (C(), 1); }
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>