Re: Partitioned LTO bug with cdtor aliases

[Jan Hubicka <hubicka at ucw dot cz>]

> Jan,
> I think I've located a bug in LTO handling.  I'm working on a gcc 5 source
> base, but AFAICT the findings are still current.  Except that on trunk the
> fix for 68881 hides this problem (not confirmed).
> 
> I have a large source base, invoke LTO in partitioned wpa mode, and end up
> with a local alias defined for a destructor, that is therefore not visible
> in the other partitions. (it should be a global hidden symbol)
> 
> In the original TUs we emit a comdat instantiation of a base and complete
> dtor pair.  The comp_dtor is a weak alias to the weakly defined base_dtor.
> So far so good.
> 
> During wpa analysis we first determine prevailing decls.  That's fine. We
> pick base and comp dtor decls from the same comdat group from one of the
> input TUs.
> 
> Then we determine function_and_variable_visibility.  We encounter the
> comp_dtor alias first (not sure if that's an invariant, or just accident of
> test case).  Anyway its symtab node marks it correctly as an
> externally_visible alias in the same comdat group as the base dtor. It has a
> resolution of LDPR_PREVAILING_DEF, and we therefore think
> cgraph_externally_visible_p (node, whole_program) is true and keep it
> externally visible.
> 
> Next we encounter the base dtor.  That has resolution
> LDPR_PREVAILING_DEF_IRONLY, and we do not think it need remain externally
> visible.  We clear node->externally_visible, and then proceed to make it
> local.  During that process we iterate over the members of the same comdat
> group, making them all local.  That encounters the comp_dtor, which we apply
> make_decl_local to.  That clears TREE_PUBLIC on the decl, but leaves its
> node->externally_visible set.  This is inconsistent.

Yep, this looks like a bug.  It is fun to have both LDPR_PREVAILING_DEF and
LDPR_PREVAILING_DEF_IRONLY in one comdat group, but I guess it makes sense.
The code expects that comdat groups always act as unit, but here it doesn't
because only way to make one local and one exported is to dissolve it.

I guess when we encouter the alias first, we could dissolve the whole comdat
group when we see LDPR_PREVAILING_DEF. (we know the symbol is going to stay and
we no longer need a comdat)
> 
> That inconsistency bites us in partitioning, because we then don't promote
> the comp_dtor decl as we think it already visible.  But as its TREE_PUBLIC
> is clear, it gets output as a local symbol in the partition defining it.
> (the inconsistency is harmless in non-partitioned lto mode)
> 
> Not sure if the right fix is to
> a) clear its externally_visible flag too in
> function_and_variable_visibility.  (What if we encounter the nodes in the
> other order?)  [Perhaps the first loop should be split into one setting
> externally_visible as appropriate and then a second loop fixing up mismatch
> between externally_visible and TREE_PUBLIC?]
> 
> b) check in lto_promote_cross_file_statics for both externally_visible &&
> TREE_PUBLIC:
> 	  /* No need to promote if symbol already is externally visible ... */
> 	  if ((node->externally_visible && TREE_PUBLIC(node->decl))
> 
> c) Something else?

I would try the dissolve the comdat group when seeing the alias.
> 
> clues?
> 
> nathan
> -- 
> Nathan Sidwell