> honza,
> this is the fix for the partitioned WPA bug I was tracking down.
>
> We have base and complete dtors sharing a comdat group (one's an alias for
> the other). The linker tells us the complete dtor is PREVAILING_DEF, as
> it's referenced from some other library. The base dtor is UNKNOWN.
>
> We therefore internalize the base dtor, making it PREVAILING_DEF_IRONLY and
> split the comdat group. But the comdat group splitting also internalizes
> the complete dtor /and/ it does so inconsistently.
>
> The bug manifested at runtime w/o link error as the complete dtor resolved
> to zero in the final link from wpa partitions that didn't contain its
> definition. (For extra fun, that was via a call to __cxa_at_exit registering
> a null function pointer, and getting the subsequent seg fault at program
> exit.) When we created WPA partitions node->externally_visible was still
> set, so we thought the now-internalized complete dtor was still externally
> visible -- but varasm looks at the DECL itself and emits an internal one.
> Plus the references to it were weak (& now hidden), so resolved to zero,
> rather than link error. And the external library either had its own
> definition which then prevailed for it. All rather 'ew'.
>
> Anyway, this patch does 3 things
> 1) Moves the next->unique_name adjustment to before make_decl_local for
> members of the comdat group -- that matches the behaviour of the decl of
> interest itself.
That part is OK.
>
> 2) For LDPR_PREVAILING_DEF members we don't make_decl_local, but instead
> clear DECL_COMDAT and DECL_WEAK. Thus forcing this decl to be the
> prevailing decl in the final link
>
> 3) For decls we localize, we also clear node->externally_visible and
> node->force_by_abi. That matches the behavior for the decl of interest too
> and will clue the wpa partitioning logic into knowing it needs to
> hidden-externalize the decl.
So at the moment it works in a way
1) we walk first symbol of the comdat and it is LDPR_PREVAILING_DEF and thus
we set externall visible flag
2) we walk second symbol of the comdat and it is LDPR_PREVAILING_DEF_IRONLY
and thus we decide to privatize the whole comdat group, during this
process we force the first symbol local and clear the externally_visible
flag?
I think at a time we decide on external visibility of a symbol in a comdat
group, we need to check that the comdat group as a whole is not exporte (i.e.
no LDPR_PREVAILING_DEF_EXP or incremental linking). Then we know we can
dissolve
the comdat group (without actually affecting visibility) and then we can
handle each symbol independently.
Honza
>
> nathan
>
> --
> Nathan Sidwell
> 2017-01-18 Nathan Sidwell <nat...@acm.org>
>
> * ipa-visibility.c (localize_node): Set comdat's unique name
> before adjusting resolution. Make PREVAILING_DEF members strongly
> public. Set visibility to false for localized decls.
>
> Index: ipa-visibility.c
> ===================================================================
> --- ipa-visibility.c (revision 244546)
> +++ ipa-visibility.c (working copy)
> @@ -542,16 +542,32 @@ localize_node (bool whole_program, symta
> for (symtab_node *next = node->same_comdat_group;
> next != node; next = next->same_comdat_group)
> {
> - next->set_comdat_group (NULL);
> - if (!next->alias)
> - next->set_section (NULL);
> - if (!next->transparent_alias)
> - next->make_decl_local ();
> next->unique_name
> |= ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
> || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> && TREE_PUBLIC (next->decl)
> && !flag_incremental_link);
> +
> + next->set_comdat_group (NULL);
> + if (!next->alias)
> + next->set_section (NULL);
> + if (next->transparent_alias)
> + /* Do nothing. */;
> + else if (next->resolution == LDPR_PREVAILING_DEF)
> + {
> + /* Make this a strong defn, so the external
> + users don't mistakenly choose some other
> + instance. */
> + DECL_COMDAT (next->decl) = false;
> + DECL_WEAK (next->decl) = false;
> + }
> + else
> + {
> + next->externally_visible = false;
> + next->forced_by_abi = false;
> + next->resolution = LDPR_PREVAILING_DEF_IRONLY;
> + next->make_decl_local ();
> + }
> }
>
> /* Now everything's localized, the grouping has no meaning, and
