Looks good.
On Wed, Sep 21, 2016 at 4:37 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> 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);
