From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [IPv6:2620:20:4000:0:a9e:1ff:fe9b:1d1]) by sourceware.org (Postfix) with ESMTPS id 6D2E3385C311 for ; Mon, 8 Aug 2022 19:29:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6D2E3385C311 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id B916A116E17; Mon, 8 Aug 2022 15:29:28 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id o0hwcBDR4p5R; Mon, 8 Aug 2022 15:29:28 -0400 (EDT) Received: from free.home (tron.gnat.com [IPv6:2620:20:4000:0:46a8:42ff:fe0e:e294]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id 590C2116BFB; Mon, 8 Aug 2022 15:29:28 -0400 (EDT) Received: from livre (livre.home [172.31.160.2]) by free.home (8.15.2/8.15.2) with ESMTPS id 278JT9Pk2193765 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 8 Aug 2022 16:29:09 -0300 From: Alexandre Oliva To: "H.J. Lu" Cc: GCC Patches , Jan Hubicka , Uros Bizjak Subject: Re: [PATCH] [PR83782] i386 PIE: avoid @GOTOFF for ifuncs and their aliases Organization: Free thinker, does not speak for AdaCore References: Errors-To: aoliva@lxoliva.fsfla.org Date: Mon, 08 Aug 2022 16:29:09 -0300 In-Reply-To: (H. J. Lu's message of "Mon, 1 Aug 2022 12:03:18 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Aug 2022 19:29:33 -0000 On Aug 1, 2022, "H.J. Lu" wrote: > On Thu, Jul 28, 2022 at 9:31 AM H.J. Lu 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 (this)->ifunc_resolver) + ? !ifunc_resolver + : ifunc_resolver + ? !get_alias_target ()->ifunc_resolver + : (alias && analyzed && get_alias_target ()->ifunc_resolver)) { error ("inconsistent % 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