From: Alexandre Oliva <oliva@adacore.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
Jan Hubicka <hubicka@ucw.cz>, Uros Bizjak <ubizjak@gmail.com>
Subject: Re: [PATCH] [PR83782] i386 PIE: avoid @GOTOFF for ifuncs and their aliases
Date: Mon, 08 Aug 2022 16:29:09 -0300 [thread overview]
Message-ID: <oriln2ttyy.fsf@lxoliva.fsfla.org> (raw)
In-Reply-To: <CAMe9rOoXwrSDm+tooZsfPgOsFB8NnO45O_SFHgiNFPGw=heegw@mail.gmail.com> (H. J. Lu's message of "Mon, 1 Aug 2022 12:03:18 -0700")
On Aug 1, 2022, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> On Thu, Jul 28, 2022 at 9:31 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>> > You may also need to do something like this bit for mvc10.c on ia32 PIE.
>> > Because the ifunc is called through an alias, AFAICT we don't even
>> > notice that the call target is (an alias to) an ifunc. GCC's
>> > gotoff_operand predicate accepts it, but binutils (the linker, IIRC)
>> > then rejects that reference, because the named symbol is an alias to an
>> > ifunc.
>>
>> Yes, this change is needed.
> I think this fix should be applied to default_binds_local_p_3:
I was concerned that other tests might require such alias pointer
chasing, and figured we might be better off propagating the flag setting
to aliases.
[PR83782] ifunc: back-propagate ifunc_resolver to aliases
gcc.target/i386/mvc10.c fails with -fPIE on ia32 because we omit the
@PLT mark when calling an alias to an indirect function. Such aliases
aren't marked as ifunc_resolvers in the cgraph, so the test that would
have forced the PLT call fails.
I've arranged for ifunc_resolver to be back-propagated to aliases, and
relaxed the test that required the ifunc attribute to be attached to
directly the decl, rather than taken from an aliased decl, when the
ifunc_resolver bit is set.
Regstrapped on x86_64-linux-gnu, also tested mvc10.c with -m32 -fPIE.
Ok to install?
for gcc/ChangeLog
PR target/83782
* cgraph.h (symtab_node::set_ifunc_resolver): New, overloaded.
Back-propagate flag to aliases.
* cgraph.cc (cgraph_node::create): Use set_ifunc_resolver.
(cgraph_node::create_alias): Likewise.
* lto-cgraph.cc (input_node): Likewise.
* multiple_target.cc (create_dispatcher_calls): Propagate to
aliases when redirecting them.
* symtab.cc (symtab_node::verify_base): Accept ifunc_resolver
set in an alias to another ifunc_resolver nodes.
(symtab_node::resolve_alias): Propagate ifunc_resolver from
resolved target to alias.
* varasm.cc (do_assemble_alias): Checking for the attribute.
---
gcc/cgraph.cc | 4 ++--
gcc/cgraph.h | 13 +++++++++++++
gcc/lto-cgraph.cc | 2 +-
gcc/multiple_target.cc | 2 ++
gcc/symtab.cc | 15 ++++++++++++++-
gcc/varasm.cc | 5 ++++-
6 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
index 8d6ed38efa25d..699f2c20defa4 100644
--- a/gcc/cgraph.cc
+++ b/gcc/cgraph.cc
@@ -518,7 +518,7 @@ cgraph_node::create (tree decl)
}
if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)))
- node->ifunc_resolver = true;
+ node->set_ifunc_resolver ();
node->register_symbol ();
maybe_record_nested_function (node);
@@ -576,7 +576,7 @@ cgraph_node::create_alias (tree alias, tree target)
if (lookup_attribute ("weakref", DECL_ATTRIBUTES (alias)) != NULL)
alias_node->transparent_alias = alias_node->weakref = true;
if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (alias)))
- alias_node->ifunc_resolver = true;
+ alias_node->set_ifunc_resolver ();
return alias_node;
}
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 4be67e3cea906..9468b8a4e3662 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -467,6 +467,19 @@ public:
return decl->decl_with_vis.symtab_node;
}
+ /* Worked for the nonstatic set_ifunc_resolver, to vback-propagate
+ ifunc_resolver in the alias chain. */
+ static bool set_ifunc_resolver (symtab_node *n, void * = NULL)
+ {
+ n->ifunc_resolver = true;
+ return false;
+ }
+
+ /* Set the ifunc_resolver bit in this node and in any aliases thereof. */
+ void set_ifunc_resolver () {
+ call_for_symbol_and_aliases (set_ifunc_resolver, NULL, true);
+ }
+
/* Try to find a symtab node for declaration DECL and if it does not
exist or if it corresponds to an inline clone, create a new one. */
static inline symtab_node * get_create (tree node);
diff --git a/gcc/lto-cgraph.cc b/gcc/lto-cgraph.cc
index 6d9c36ea8b67f..3d8ade1f9f042 100644
--- a/gcc/lto-cgraph.cc
+++ b/gcc/lto-cgraph.cc
@@ -1280,7 +1280,7 @@ input_node (struct lto_file_decl_data *file_data,
node = symtab->create_empty ();
node->decl = fn_decl;
if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (fn_decl)))
- node->ifunc_resolver = 1;
+ node->set_ifunc_resolver ();
node->register_symbol ();
}
diff --git a/gcc/multiple_target.cc b/gcc/multiple_target.cc
index 3e2d26882c8e9..97801a4cc9ea9 100644
--- a/gcc/multiple_target.cc
+++ b/gcc/multiple_target.cc
@@ -160,6 +160,8 @@ create_dispatcher_calls (struct cgraph_node *node)
source->create_reference (inode, IPA_REF_ALIAS);
if (inode->get_comdat_group ())
source->add_to_same_comdat_group (inode);
+ if (!source->ifunc_resolver)
+ source->set_ifunc_resolver ();
}
else
gcc_unreachable ();
diff --git a/gcc/symtab.cc b/gcc/symtab.cc
index f2d96c0268bf3..ea34afe07f5fc 100644
--- a/gcc/symtab.cc
+++ b/gcc/symtab.cc
@@ -1107,9 +1107,19 @@ symtab_node::verify_base (void)
error ("function symbol is not function");
error_found = true;
}
+ /* If the ifunc attribute is present, the node must be marked as
+ ifunc_resolver, but it may also be marked on a node that
+ doesn't have the attribute, if it's an alias to another
+ marked node. The resolver node itself is an alias to the
+ function that performs the resolution proper, and that
+ function is not marked, but here we test other kinds of
+ aliases, that alias the indirect function. */
else if ((lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
!= NULL)
- != dyn_cast <cgraph_node *> (this)->ifunc_resolver)
+ ? !ifunc_resolver
+ : ifunc_resolver
+ ? !get_alias_target ()->ifunc_resolver
+ : (alias && analyzed && get_alias_target ()->ifunc_resolver))
{
error ("inconsistent %<ifunc%> attribute");
error_found = true;
@@ -1877,6 +1887,9 @@ symtab_node::resolve_alias (symtab_node *target, bool transparent)
if (target->implicit_section)
call_for_symbol_and_aliases (set_implicit_section, NULL, true);
+ if (target->ifunc_resolver && !ifunc_resolver)
+ set_ifunc_resolver ();
+
/* Alias targets become redundant after alias is resolved into an reference.
We do not want to keep it around or we would have to mind updating them
when renaming symbols. */
diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index 4db8506b106b6..56bf3ce6c23dc 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -6217,7 +6217,10 @@ do_assemble_alias (tree decl, tree target)
maybe_assemble_visibility (decl);
}
if (TREE_CODE (decl) == FUNCTION_DECL
- && cgraph_node::get (decl)->ifunc_resolver)
+ && cgraph_node::get (decl)->ifunc_resolver
+ /* Aliases to the ifunc decl will also have the ifunc_resolver
+ bit set, so check that this is the ifunc declaration. */
+ && lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)))
{
#if defined (ASM_OUTPUT_TYPE_DIRECTIVE)
if (targetm.has_ifunc_p ())
--
Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts. Ask me about <https://stallmansupport.org>
prev parent reply other threads:[~2022-08-08 19:29 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-27 5:13 Alexandre Oliva
2022-07-27 14:31 ` H.J. Lu
2022-07-28 8:26 ` Alexandre Oliva
2022-07-28 16:31 ` H.J. Lu
2022-08-01 19:03 ` H.J. Lu
2022-08-01 19:15 ` Fangrui Song
2022-08-08 19:29 ` Alexandre Oliva [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=oriln2ttyy.fsf@lxoliva.fsfla.org \
--to=oliva@adacore.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hjl.tools@gmail.com \
--cc=hubicka@ucw.cz \
--cc=ubizjak@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).