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.

Regstrapped on x86_64-linux-gnu; also tested, along with other patches
I'm posting today with "i386 PIE" in the subject, and compared
default-PIE and default-nonPIE results on it, and on i686-linux-gnu.  Ok
to install?


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.
---
 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 aab28da4b5d4b..5c5dc8d2373ff 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 42053ea7209f6..411c06e22e600 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -596,7 +596,9 @@ (define_predicate "local_symbolic_operand"
   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 ce97b12e65d58..af52278ec4df2 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 e25d258bbda43..15c7dc04e88c3 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 } } } 
} */

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

Reply via email to