[v2,PR83782] ifunc: back-propagate ifunc_resolver to aliases

Message ID orv8acgpru.fsf@lxoliva.fsfla.org
State Accepted
Headers
Series [v2,PR83782] ifunc: back-propagate ifunc_resolver to aliases |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Alexandre Oliva Nov. 8, 2023, 4:42 p.m. UTC
  Ping?
https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599453.html

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 with gcc-13 on i686- and
x86_64-.  Ok to install?

(in the initial patchset for PR83782 and mvc10, I also needed
https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598873.html but I'm
not getting that fail any more with gcc-13, apparently because a
different patch was put in to address that part)


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(-)
  

Patch

diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
index e41e5ad3ae74d..046dadf53af93 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 cedaaac3a45b7..e118ac75121ac 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -471,6 +471,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 32c0f5ac6dbc1..e7f77ca72242a 100644
--- a/gcc/lto-cgraph.cc
+++ b/gcc/lto-cgraph.cc
@@ -1294,7 +1294,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 a2ed048d7dd28..26c73d6a1e4cf 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 0470509a98d2a..b35b879028def 100644
--- a/gcc/symtab.cc
+++ b/gcc/symtab.cc
@@ -1109,9 +1109,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;
@@ -1879,6 +1889,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 6ae35edc5aed9..c6aa7c1745d49 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -6262,7 +6262,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 ())