Re: [PATCH 2/8] s390: Refactor ifunc resolvers due to false debuginfo.

[Florian Weimer <fweimer at redhat dot com>]

On 06/23/2016 01:55 PM, Stefan Liebler wrote:
> This patch adjusts the s390 specific ifunc helper macros in ifunc-resolve.h to
> use the common __ifunc macro, which uses gcc attribute ifunc to get rid of the
> false debuginfo. Therefore the redirection construct is applied where needed.
>
> Perhaps in future we can switch some of the internal symbols __GI_* from the
> fallback variant to the ifunc function. But this change is not
> straightforward due to a segmentation fault while linking libc.so with older
> binutils on s390.
>
> ChangeLog:
>
> 	* sysdeps/s390/multiarch/ifunc-resolve.h
> 	(s390_vx_libc_ifunc2, s390_libc_ifunc): Use __ifunc from libc-symbols.h
> 	to create ifunc symbols.
> 	(s390_vx_libc_ifunc_init, s390_libc_ifunc_init): New define.
> 	* sysdeps/s390/multiarch/memchr.c: Redirect ifunced function in header
> 	and create a copy of the prototype for using as ifunc function.
> 	Add appropiate aliases to the real symbol names.

“appropriate”

> +  __ifunc (FUNC,							\
> +	   __glibc_likely (S390_IS_Z196 (stfle_bits))			\
> +	   ? RESOLVERFUNC##_z196 :					\
> +	     __glibc_likely (S390_IS_Z10 (stfle_bits))			\
> +	     ? RESOLVERFUNC##_z10 :					\
> +	     RESOLVERFUNC##_default					\
> +	   , unsigned long int dl_hwcap, s390_libc_ifunc_init);

I don't think GNU style allows starting a line with a comma, and “:” 
should be at the start of the line, not the end.

Otherwise, the changes look reasonable.

Thanks,
Florian