r228053 - DebugInfo: Ensure calls to functions with default arguments which themselves have default arguments, still have locations.

[Hans Wennborg]

Merged in r228078.

Thanks,
Hans

On Tue, Feb 3, 2015 at 2:58 PM, David Blaikie <dblaikie at gmail.com> wrote:
> Another one for 3.6 ( http://llvm.org/bugs/show_bug.cgi?id=22257 again)
>
> On Tue, Feb 3, 2015 at 2:37 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> Author: dblaikie
>> Date: Tue Feb  3 16:37:17 2015
>> New Revision: 228053
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=228053&view=rev
>> Log:
>> DebugInfo: Ensure calls to functions with default arguments which
>> themselves have default arguments, still have locations.
>>
>> To handle default arguments in C++ in the debug info, we disable code
>> updating the debug location during the emission of default arguments.
>>
>> This code was buggy in the case of default arguments which, themselves,
>> have default arguments - the inner default argument would re-enable
>> debug info when it was finished, but before the outer default argument
>> was finished.
>>
>> This was already a bug, but got worse (because a crasher instead of just
>> a quality bug) with the recent improvements to debug info line quality
>> because... The ApplyDebugLocation scoped device would find the debug
>> info disabled and not save any debug location. But then in
>> ~ApplyDebugLocation it would find the debug info had been enabled and
>> would then apply the no-location. Then the outer function call would be
>> emitted without any location. That's bad.
>>
>> Arguably we could /also/ fix the ApplyDebugLocation to assert on this
>> situation (where debug info was disabled in the ctor and enabled in the
>> dtor, or the other way around) but this is at least the necessary fix
>> regardless.
>>
>> (also, I imagine this disabling behavior might need to be in-place for
>> CGExprComplex and CGExprAgg too, maybe... ?)
>>
>> And I seem to recall seeing some weird default arg stepping behavior
>> recently which might be related to this too... I'll have to look into
>> it.
>>
>> Modified:
>>     cfe/trunk/lib/CodeGen/CGExprScalar.cpp
>>     cfe/trunk/test/CodeGenCXX/debug-info-line.cpp
>>
>> Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=228053&r1=228052&r2=228053&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Tue Feb  3 16:37:17 2015
>> @@ -3393,11 +3393,12 @@ Value *CodeGenFunction::EmitScalarExpr(c
>>    assert(E && hasScalarEvaluationKind(E->getType()) &&
>>           "Invalid scalar expression to emit");
>>
>> +  bool hasDebugInfo = getDebugInfo();
>>    if (isa<CXXDefaultArgExpr>(E))
>>      disableDebugInfo();
>>    Value *V = ScalarExprEmitter(*this, IgnoreResultAssign)
>>      .Visit(const_cast<Expr*>(E));
>> -  if (isa<CXXDefaultArgExpr>(E))
>> +  if (isa<CXXDefaultArgExpr>(E) && hasDebugInfo)
>>      enableDebugInfo();
>>    return V;
>>  }
>>
>> 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=228053&r1=228052&r2=228053&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/debug-info-line.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/debug-info-line.cpp Tue Feb  3 16:37:17 2015
>> @@ -250,6 +250,15 @@ void f20(int a, int b, int c) {
>>      ;
>>  }
>>
>> +// CHECK-LABEL: define
>> +int f21_a(int = 0);
>> +void f21_b(int = f21_a());
>> +void f21() {
>> +// CHECK: call {{.*}}f21_b{{.*}}, !dbg [[DBG_F21:![0-9]*]]
>> +#line 2300
>> +  f21_b();
>> +}
>> +
>>  // CHECK: [[DBG_F1]] = !MDLocation(line: 100,
>>  // CHECK: [[DBG_FOO_VALUE]] = !MDLocation(line: 200,
>>  // CHECK: [[DBG_FOO_REF]] = !MDLocation(line: 202,
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>