diff mbox

Fix strftime wcschr namespace (bug 17634)

Message ID 548788AE.50808@linux.vnet.ibm.com
State New
Headers show

Commit Message

Adhemerval Zanella Dec. 9, 2014, 11:41 p.m. UTC 
Hi Joseph


On 27-11-2014 22:45, Joseph Myers wrote:
> Use of strftime, a C90 function, ends up bringing in wcschr, which is
> not a C90 function.  Although not a conformance bug (C90 reserves
> wcs*), this is still contrary to glibc practice of avoiding relying on
> those reservations; this patch arranges for the internal uses to use
> __wcschr instead, with wcschr being a weak alias.  This is more
> complicated than some such patches because of the various IFUNC
> definitions of wcschr (which include code redefining libc_hidden_def
> in a way that involves creating __GI_wcschr manually and so also needs
> to create __GI___wcschr after the change of internal uses to use
> __wcschr).
>
> Tested for x86_64 and 32-bit x86 (testsuite, and that disassembly of
> installed shared libraries is unchanged by the patch).

This patch breaks both powerpc32 and powerpc64 builds and I fixed it by using
these changes over yours:


>
> 2014-11-28  Joseph Myers  <joseph@codesourcery.com>
>
> 	[BZ #17634]
> 	* wcsmbs/wcschr.c [!WCSCHR] (wcschr): Define as __wcschr.
> 	Undefine after defining function.  Define as weak alias of
> 	__wcschr.  Use libc_hidden_weak.
> 	* include/wchar.h (__wcschr): Declare.  Use libc_hidden_proto.
> 	* sysdeps/i386/i686/multiarch/wcschr-c.c [IS_IN (libc) && SHARED]
> 	(libc_hidden_def): Also define __GI___wcschr alias.
> 	* sysdeps/i386/i686/multiarch/wcschr.S (wcschr): Rename to
> 	__wcschr and define as weak alias of __wcschr.
> 	* sysdeps/powerpc/power6/wcschr.c [!WCSCHR] (WCSCHR): Define as
> 	__wcschr.
> 	[!WCSCHR] (DEFAULT_WCSCHR): Define.
> 	[DEFAULT_WCSCHR] (__wcschr): Use libc_hidden_def.
> 	[DEFAULT_WCSCHR] (wcschr): Define as weak alias of __wcschr.  Use
> 	libc_hidden_weak.  Do not use libc_hidden_def.
> 	* sysdeps/powerpc/powerpc32/power4/multiarch/wcschr-ppc32.c
> 	[IS_IN (libc) && SHARED] (libc_hidden_def): Also define
> 	__GI___wcschr alias.
> 	* sysdeps/powerpc/powerpc32/power4/multiarch/wcschr.c (wcschr):
> 	Rename to __wcschr and define as weak alias of __wcschr.
> 	* sysdeps/powerpc/powerpc64/multiarch/wcschr.c (wcschr): Likewise.
> 	* sysdeps/x86_64/wcschr.S (wcschr): Rename to __wcschr and define
> 	as weak alias of __wcschr.  Use libc_hidden_weak.
> 	* time/alt_digit.c (_nl_get_walt_digit): Use __wcschr instead of
> 	wcschr.
> 	* time/era.c (_nl_init_era_entries): Likewise.
> 	* conform/Makefile (test-xfail-ISO/time.h/linknamespace): Remove
> 	variable.
> 	(test-xfail-XPG3/time.h/linknamespace): Likewise.
>
> diff --git a/conform/Makefile b/conform/Makefile
> index 4a498e4..5c9bc66 100644
> --- a/conform/Makefile
> +++ b/conform/Makefile
> @@ -325,7 +325,6 @@ test-xfail-XOPEN2K/ndbm.h/linknamespace = yes
>  test-xfail-XOPEN2K8/ndbm.h/linknamespace = yes
>
>  # Unsorted expected failures.
> -test-xfail-ISO/time.h/linknamespace = yes
>  test-xfail-ISO99/ctype.h/linknamespace = yes
>  test-xfail-ISO11/ctype.h/linknamespace = yes
>  test-xfail-XPG3/ctype.h/linknamespace = yes
> @@ -334,7 +333,6 @@ test-xfail-XPG3/glob.h/linknamespace = yes
>  test-xfail-XPG3/regex.h/linknamespace = yes
>  test-xfail-XPG3/search.h/linknamespace = yes
>  test-xfail-XPG3/stdio.h/linknamespace = yes
> -test-xfail-XPG3/time.h/linknamespace = yes
>  test-xfail-XPG3/unistd.h/linknamespace = yes
>  test-xfail-XPG3/wordexp.h/linknamespace = yes
>  test-xfail-XPG4/ctype.h/linknamespace = yes
> diff --git a/include/wchar.h b/include/wchar.h
> index 8207a53..7b5e1a1 100644
> --- a/include/wchar.h
> +++ b/include/wchar.h
> @@ -85,6 +85,12 @@ libc_hidden_proto (wcscmp)
>  libc_hidden_proto (wcsftime)
>  libc_hidden_proto (wcsspn)
>  libc_hidden_proto (wcschr)
> +/* The C++ overloading of wcschr means we have to repeat the type to
> +   declare __wcschr instead of using typeof, to avoid errors in C++
> +   tests.  */
> +extern wchar_t *__wcschr (const wchar_t *__wcs, wchar_t __wc)
> +     __THROW __attribute_pure__;
> +libc_hidden_proto (__wcschr)
>  libc_hidden_proto (wcscoll)
>  libc_hidden_proto (wcspbrk)
>
> diff --git a/sysdeps/i386/i686/multiarch/wcschr-c.c b/sysdeps/i386/i686/multiarch/wcschr-c.c
> index 4c5f0f6..786c132 100644
> --- a/sysdeps/i386/i686/multiarch/wcschr-c.c
> +++ b/sysdeps/i386/i686/multiarch/wcschr-c.c
> @@ -4,7 +4,9 @@
>  # ifdef SHARED
>  #  undef libc_hidden_def
>  #  define libc_hidden_def(name) \
> -   __hidden_ver1 (__wcschr_ia32, __GI_wcschr, __wcschr_ia32);
> +   __hidden_ver1 (__wcschr_ia32, __GI_wcschr, __wcschr_ia32); \
> +   strong_alias (__wcschr_ia32, __wcschr_ia32_1); \
> +   __hidden_ver1 (__wcschr_ia32_1, __GI___wcschr, __wcschr_ia32_1);
>  # endif
>  # define WCSCHR  __wcschr_ia32
>  #endif
> diff --git a/sysdeps/i386/i686/multiarch/wcschr.S b/sysdeps/i386/i686/multiarch/wcschr.S
> index 9a80a33..ccbc058 100644
> --- a/sysdeps/i386/i686/multiarch/wcschr.S
> +++ b/sysdeps/i386/i686/multiarch/wcschr.S
> @@ -23,7 +23,7 @@
>
>  #if IS_IN (libc)
>  	.text
> -ENTRY(wcschr)
> +ENTRY(__wcschr)
>  	.type	wcschr, @gnu_indirect_function
>  	pushl	%ebx
>  	cfi_adjust_cfa_offset (4)
> @@ -40,5 +40,6 @@ ENTRY(wcschr)
>  	cfi_adjust_cfa_offset (-4);
>  	cfi_restore (ebx)
>  	ret
> -END(wcschr)
> +END(__wcschr)
> +weak_alias (__wcschr, wcschr)
>  #endif
> diff --git a/sysdeps/powerpc/power6/wcschr.c b/sysdeps/powerpc/power6/wcschr.c
> index 7045677..af5dc8c 100644
> --- a/sysdeps/powerpc/power6/wcschr.c
> +++ b/sysdeps/powerpc/power6/wcschr.c
> @@ -19,7 +19,8 @@
>  #include <wchar.h>
>
>  #ifndef WCSCHR
> -# define WCSCHR wcschr
> +# define WCSCHR __wcschr
> +# define DEFAULT_WCSCHR
>  #endif
>
>  /* Find the first occurrence of WC in WCS.  */
> @@ -86,4 +87,10 @@ WCSCHR (const wchar_t *wcs, const wchar_t wc)
>
>    return NULL;
>  }
> +#ifdef DEFAULT_WCSCHR
> +libc_hidden_def (__wcschr)
> +weak_alias (__wcschr, wcschr)
> +libc_hidden_weak (wcschr)
> +#else
>  libc_hidden_def (wcschr)
> +#endif
> diff --git a/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr-ppc32.c b/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr-ppc32.c
> index 9a067af..d698efc 100644
> --- a/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr-ppc32.c
> +++ b/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr-ppc32.c
> @@ -21,7 +21,9 @@
>  # ifdef SHARED
>  #   undef libc_hidden_def
>  #   define libc_hidden_def(name)  \
> -    __hidden_ver1 (__wcschr_ppc, __GI_wcschr, __wcschr_ppc);
> +    __hidden_ver1 (__wcschr_ppc, __GI_wcschr, __wcschr_ppc); \
> +    strong_alias (__wcschr_ppc, __wcschr_ppc_1); \
> +    __hidden_ver1 (__wcschr_ppc_1, __GI___wcschr, __wcschr_ppc_1);
>  # endif
>  # define WCSCHR  __wcschr_ppc
>  #endif
> diff --git a/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr.c b/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr.c
> index 6b8cd9c..d273d8d 100644
> --- a/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr.c
> +++ b/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr.c
> @@ -25,12 +25,13 @@ extern __typeof (wcschr) __wcschr_ppc attribute_hidden;
>  extern __typeof (wcschr) __wcschr_power6 attribute_hidden;
>  extern __typeof (wcschr) __wcschr_power7 attribute_hidden;
>
> -libc_ifunc (wcschr,
> +libc_ifunc (__wcschr,
>  	     (hwcap & PPC_FEATURE_HAS_VSX)
>               ? __wcschr_power7 :
>  	       (hwcap & PPC_FEATURE_ARCH_2_05)
>  	       ? __wcschr_power6
>               : __wcschr_ppc);
> +weak_alias (__wcschr, wcschr)
>  #else
>  #undef libc_hidden_def
>  #define libc_hidden_def(a)
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/wcschr.c b/sysdeps/powerpc/powerpc64/multiarch/wcschr.c
> index 6b8cd9c..d273d8d 100644
> --- a/sysdeps/powerpc/powerpc64/multiarch/wcschr.c
> +++ b/sysdeps/powerpc/powerpc64/multiarch/wcschr.c
> @@ -25,12 +25,13 @@ extern __typeof (wcschr) __wcschr_ppc attribute_hidden;
>  extern __typeof (wcschr) __wcschr_power6 attribute_hidden;
>  extern __typeof (wcschr) __wcschr_power7 attribute_hidden;
>
> -libc_ifunc (wcschr,
> +libc_ifunc (__wcschr,
>  	     (hwcap & PPC_FEATURE_HAS_VSX)
>               ? __wcschr_power7 :
>  	       (hwcap & PPC_FEATURE_ARCH_2_05)
>  	       ? __wcschr_power6
>               : __wcschr_ppc);
> +weak_alias (__wcschr, wcschr)
>  #else
>  #undef libc_hidden_def
>  #define libc_hidden_def(a)
> diff --git a/sysdeps/x86_64/wcschr.S b/sysdeps/x86_64/wcschr.S
> index faca759..5c2f2d4 100644
> --- a/sysdeps/x86_64/wcschr.S
> +++ b/sysdeps/x86_64/wcschr.S
> @@ -20,7 +20,7 @@
>  #include <sysdep.h>
>
>  	.text
> -ENTRY (wcschr)
> +ENTRY (__wcschr)
>
>  	movd	%rsi, %xmm1
>  	pxor	%xmm2, %xmm2
> @@ -149,6 +149,8 @@ L(return_null):
>  	xor	%rax, %rax
>  	ret
>
> -END (wcschr)
> +END (__wcschr)
>
> -libc_hidden_def(wcschr)
> +libc_hidden_def(__wcschr)
> +weak_alias (__wcschr, wcschr)
> +libc_hidden_weak (wcschr)
> diff --git a/time/alt_digit.c b/time/alt_digit.c
> index bd13abd..36859a9 100644
> --- a/time/alt_digit.c
> +++ b/time/alt_digit.c
> @@ -132,7 +132,7 @@ _nl_get_walt_digit (unsigned int number, struct __locale_data *current)
>  		data->walt_digits[cnt] = ptr;
>
>  		/* Skip digit format. */
> -		ptr = wcschr (ptr, L'\0') + 1;
> +		ptr = __wcschr (ptr, L'\0') + 1;
>  	      }
>  	}
>      }
> diff --git a/time/era.c b/time/era.c
> index fd311c6..5ac75e0 100644
> --- a/time/era.c
> +++ b/time/era.c
> @@ -122,11 +122,11 @@ _nl_init_era_entries (struct __locale_data *current)
>
>  		  /* Set and skip wide era name.  */
>  		  new_eras[cnt].era_wname = (wchar_t *) ptr;
> -		  ptr = (char *) (wcschr ((wchar_t *) ptr, L'\0') + 1);
> +		  ptr = (char *) (__wcschr ((wchar_t *) ptr, L'\0') + 1);
>
>  		  /* Set and skip wide era format.  */
>  		  new_eras[cnt].era_wformat = (wchar_t *) ptr;
> -		  ptr = (char *) (wcschr ((wchar_t *) ptr, L'\0') + 1);
> +		  ptr = (char *) (__wcschr ((wchar_t *) ptr, L'\0') + 1);
>  		}
>  	    }
>  	}
> diff --git a/wcsmbs/wcschr.c b/wcsmbs/wcschr.c
> index 1a35b80..bed66a7 100644
> --- a/wcsmbs/wcschr.c
> +++ b/wcsmbs/wcschr.c
> @@ -20,6 +20,8 @@
>  /* Find the first occurrence of WC in WCS.  */
>  #ifdef WCSCHR
>  # define wcschr WCSCHR
> +#else
> +# define wcschr __wcschr
>  #endif
>
>  wchar_t *
> @@ -35,3 +37,8 @@ wcschr (wcs, wc)
>    return NULL;
>  }
>  libc_hidden_def (wcschr)
> +#ifndef WCSCHR
> +# undef wcschr
> +weak_alias (__wcschr, wcschr)
> +libc_hidden_weak (wcschr)
> +#endif
>

Comments

Joseph Myers Dec. 10, 2014, 12:37 a.m. UTC | #1 
On Tue, 9 Dec 2014, Adhemerval Zanella wrote:

> This patch breaks both powerpc32 and powerpc64 builds and I fixed it by using
> these changes over yours:

This doesn't apply cleanly to me - could you send it as an attachment?

patching file sysdeps/powerpc/powerpc32/power4/multiarch/wcschr.c
Hunk #1 FAILED at 17.
1 out of 1 hunk FAILED -- saving rejects to file sysdeps/powerpc/powerpc32/power4/multiarch/wcschr.c.rej
patching file sysdeps/powerpc/powerpc64/multiarch/wcschr.c
Hunk #1 succeeded at 32 with fuzz 1.
diff mbox

Patch

diff --git a/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr.c b/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr.c
index d273d8d..ae5c517 100644
--- a/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr.c
+++ b/sysdeps/powerpc/powerpc32/power4/multiarch/wcschr.c
@@ -17,23 +17,25 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #if IS_IN (libc)
+# define wcschr __redirect_wcschr
 # include <wchar.h>
 # include <shlib-compat.h>
 # include "init-arch.h"
 
-extern __typeof (wcschr) __wcschr_ppc attribute_hidden;
-extern __typeof (wcschr) __wcschr_power6 attribute_hidden;
-extern __typeof (wcschr) __wcschr_power7 attribute_hidden;
+extern __typeof (__redirect_wcschr) __wcschr_ppc attribute_hidden;
+extern __typeof (__redirect_wcschr) __wcschr_power6 attribute_hidden;
+extern __typeof (__redirect_wcschr) __wcschr_power7 attribute_hidden;
 
-libc_ifunc (__wcschr,
+extern __typeof (__redirect_wcschr) __libc_wcschr;
+
+libc_ifunc (__libc_wcschr,
             (hwcap & PPC_FEATURE_HAS_VSX)
              ? __wcschr_power7 :
               (hwcap & PPC_FEATURE_ARCH_2_05)
               ? __wcschr_power6
              : __wcschr_ppc);
-weak_alias (__wcschr, wcschr)
+#undef wcschr
+weak_alias (__libc_wcschr, wcschr)
 #else
-#undef libc_hidden_def
-#define libc_hidden_def(a)
 #include <wcsmbs/wcschr.c>
 #endif
diff --git a/sysdeps/powerpc/powerpc64/multiarch/wcschr.c b/sysdeps/powerpc/powerpc64/multiarch/wcschr.c
index d273d8d..658cd93 100644
--- a/sysdeps/powerpc/powerpc64/multiarch/wcschr.c
+++ b/sysdeps/powerpc/powerpc64/multiarch/wcschr.c
@@ -32,6 +32,7 @@  libc_ifunc (__wcschr,
               ? __wcschr_power6
              : __wcschr_ppc);
 weak_alias (__wcschr, wcschr)
+libc_hidden_builtin_def (wcschr)
 #else
 #undef libc_hidden_def
 #define libc_hidden_def(a)