> 2013-06-11 Jakub Jelinek <ja...@redhat.com>
>
> PR target/56564
> * varasm.c (decl_binds_to_current_def_p): Call binds_local_p
> target hook even for !TREE_PUBLIC decls. If no resolution info
> is available, return false for common and external decls.
>
> --- gcc/varasm.c.jj 2013-06-11 20:11:34.000000000 +0200
> +++ gcc/varasm.c 2013-06-11 20:15:15.729363893 +0200
> @@ -6781,10 +6781,10 @@ bool
> decl_binds_to_current_def_p (tree decl)
> {
> gcc_assert (DECL_P (decl));
> - if (!TREE_PUBLIC (decl))
> - return true;
> if (!targetm.binds_local_p (decl))
> return false;
> + if (!TREE_PUBLIC (decl))
> + return true;
> /* When resolution is available, just use it. */
> if (TREE_CODE (decl) == VAR_DECL
> && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)))
> @@ -6802,10 +6802,20 @@ decl_binds_to_current_def_p (tree decl)
> return resolution_to_local_definition_p (node->symbol.resolution);
> }
> /* Otherwise we have to assume the worst for DECL_WEAK (hidden weaks
> - binds locally but still can be overwritten).
> + binds locally but still can be overwritten), DECL_COMMON (can be merged
> + with a non-common definition somewhere in the same module) or
> + DECL_EXTERNAL.
> This rely on fact that binds_local_p behave as decl_replaceable_p
> for all other declaration types. */
> - return !DECL_WEAK (decl);
> + if (DECL_WEAK (decl))
> + return false;
> + if (DECL_COMMON (decl)
> + && (DECL_INITIAL (decl) == NULL
> + || DECL_INITIAL (decl) == error_mark_node))
> + return false;
As discussed on IRC, this will return unnecesarily conservative answer for
HIDDEN visibility and (resolution == LDPR_PREVAILING_DEF_IRONLY or resolution
== LDPR_PREVAILING_DEF) symbols.
I belive in these cases for both returns above can be true. here we know
symbol binds locally from linker.
We however can not trust resolution == LDPR_PREVAILING_DEF completely because
DEFAULT visibility symbols should still return false for shlib.
One case to handle it would bo whole_program_visibility to look for this case
and remove
DECL_WEAK, DECL_COMMON and COMDAT flags. It already does that for
LDPR_PREVAILING_DEF_IRONLY
via make_decl_local, but I am not quite convinced we can safely do it for
COMDATs that are
resolution == LDPR_PREVAILING_DEF, since it will make the symbol to change its
type for
linker. Linker API specification do not speak about possibility of such changes.
While looking into the details, I also convinced myself that the following test:
/* Variables defined outside this object might not be local. */
else if (DECL_EXTERNAL (exp) && !resolved_locally)
local_p = false;
/* If defined in this object and visibility is not default, must be
local. */
else if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
local_p = true;
should really stand
/* Variables defined outside this object might not be local. */
else if (DECL_EXTERNAL (exp) && !resolved_locally)
local_p = false;
/* If defined in this object and visibility is not default, must be
local. */
else if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT && !DECL_EXTERNAL (exp))
local_p = true;
We do not know if the defining unit is defining the symbol with hidden
visibility of not.
Perhaps linker can tell us via the resolution info, too?
At the moment I can not think of other problems here.
Honza
