Re: [PATCH] Early LTO debug TODO

[Jason Merrill <jason at redhat dot com>]

On Sat, Sep 9, 2017 at 4:00 PM, Jason Merrill <jason@redhat.com> wrote:
> On Thu, Aug 31, 2017 at 4:03 PM, Richard Biener <rguenther@suse.de> wrote:
>> On Thu, 31 Aug 2017, Richard Biener wrote:
>>
>>>
>>> As suspected during review the DECL_ABSTRACT_P handling in
>>> gen_formal_parameter_die is no longer necessary so the following
>>> patch removes it.
>>>
>>> [LTO] bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>>> The gdb testsuite shows no regression.
>>>
>>> Will apply after testing finished.
>>
>> Ok, so it doesn't work - it regresses for example
>> gcc.dg/debug/dwarf2/inline1.c because we end up annotating the
>> abstract DIE with a location.  This is because in gen_subprogram_die
>> (with decl set as abstract-self and thus generating a concrete
>> instance subroutine die) we call
>>
>>           else if (parm && !POINTER_BOUNDS_P (parm))
>>             {
>>               dw_die_ref parm_die = gen_decl_die (parm, NULL, NULL,
>> subr_die);
>>
>> thus unconditionally pass NULL as origin where gen_formal_parameter_die
>> just has
>>
>>   /* If we have a previously generated DIE, use it, unless this is an
>>      concrete instance (origin != NULL), in which case we need a new
>>      DIE with a corresponding DW_AT_abstract_origin.  */
>>   bool reusing_die;
>>   if (parm_die && origin == NULL)
>>     reusing_die = true;
>>   else
>>     {
>>       parm_die = new_die (DW_TAG_formal_parameter, context_die, node);
>>       reusing_die = false;
>>     }
>>
>> but obviously that logic is flawed with us passing origin as NULL...
>>
>> What saved us here is the check whether the existing param_die has
>> the correct parent, if not we didn't re-use it.  OTOH for die_parent
>> == NULL we have the extra check that would have been dead code.
>>
>> Any hint as to whether we should pass in anything as origin or
>> whether we should keep the existing die_parent logic somehow?
>> (do we ever get a NULL context_die passed?)
>
> The problem is that we aren't checking decl_ultimate_origin soon
> enough, and thereby setting origin.
>
> Tested x86_64-pc-linux-gnu, applying to trunk:

...and reverting, as it caused a bunch of GDB regressions.

> FAIL: gdb.cp/anon-struct.exp: print type of t3::~t3
> FAIL: gdb.cp/anon-struct.exp: print type of t::t
> FAIL: gdb.cp/anon-struct.exp: print type of X::t2::t2
> FAIL: gdb.cp/cpexprs.exp: print base1::base1(int)
> FAIL: gdb.cp/cpexprs.exp: print base1::base1(void)
> FAIL: gdb.cp/cpexprs.exp: print base2::base2
> FAIL: gdb.cp/cpexprs.exp: print base::~base
> FAIL: gdb.cp/cpexprs.exp: print base::base(int)
> FAIL: gdb.cp/cpexprs.exp: print base::base(void)
> FAIL: gdb.cp/cpexprs.exp: print derived::derived
> FAIL: gdb.cp/cpexprs.exp: print policy1::policy
> FAIL: gdb.cp/cpexprs.exp: print policy2::policy
> FAIL: gdb.cp/cpexprs.exp: print policy3::policy
> FAIL: gdb.cp/cpexprs.exp: print policy4::policy
> FAIL: gdb.cp/cpexprs.exp: print policyd1::policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd1::~policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd2::policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd2::~policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd3::policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd3::~policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd4::policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd4::~policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd5::policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd5::~policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd<base, operation_1<base> >::policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd<base, operation_1<base> >::~policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd<char, operation_1<char> >::policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd<char, operation_1<char> >::~policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd<int, operation_1<int> >::policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd<int, operation_1<int> >::~policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd<long, operation_1<long> >::policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd<long, operation_1<long> >::~policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd<tclass<int>, operation_1<tclass<int> > >::policyd
> FAIL: gdb.cp/cpexprs.exp: print policyd<tclass<int>, operation_1<tclass<int> > >::~policyd

Jason