[gcc(refs/users/aoliva/heads/testme)] i386 PIE: avoid @GOTOFF for ifuncs and their aliases

[gcc(refs/users/aoliva/heads/testme)] i386 PIE: avoid @GOTOFF for ifuncs and their aliases

[Alexandre Oliva]

https://gcc.gnu.org/g:dc8437273ef95e3100dcbb228f60eb133056d37d

commit dc8437273ef95e3100dcbb228f60eb133056d37d
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Tue Jul 26 00:37:25 2022 -0300

    i386 PIE: avoid @GOTOFF for ifuncs and their aliases
    
    g++.dg/ext/attr-ifunc-3.C and gcc.target/i386/mvc10.c, not changed,
    have made it clear that there were problems in the optimizations to
    use @GOTOFF to refer to locally-bound ifuncs.  GNU ld as recently as
    May 2018 would reject such constructs, whereas later versions will
    silently accept but generate incorrect PIE with them (attr-ifunc-3.C)
    or still reject them if referenced through aliases (mvc10.c).  The use
    of @GOTOFF for locally-bound but externally-visible symbols
    (e.g. protected visibility) also breaks pointer identity if the
    canonical address ends up preempted by a PLT entry.  This patch
    modifies the local_symbolic_operand predicate to disable @GOTOFF for
    locally-bound symbols that would require @PLT for calls, restoring
    earlier behavior and disabling the optimization that has proven
    problematic even on amd64.  Eventually we may reintroduce the
    optimization, when the linker is fixed and we test for the fix before
    enabling it, and we exclude symbols whose canonical addresses may be
    preempted even when the symbol definition can't.  pr83782 tests have
    been adjusted to expect @GOT instead of @GOTOFF.
    
    
    for  gcc/ChangeLog
    
            PR target/83782
            * config/i386/predicates.md (local_symbolic_operand): Disable
            GOTOFF even for locally-bound ifuncs.
            * config/i386/i386.cc (ix86_call_use_plt_p): Follow the alias
            chain looking for an ifunc, as in gcc.target/i386/mvc10.c.
    
    for  gcc/testsuite/ChangeLog
    
            PR target/83782
            * gcc.target/i386/pr83782-1.c: Adjust to require GOT rather
            than GOTOFF on ia32.
            * gcc.target/i386/pr83782-2.c: Likewise.

Diff:
---
 gcc/config/i386/i386.cc                   | 16 ++++++++++------
 gcc/config/i386/predicates.md             |  4 +++-
 gcc/testsuite/gcc.target/i386/pr83782-1.c |  4 ++--
 gcc/testsuite/gcc.target/i386/pr83782-2.c |  4 ++--
 4 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index aab28da4b5d..5c5dc8d2373 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -16058,13 +16058,17 @@ ix86_call_use_plt_p (rtx call_op)
     {
       if (SYMBOL_REF_DECL (call_op)
 	  && TREE_CODE (SYMBOL_REF_DECL (call_op)) == FUNCTION_DECL)
-	{
-	  /* NB: All ifunc functions must be called via PLT.  */
-	  cgraph_node *node
-	    = cgraph_node::get (SYMBOL_REF_DECL (call_op));
-	  if (node && node->ifunc_resolver)
+	/* NB: All ifunc functions must be called via PLT, and we have
+	   to explicitly iterate over an alias chain looking for a
+	   node marked as an ifunc(_resolver) to tell.  That node is
+	   itself aliased to the actual resolver function, so
+	   ultimate_alias_target would skip the marker, and the call
+	   may be to another declaration aliased to the ifunc.  */
+	for (cgraph_node *node
+	       = cgraph_node::get (SYMBOL_REF_DECL (call_op));
+	     node && node->alias; node = node->get_alias_target ())
+	  if (node->ifunc_resolver)
 	    return true;
-	}
       return false;
     }
   return true;
diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index 064596d9594..76ff745f7b5 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -596,7 +596,9 @@
   if (TARGET_DLLIMPORT_DECL_ATTRIBUTES && SYMBOL_REF_DLLIMPORT_P (op))
     return false;
   if (SYMBOL_REF_LOCAL_P (op))
-    return true;
+    /* ifuncname@GOTOFF was rejected by the x86 linker before May
+       2018, and silently generated wrong code for PIE afterwards.  */
+    return !ix86_call_use_plt_p (op);
 
   /* There is, however, a not insubstantial body of code in the rest of
      the compiler that assumes it can just stick the results of
diff --git a/gcc/testsuite/gcc.target/i386/pr83782-1.c b/gcc/testsuite/gcc.target/i386/pr83782-1.c
index ce97b12e65d..af52278ec4d 100644
--- a/gcc/testsuite/gcc.target/i386/pr83782-1.c
+++ b/gcc/testsuite/gcc.target/i386/pr83782-1.c
@@ -20,7 +20,7 @@ bar(void)
   return foo;
 }
 
-/* { dg-final { scan-assembler {leal[ \t]foo@GOTOFF\(%[^,]*\),[ \t]%eax} { target ia32 } } } */
+/* { dg-final { scan-assembler-not {leal[ \t]foo@GOTOFF\(%[^,]*\),[ \t]%eax} { target ia32 } } } */
 /* { dg-final { scan-assembler {lea(?:l|q)[ \t]foo\(%rip\),[ \t]%(?:e|r)ax} { target { ! ia32 } } } } */
-/* { dg-final { scan-assembler-not "foo@GOT\\\(" { target ia32 } } } */
+/* { dg-final { scan-assembler "foo@GOT\\\(" { target ia32 } } } */
 /* { dg-final { scan-assembler-not "foo@GOTPCREL\\\(" { target { ! ia32 } } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr83782-2.c b/gcc/testsuite/gcc.target/i386/pr83782-2.c
index e25d258bbda..15c7dc04e88 100644
--- a/gcc/testsuite/gcc.target/i386/pr83782-2.c
+++ b/gcc/testsuite/gcc.target/i386/pr83782-2.c
@@ -20,7 +20,7 @@ bar(void)
   return foo;
 }
 
-/* { dg-final { scan-assembler {leal[ \t]foo@GOTOFF\(%[^,]*\),[ \t]%eax} { target ia32 } } } */
+/* { dg-final { scan-assembler-not {leal[ \t]foo@GOTOFF\(%[^,]*\),[ \t]%eax} { target ia32 } } } */
 /* { dg-final { scan-assembler {lea(?:l|q)[ \t]foo\(%rip\),[ \t]%(?:e|r)ax} { target { ! ia32 } } } } */
-/* { dg-final { scan-assembler-not "foo@GOT\\\(" { target ia32 } } } */
+/* { dg-final { scan-assembler "foo@GOT\\\(" { target ia32 } } } */
 /* { dg-final { scan-assembler-not "foo@GOTPCREL\\\(" { target { ! ia32 } } } } */