On Sat, Sep 9, 2017 at 4:00 PM, Jason Merrill <ja...@redhat.com> wrote: > On Thu, Aug 31, 2017 at 4:03 PM, Richard Biener <rguent...@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