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

[David Blaikie]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150203/29f2e120/attachment.html>