diff mbox series

cgraph: ifunc resolvers cannot be made local (PR 92697)

Message ID ri6r21st479.fsf@suse.cz
State New
Headers show
Series cgraph: ifunc resolvers cannot be made local (PR 92697) | expand

Commit Message

Martin Jambor Nov. 28, 2019, 3:23 p.m. UTC 
Hi,

In the attached testcase, IPA-SRA thinks that an ifunc resolver
(meanwhile IPA-split into two functions) function can be changed and so
goes ahead.  The cgraph machinery then however throws away the new clone
of the caller instead of the "old" caller and inliner inlines the clone
of the ".part" function into the original resolver, which results into
an interesting miscompilation because IPA-SRA counted on that both the
caller and the callee are modified.

Fixed by making cgraph_node::can_be_local_p return false for ifunc
resolvers, as it should.  The patch also adds dumping of the symtab_node
flag.  Bootstrapped and tested on x86_64-linux, OK for trunk?

Thanks,

Martin



2019-11-27  Martin Jambor  <mjambor@suse.cz>

	PR ipa/92697
	* cgraph.c (cgraph_node_cannot_be_local_p_1): Return true for
	ifunc_resolvers.
	* symtab.c (symtab_node::dump_base): Dump ifunc_resolver flag.
	Removed trailig whitespace.

	testsuite/
	* g++.dg/ipa/pr92697.C: New.
---
 gcc/cgraph.c                       |  1 +
 gcc/symtab.c                       |  4 ++-
 gcc/testsuite/g++.dg/ipa/pr92697.C | 51 ++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/ipa/pr92697.C

Comments

Jan Hubicka Nov. 28, 2019, 3:28 p.m. UTC | #1 
> Hi,
> 
> In the attached testcase, IPA-SRA thinks that an ifunc resolver
> (meanwhile IPA-split into two functions) function can be changed and so
> goes ahead.  The cgraph machinery then however throws away the new clone
> of the caller instead of the "old" caller and inliner inlines the clone
> of the ".part" function into the original resolver, which results into
> an interesting miscompilation because IPA-SRA counted on that both the
> caller and the callee are modified.
> 
> Fixed by making cgraph_node::can_be_local_p return false for ifunc
> resolvers, as it should.  The patch also adds dumping of the symtab_node
> flag.  Bootstrapped and tested on x86_64-linux, OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 
> 2019-11-27  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR ipa/92697
> 	* cgraph.c (cgraph_node_cannot_be_local_p_1): Return true for
> 	ifunc_resolvers.
> 	* symtab.c (symtab_node::dump_base): Dump ifunc_resolver flag.
> 	Removed trailig whitespace.
> 
> 	testsuite/
> 	* g++.dg/ipa/pr92697.C: New.
OK,
thanks!

Honza
diff mbox series

Patch

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 1f7a5c58d98..dd07516b83e 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -2227,6 +2227,7 @@  static bool
 cgraph_node_cannot_be_local_p_1 (cgraph_node *node, void *)
 {
   return !(!node->force_output
+	   && !node->ifunc_resolver
 	   && ((DECL_COMDAT (node->decl)
 		&& !node->forced_by_abi
 		&& !node->used_from_object_file_p ()
diff --git a/gcc/symtab.c b/gcc/symtab.c
index 3e634e22c86..5a3122fc8bb 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -914,8 +914,10 @@  symtab_node::dump_base (FILE *f)
       if (DECL_STATIC_DESTRUCTOR (decl))
 	fprintf (f, " destructor");
     }
+  if (ifunc_resolver)
+    fprintf (f, " ifunc_resolver");
   fprintf (f, "\n");
-  
+
   if (same_comdat_group)
     fprintf (f, "  Same comdat group as: %s\n",
 	     same_comdat_group->dump_asm_name ());
diff --git a/gcc/testsuite/g++.dg/ipa/pr92697.C b/gcc/testsuite/g++.dg/ipa/pr92697.C
new file mode 100644
index 00000000000..8958bd0dcf2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr92697.C
@@ -0,0 +1,51 @@ 
+/* { dg-do compile } */
+/* { dg-require-ifunc "" } */
+/* { dg-options "-O2 -fdump-ipa-sra" } */
+
+extern int have_avx2;
+extern int have_ssse3;
+
+namespace NTL
+{
+
+  static void randomstream_impl_init_base ()
+  {
+    __builtin_printf ("Frob1\n");
+  }
+
+  static void // __attribute__ ((target ("ssse3")))
+    randomstream_impl_init_ssse3 ()
+  {
+    __builtin_printf ("Frob2\n");
+  }
+
+  static void
+    //__attribute__ ((target ("avx2,fma,avx,pclmul,ssse3")))
+    randomstream_impl_init_avx2 ()
+  {
+    __builtin_printf ("Frob3\n");
+  }
+
+  extern "C"
+  {
+    static void (*resolve_randomstream_impl_init (void)) ()
+    {
+      if (have_avx2)
+	return &randomstream_impl_init_avx2;
+      if (have_ssse3)
+	return &randomstream_impl_init_ssse3;
+      return &randomstream_impl_init_base;
+    }
+  }
+  static void
+    __attribute__ ((ifunc ("resolve_" "randomstream_impl_init")))
+    randomstream_impl_init ();
+  void foo ()
+  {
+    randomstream_impl_init ();
+  }
+
+}
+
+
+/* { dg-final { scan-ipa-dump-not "Created new node" "sra" } } */