[1/2] Only support ifunc in nptl/pt-vfork.c

  * nptl/pt-vfork.c: Error if !HAVE_IFUNC.
	[!HAVE_IFUNC] (vfork_compat): Remove.
	[!HAVE_IFUNC] (DEFINE_VFORK): Remove.
---
 nptl/pt-vfork.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)
  

> 	* nptl/pt-vfork.c: Error if !HAVE_IFUNC.
> 	[!HAVE_IFUNC] (vfork_compat): Remove.
> 	[!HAVE_IFUNC] (DEFINE_VFORK): Remove.

Yeah, but then you added it right back for aarch64.  I don't know what
about aarch64 makes you certain that the compiler will always do a proper
tail call there.  If it's something like that GCC is the only compiler for
that machine and GCC support for the machine only exists in GCC versions
that you're positive always do a proper tail call for this case at -O!=0
(since we don't support compiling libc with -O0 anyway) even with explicit
-fno-omit-frame-pointer (which people building libc are in fact free to
do), then similar facts are likely to exist about some other machines too.
So we might as well keep the shared code rather than having people start
copying and pasting your aarch64 version.  For paranoia about a compiler
that fails to do a proper tail call for a trivial argumentless function
tail-calling another argumentless function at -O1 (are there really any
such compilers that could compile libc?), we could make machines explicitly
define a macro to say it's OK or something like that.

> +/* Note! If the architecture doesn't support IFUNC, then we need an
> +   alternate target-specific mechanism to implement this.  So we just
> +   assume IFUNC here and require that the target override this file
> +   if necessary.  */
> +
> +#if !HAVE_IFUNC
> +# error
> +#endif

I suppose it doesn't much matter, but my preference is to always have some
text in an #error or #warning.  The nearby comment can be longer and of
course is what provides the real context.  But something like:

	# error must write pt-vfork for this machine or get IFUNC support!

makes it plain even to someone who looks at the build log without
consulting the source.


Thanks,
Roland
  

On 05/23/2014 07:13 PM, Roland McGrath wrote:
>> 	* nptl/pt-vfork.c: Error if !HAVE_IFUNC.
>> 	[!HAVE_IFUNC] (vfork_compat): Remove.
>> 	[!HAVE_IFUNC] (DEFINE_VFORK): Remove.
> 
> Yeah, but then you added it right back for aarch64.  I don't know what
> about aarch64 makes you certain that the compiler will always do a proper
> tail call there.  If it's something like that GCC is the only compiler for
> that machine and GCC support for the machine only exists in GCC versions
> that you're positive always do a proper tail call for this case at -O!=0
> (since we don't support compiling libc with -O0 anyway) even with explicit
> -fno-omit-frame-pointer (which people building libc are in fact free to
> do), then similar facts are likely to exist about some other machines too.

For aarch64, gcc must be >= 4.8, since the port is quite new.

The target does not have a got/toc/gp type register that must be preserved in
some way (unlike e.g. alpha, mips, ppc), so the compiler can (and does) emit a
normal tail-call to the plt entry at -O1.

I did initially write this as an asm(""), but then I thought...

> So we might as well keep the shared code rather than having people start
> copying and pasting your aarch64 version.

... someone could make an informed decision to share the code via

#include <sysdeps/unix/sysv/linux/aarch64/pt-vfork.c>

instead of having it in the nptl/pt-vfork.c where a port could get in trouble
by default.

>> +#if !HAVE_IFUNC
>> +# error
>> +#endif
> 
> I suppose it doesn't much matter, but my preference is to always have some
> text in an #error or #warning.  The nearby comment can be longer and of
> course is what provides the real context.  But something like:
> 
> 	# error must write pt-vfork for this machine or get IFUNC support!
> 
> makes it plain even to someone who looks at the build log without
> consulting the source.

Fair enough.


r~
  

> ... someone could make an informed decision to share the code via
> 
> #include <sysdeps/unix/sysv/linux/aarch64/pt-vfork.c>
> 
> instead of having it in the nptl/pt-vfork.c where a port could get in trouble
> by default.

Agreed.  That seems simpler than more internal flag macros.  I don't know
why I didn't think of that.  Make sure the longer comment in pt-vfork.c
that one sees for the !HAVE_IFUNC #error suggests this explicitly.


Thanks,
Roland