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:
commit b7702a2cbca9c50e94053cc125ff93438d377126
Author: Jason Merrill <ja...@redhat.com>
Date: Sat Sep 9 11:33:14 2017 +0200
* dwarf2out.c (gen_formal_parameter_die): Remove obsolete hunk.
Check ultimate_origin before setting reusing_die.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 651dd0c..cc93db3 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -21285,30 +21285,15 @@ gen_formal_parameter_die (tree node, tree origin,
bool emit_name_p,
dw_die_ref context_die)
{
tree node_or_origin = node ? node : origin;
- tree ultimate_origin;
dw_die_ref parm_die = NULL;
- if (TREE_CODE_CLASS (TREE_CODE (node_or_origin)) == tcc_declaration)
+ if (DECL_P (node_or_origin))
{
parm_die = lookup_decl_die (node);
- /* If the contexts differ, we may not be talking about the same
- thing.
- ??? When in LTO the DIE parent is the "abstract" copy and the
- context_die is the specification "copy". But this whole block
- should eventually be no longer needed. */
- if (parm_die && parm_die->die_parent != context_die && !in_lto_p)
- {
- if (!DECL_ABSTRACT_P (node))
- {
- /* This can happen when creating an inlined instance, in
- which case we need to create a new DIE that will get
- annotated with DW_AT_abstract_origin. */
- parm_die = NULL;
- }
- else
- gcc_unreachable ();
- }
+ tree ultimate_origin = decl_ultimate_origin (node_or_origin);
+ if (node || ultimate_origin)
+ origin = ultimate_origin;
if (parm_die && parm_die->die_parent == NULL)
{
@@ -21343,10 +21328,6 @@ gen_formal_parameter_die (tree node, tree origin, bool
emit_name_p,
switch (TREE_CODE_CLASS (TREE_CODE (node_or_origin)))
{
case tcc_declaration:
- ultimate_origin = decl_ultimate_origin (node_or_origin);
- if (node || ultimate_origin)
- origin = ultimate_origin;
-
if (reusing_die)
goto add_location;
