On Wed, Aug 26, 2015 at 1:34 PM, Richard Biener <rguent...@suse.de> wrote:
>
> The following fixes a GC issue I run into when doing
> prune_unused_types_prune early. The issue is that the DIE struct
> has a chain_circular marked field (die_sib) which cannot tolerate
> spurious extra entries from old removed entries into the circular
> chain. Otherwise we fail to properly mark parts of the chain.
> Those stray entries are kept live referenced from TYPE_SYMTAB_DIE.
>
> So the following patch that makes sure to clear ->die_sib for
> nodes we remove. (these DIEs remaining in TYPE_SYMTAB_DIE also
> means we may end up re-using them which is probably not what we
> want ... in the original LTO experiment I had a ->removed flag
> in the DIE struct and removed DIEs from the cache at cache lookup
> time if I hit a removed DIE)
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, gdb tested there
> as well.
>
> Ok for trunk?
I am re-testing this now and will commit it as part of the piecewise early LTO
merge.
Richard.
> Thanks,
> Richard.
>
> 2015-08-26 Richard Biener <rguent...@suse.de>
>
> * dwarf2out.c (remove_child_with_prev): Clear child->die_sib.
> (replace_child): Likewise.
> (remove_child_TAG): Adjust.
> (move_marked_base_types): Likewise.
> (prune_unused_types_prune): Clear die_sib of removed children.
>
> Index: trunk/gcc/dwarf2out.c
> ===================================================================
> --- trunk.orig/gcc/dwarf2out.c 2015-08-26 09:30:54.679185817 +0200
> +++ trunk/gcc/dwarf2out.c 2015-08-25 16:54:09.150506037 +0200
> @@ -4827,6 +4827,7 @@ remove_child_with_prev (dw_die_ref child
> prev->die_sib = child->die_sib;
> if (child->die_parent->die_child == child)
> child->die_parent->die_child = prev;
> + child->die_sib = NULL;
> }
>
> /* Replace OLD_CHILD with NEW_CHILD. PREV must have the property that
> @@ -4853,6 +4854,7 @@ replace_child (dw_die_ref old_child, dw_
> }
> if (old_child->die_parent->die_child == old_child)
> old_child->die_parent->die_child = new_child;
> + old_child->die_sib = NULL;
> }
>
> /* Move all children from OLD_PARENT to NEW_PARENT. */
> @@ -4883,9 +4885,9 @@ remove_child_TAG (dw_die_ref die, enum d
> remove_child_with_prev (c, prev);
> c->die_parent = NULL;
> /* Might have removed every child. */
> - if (c == c->die_sib)
> + if (die->die_child == NULL)
> return;
> - c = c->die_sib;
> + c = prev->die_sib;
> }
> } while (c != die->die_child);
> }
> @@ -24565,8 +24590,8 @@ prune_unused_types_prune (dw_die_ref die
>
> c = die->die_child;
> do {
> - dw_die_ref prev = c;
> - for (c = c->die_sib; ! c->die_mark; c = c->die_sib)
> + dw_die_ref prev = c, next;
> + for (c = c->die_sib; ! c->die_mark; c = next)
> if (c == die->die_child)
> {
> /* No marked children between 'prev' and the end of the list. */
> @@ -24578,8 +24603,14 @@ prune_unused_types_prune (dw_die_ref die
> prev->die_sib = c->die_sib;
> die->die_child = prev;
> }
> + c->die_sib = NULL;
> return;
> }
> + else
> + {
> + next = c->die_sib;
> + c->die_sib = NULL;
> + }
>
> if (c != prev->die_sib)
> prev->die_sib = c;
> @@ -24824,8 +24855,8 @@ move_marked_base_types (void)
> remove_child_with_prev (c, prev);
> /* As base types got marked, there must be at least
> one node other than DW_TAG_base_type. */
> - gcc_assert (c != c->die_sib);
> - c = c->die_sib;
> + gcc_assert (die->die_child != NULL);
> + c = prev->die_sib;
> }
> }
> while (c != die->die_child);
