[RS6000] ABI_V4 ifunc

[RS6000] ABI_V4 ifunc

[Alan Modra]

Given a hidden visibility function declaration, the toolchain can say
that the function is local to the module.  This generally means that a
call to the function can be direct, ie. doesn't need to go via the PLT
even in a shared library.  However, ifunc breaks this promise.  GNU
indirect functions may resolve non-locally, and are implemented by
always using a PLT call.

This causes trouble for targets like ppc32 where the -msecure-plt PIC
PLT call stub needs a valid GOT pointer.  Any call that potentially
might be to an ifunc therefore requires a GOT pointer, and can't be a
sibling call (because the GOT pointer on ppc32 is a caller saved reg).

So unless we require that all ifuncs are declared as ifunc, it seems
that ppc32 can't assume extern or weak functions are local.  This
patch does the latter.  Boostrapped and regression tested
powerpc64-linux biarch.  OK to apply?

Also see https://sourceware.org/bugzilla/show_bug.cgi?id=19784 and
testcase ld/testsuite/ld-ifunc/pr17984*.c.  Note that the problem does
not need -Bsymbolic-functions to exhibit on powerpc.

	* config/rs6000/predicates.md (current_file_function_operand):
	Exclude extern and weak sym_refs for ABI_V4 when PIC and
	secure-plt.
	* config/rs6000/rs6000.c (rs6000_function_ok_for_sibcall): Don't
	allow sibcalls for ABI_V4 secure-plt PIC extern and weak.
	(rs6000_elf_encode_section_info): Remove SYMBOL_FLAG_LOCAL on ifuncs.
	* config/rs6000/rs6000.md (call_nonlocal_sysv<mode>): Split when
	extern or weak.
	(call_value_nonlocal_sysv<mode>): Likewise.
	(call_nonlocal_sysv_secure<mode>): Accept extern and weak.
	(call_value_nonlocal_sysv_secure<mode>): Likewise.

diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index cc0a8b8..0204a6f 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -1041,14 +1041,22 @@
      (match_code "symbol_ref")))
 
 ;; Return 1 if the operand is a SYMBOL_REF for a function known to be in
-;; this file.
+;; this file, at least for ABI_AIX, ABI_ELFv2, and ABI_V4 when pic.  For
+;; other cases, note that SYMBOL_REF_LOCAL_P is only local to this module
+;; so the condition is not as strict.
+;; ABI_AIX and ABI_ELFv2 must treat calls to functions outside the current
+;; file specially because they may be using a different TOC base.
+;; ABI_V4 cannot decorate calls to ifuncs with @local when pic.
 (define_predicate "current_file_function_operand"
   (and (match_code "symbol_ref")
        (match_test "(DEFAULT_ABI != ABI_AIX || SYMBOL_REF_FUNCTION_P (op))
 		    && (SYMBOL_REF_LOCAL_P (op)
 			|| op == XEXP (DECL_RTL (current_function_decl), 0))
 		    && !((DEFAULT_ABI == ABI_AIX
-			  || DEFAULT_ABI == ABI_ELFv2)
+			  || DEFAULT_ABI == ABI_ELFv2
+			  || (DEFAULT_ABI == ABI_V4
+			      && TARGET_SECURE_PLT
+			      && flag_pic))
 			 && (SYMBOL_REF_EXTERNAL_P (op)
 			     || SYMBOL_REF_WEAK (op)))")))
 
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 3bc7516..ec1e125 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -25474,16 +25474,11 @@ rs6000_function_ok_for_sibcall (tree decl, tree exp)
      we return.  With the secure-plt SYSV ABI we can't make non-local
      calls when -fpic/PIC because the plt call stubs use r30.  */
   if (DEFAULT_ABI == ABI_DARWIN
-      || ((DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
-	  && decl
+      || (DEFAULT_ABI == ABI_V4 && !(TARGET_SECURE_PLT && flag_pic))
+      || (decl
 	  && !DECL_EXTERNAL (decl)
 	  && !DECL_WEAK (decl)
-	  && (*targetm.binds_local_p) (decl))
-      || (DEFAULT_ABI == ABI_V4
-	  && (!TARGET_SECURE_PLT
-	      || !flag_pic
-	      || (decl
-		  && (*targetm.binds_local_p) (decl)))))
+	  && (*targetm.binds_local_p) (decl)))
     {
       tree attr_list = TYPE_ATTRIBUTES (fntype);
 
@@ -33018,6 +33013,14 @@ rs6000_elf_encode_section_info (tree decl, rtx rtl, int first)
 {
   default_encode_section_info (decl, rtl, first);
 
+  if (TREE_CODE (decl) == FUNCTION_DECL && MEM_P (rtl))
+    {
+      rtx symbol = XEXP (rtl, 0);
+      if (GET_CODE (symbol) == SYMBOL_REF
+	  && lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)))
+	SYMBOL_REF_FLAGS (symbol) &= ~SYMBOL_FLAG_LOCAL;
+    }
+
   if (first
       && TREE_CODE (decl) == FUNCTION_DECL
       && !TARGET_AIX
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 58dccdd..07e79ab 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -10278,8 +10278,10 @@
     return "bl %z0";
 #endif
 }
-  "DEFAULT_ABI == ABI_V4
-   && TARGET_SECURE_PLT && flag_pic && !SYMBOL_REF_LOCAL_P (operands[0])
+  "DEFAULT_ABI == ABI_V4 && TARGET_SECURE_PLT && flag_pic
+   && (!SYMBOL_REF_LOCAL_P (operands[0])
+       || SYMBOL_REF_EXTERNAL_P (operands[0])
+       || SYMBOL_REF_WEAK (operands[0]))
    && (INTVAL (operands[2]) & CALL_LONG) == 0"
   [(parallel [(call (mem:SI (match_dup 0))
 		    (match_dup 1))
@@ -10298,9 +10300,11 @@
    (use (match_operand:SI 2 "immediate_operand" "O,n"))
    (use (match_operand:SI 3 "register_operand" "r,r"))
    (clobber (reg:SI LR_REGNO))]
-  "(DEFAULT_ABI == ABI_V4
-    && TARGET_SECURE_PLT && flag_pic && !SYMBOL_REF_LOCAL_P (operands[0])
-    && (INTVAL (operands[2]) & CALL_LONG) == 0)"
+  "DEFAULT_ABI == ABI_V4 && TARGET_SECURE_PLT && flag_pic
+   && (!SYMBOL_REF_LOCAL_P (operands[0])
+       || SYMBOL_REF_EXTERNAL_P (operands[0])
+       || SYMBOL_REF_WEAK (operands[0]))
+   && (INTVAL (operands[2]) & CALL_LONG) == 0"
 {
   if (INTVAL (operands[2]) & CALL_V4_SET_FP_ARGS)
     output_asm_insn ("crxor 6,6,6", operands);
@@ -10367,8 +10371,10 @@
     return "bl %z1";
 #endif
 }
-  "DEFAULT_ABI == ABI_V4
-   && TARGET_SECURE_PLT && flag_pic && !SYMBOL_REF_LOCAL_P (operands[1])
+  "DEFAULT_ABI == ABI_V4 && TARGET_SECURE_PLT && flag_pic
+   && (!SYMBOL_REF_LOCAL_P (operands[1])
+       || SYMBOL_REF_EXTERNAL_P (operands[1])
+       || SYMBOL_REF_WEAK (operands[1]))
    && (INTVAL (operands[3]) & CALL_LONG) == 0"
   [(parallel [(set (match_dup 0)
 		   (call (mem:SI (match_dup 1))
@@ -10389,9 +10395,11 @@
    (use (match_operand:SI 3 "immediate_operand" "O,n"))
    (use (match_operand:SI 4 "register_operand" "r,r"))
    (clobber (reg:SI LR_REGNO))]
-  "(DEFAULT_ABI == ABI_V4
-    && TARGET_SECURE_PLT && flag_pic && !SYMBOL_REF_LOCAL_P (operands[1])
-    && (INTVAL (operands[3]) & CALL_LONG) == 0)"
+  "DEFAULT_ABI == ABI_V4 && TARGET_SECURE_PLT && flag_pic
+   && (!SYMBOL_REF_LOCAL_P (operands[1])
+       || SYMBOL_REF_EXTERNAL_P (operands[1])
+       || SYMBOL_REF_WEAK (operands[1]))
+   && (INTVAL (operands[3]) & CALL_LONG) == 0"
 {
   if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS)
     output_asm_insn ("crxor 6,6,6", operands);

-- 
Alan Modra
Australia Development Lab, IBM

Re: [RS6000] ABI_V4 ifunc

[Segher Boessenkool]

On Wed, Aug 24, 2016 at 10:14:29AM +0930, Alan Modra wrote:
> Given a hidden visibility function declaration, the toolchain can say
> that the function is local to the module.  This generally means that a
> call to the function can be direct, ie. doesn't need to go via the PLT
> even in a shared library.  However, ifunc breaks this promise.  GNU
> indirect functions may resolve non-locally, and are implemented by
> always using a PLT call.
> 
> This causes trouble for targets like ppc32 where the -msecure-plt PIC
> PLT call stub needs a valid GOT pointer.  Any call that potentially
> might be to an ifunc therefore requires a GOT pointer, and can't be a
> sibling call (because the GOT pointer on ppc32 is a caller saved reg).
> 
> So unless we require that all ifuncs are declared as ifunc, it seems
> that ppc32 can't assume extern or weak functions are local.  This
> patch does the latter.  Boostrapped and regression tested
> powerpc64-linux biarch.  OK to apply?

Okay for trunk.  (Unless "boostrapped" means that it failed horribly ;-) )


> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 58dccdd..07e79ab 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -10278,8 +10278,10 @@

This doesn't show what pattern this is.  If you make a ".gitattributes"
somewhere in your tree (I have it in the gcc/ subdir), containing

*.md  diff=md

and then in your git config (say, in .git/config in the GCC source tree)
you have

[diff "md"]
        xfuncname = "^\\(define.*$"

then magic happens.

Cheers,


Segher

Re: [RS6000] ABI_V4 ifunc

[Alexander Monakov]

On Wed, 24 Aug 2016, Alan Modra wrote:
> Given a hidden visibility function declaration, the toolchain can say
> that the function is local to the module.  This generally means that a
> call to the function can be direct, ie. doesn't need to go via the PLT
> even in a shared library.  However, ifunc breaks this promise.  GNU
> indirect functions may resolve non-locally, and are implemented by
> always using a PLT call.
> 
> This causes trouble for targets like ppc32 where the -msecure-plt PIC
> PLT call stub needs a valid GOT pointer.  Any call that potentially
> might be to an ifunc therefore requires a GOT pointer, and can't be a
> sibling call (because the GOT pointer on ppc32 is a caller saved reg).

The same issue exists on 32-bit x86: PLT calls require that %ebx holds the
address of GOT (and the sibcall issue arises as well).  I've just confirmed
using a simple testcase that the scenario you describe leads to a runtime error
on i386, and even LD_BIND_NOW=1 doesn't help, as it doesn't trigger early
resolution of ifuncs.

> So unless we require that all ifuncs are declared as ifunc,

(note, that would be impossible with today's GCC because the ifunc attribute
requires designating the resolver, and the resolver cannot be extern -- so
ultimately you cannot declare an extern-ifunc symbol)

> it seems that ppc32 can't assume extern or weak functions are local.

It doesn't seem nice to penalize all normal calls due to this issue. I think a
solution without this cost is possible: have ld synthesize a forwarder function
when it sees a non-plt call to an ifunc symbol. The forwarder can push the GOT
register, load the GOT address, call the ifunc symbol, pop the GOT register and
return. Does this sound right?

HTH.
Alexander

Re: [RS6000] ABI_V4 ifunc

[Richard Earnshaw (lists)]

On 24/08/16 02:55, Segher Boessenkool wrote:

>  If you make a ".gitattributes"
> somewhere in your tree (I have it in the gcc/ subdir), containing
> 
> *.md  diff=md
> 
> and then in your git config (say, in .git/config in the GCC source tree)
> you have
> 
> [diff "md"]
>         xfuncname = "^\\(define.*$"
> 
> then magic happens.

Which sort of begs the question as to why nobody has proposed at least
the top bit for GCC's toplevel .gitattributes file.

R.

Re: [RS6000] ABI_V4 ifunc

[Mike Stump]

On Aug 25, 2016, at 3:14 AM, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
> 
> On 24/08/16 02:55, Segher Boessenkool wrote:
>> If you make a ".gitattributes"
>> somewhere in your tree (I have it in the gcc/ subdir), containing
>> 
>> *.md  diff=md
>> 
>> and then in your git config (say, in .git/config in the GCC source tree)
>> you have
>> 
>> [diff "md"]
>>        xfuncname = "^\\(define.*$"
>> 
>> then magic happens.
> 
> Which sort of begs the question as to why nobody has proposed at least
> the top bit for GCC's toplevel .gitattributes file.

No expert has considered it obvious yet.  We can encourage them.  :-)  For the people that don't know git, it might not be obvious.

Re: [RS6000] ABI_V4 ifunc

[Segher Boessenkool]

On Thu, Aug 25, 2016 at 11:14:49AM +0100, Richard Earnshaw (lists) wrote:
> On 24/08/16 02:55, Segher Boessenkool wrote:
> 
> >  If you make a ".gitattributes"
> > somewhere in your tree (I have it in the gcc/ subdir), containing
> > 
> > *.md  diff=md
> > 
> > and then in your git config (say, in .git/config in the GCC source tree)
> > you have
> > 
> > [diff "md"]
> >         xfuncname = "^\\(define.*$"
> > 
> > then magic happens.
> 
> Which sort of begs the question as to why nobody has proposed at least
> the top bit for GCC's toplevel .gitattributes file.

Because it will do no good without the second part (which is more
complicated, so anyone doing that can easily do the first part), and
only the first part on its own does only hurt, not help.


Segher

Re: [RS6000] ABI_V4 ifunc

[Richard Earnshaw (lists)]

On 25/08/16 20:13, Segher Boessenkool wrote:
> On Thu, Aug 25, 2016 at 11:14:49AM +0100, Richard Earnshaw (lists) wrote:
>> On 24/08/16 02:55, Segher Boessenkool wrote:
>>
>>>  If you make a ".gitattributes"
>>> somewhere in your tree (I have it in the gcc/ subdir), containing
>>>
>>> *.md  diff=md
>>>
>>> and then in your git config (say, in .git/config in the GCC source tree)
>>> you have
>>>
>>> [diff "md"]
>>>         xfuncname = "^\\(define.*$"
>>>
>>> then magic happens.
>>
>> Which sort of begs the question as to why nobody has proposed at least
>> the top bit for GCC's toplevel .gitattributes file.
> 
> Because it will do no good without the second part (which is more
> complicated, so anyone doing that can easily do the first part), and
> only the first part on its own does only hurt, not help.
> 
> 
> Segher
> 

I disagree.  The first part has to be in-tree (as I understand it) so
having to keep that bit separate from any patches is a real pain.  Yes
you need both parts to make it work, but if the in-tree part is just
there then it's not that hard to maintain the out-of-tree part yourself.

R.