[PATCH 0/3] Build fixes for older gcc

[PATCH 0/3] Build fixes for older gcc

[Adhemerval Zanella]

Checking if glibc still builds with the minimum advertised gcc version
showed some issues, which are easily fixed.  There are some issues that
are not easily fixed that either would require GCC backports to old
releases (like ia64 GPREL22 overflows issues again __dso_handle),
a large backport on old glibc releases (riscv R_RISCV_GPREL_I relocation
against __ehdr_start that was fixed by a code refactor only presented
in 2.35), or large compiler backports (like for arc that prevented build
with gcc older than 9).

Adhemerval Zanella (3):
  malloc: Fix set-freeres.c with gcc 6
  sunrpc: Fix netname build with older gcc
  powerpc: Fix powerpc64 strchrnul build with old gcc

 malloc/set-freeres.c                          | 46 +++++++++++++++++++
 sunrpc/netname.c                              |  4 ++
 .../powerpc/powerpc64/multiarch/strchrnul.c   | 14 +++---
 3 files changed, 57 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH 1/3] malloc: Fix set-freeres.c with gcc 6

[Adhemerval Zanella]

Old GCC might trigger the the comparison will always evaluate as ‘true’
warnig for static build:

set-freeres.c:87:14: error: the comparison will always evaluate as
‘true’ for the address of ‘__libc_getgrgid_freemem_ptr’ will never be
NULL [-Werror=address]
   if (&__ptr != NULL)      \

So add pragma weak for all affected usages.

Checked on x86_64 and i686 with gcc 6 and 13.
---
 malloc/set-freeres.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/malloc/set-freeres.c b/malloc/set-freeres.c
index 1aab34b586..41ef9c2dd6 100644
--- a/malloc/set-freeres.c
+++ b/malloc/set-freeres.c
@@ -63,6 +63,52 @@
 # pragma weak __libpthread_freeres
 # pragma weak __libc_dlerror_result_free
 # pragma weak __check_pf_freemem
+# pragma weak __libc_fgetgrent_freemem_ptr
+# pragma weak __libc_fgetsgent_freeres_ptr
+# pragma weak __libc_getnetgrent_freemem_ptr
+# pragma weak __libc_rcmd_freemem_ptr
+# pragma weak __libc_rexec_freemem_ptr
+# pragma weak __libc_mntent_freemem_ptr
+# pragma weak __libc_fgetpwent_freemem_ptr
+# pragma weak __libc_getspent_freemem_ptr
+# pragma weak __libc_resolv_res_hconf_freemem_ptr
+# pragma weak __libc_fgetspent_freemem_ptr
+# pragma weak __libc_tzfile_freemem_ptr
+# pragma weak __libc_getnameinfo_freemem_ptr
+# pragma weak __libc_getutent_freemem_ptr
+# pragma weak __libc_getutid_freemem_ptr
+# pragma weak __libc_getutline_freemem_ptr
+# pragma weak __libc_reg_printf_freemem_ptr
+# pragma weak __libc_reg_type_freemem_ptr
+# pragma weak __libc_getgrgid_freemem_ptr
+# pragma weak __libc_getgrnam_freemem_ptr
+# pragma weak __libc_getpwnam_freemem_ptr
+# pragma weak __libc_getpwuid_freemem_ptr
+# pragma weak __libc_getspnam_freemem_ptr
+# pragma weak __libc_getaliasbyname_freemem_ptr
+# pragma weak __libc_gethostbyaddr_freemem_ptr
+# pragma weak __libc_gethostbyname_freemem_ptr
+# pragma weak __libc_gethostbyname2_freemem_ptr
+# pragma weak __libc_getnetbyaddr_freemem_ptr
+# pragma weak __libc_getnetbyname_freemem_ptr
+# pragma weak __libc_getprotobynumber_freemem_ptr
+# pragma weak __libc_getprotobyname_freemem_ptr
+# pragma weak __libc_getrpcbyname_freemem_ptr
+# pragma weak __libc_getrpcbynumber_freemem_ptr
+# pragma weak __libc_getservbyname_freemem_ptr
+# pragma weak __libc_getservbyport_freemem_ptr
+# pragma weak __libc_getgrent_freemem_ptr
+# pragma weak __libc_getpwent_freemem_ptr
+# pragma weak __libc_getaliasent_freemem_ptr
+# pragma weak __libc_gethostent_freemem_ptr
+# pragma weak __libc_getnetent_freemem_ptr
+# pragma weak __libc_getprotoent_freemem_ptr
+# pragma weak __libc_getrpcent_freemem_ptr
+# pragma weak __libc_getservent_freemem_ptr
+# pragma weak __libc_efgcvt_freemem_ptr
+# pragma weak __libc_qefgcvt_freemem_ptr
+# pragma weak __libc_qefgcvt_freemem_ptr
+# pragma weak __ttyname_freemem_ptr
 #endif
 
 #ifdef SHARED
-- 
2.34.1

Re: [PATCH 1/3] malloc: Fix set-freeres.c with gcc 6

[Carlos O'Donell]

On 7/24/23 13:15, Adhemerval Zanella via Libc-alpha wrote:
> Old GCC might trigger the the comparison will always evaluate as ‘true’
> warnig for static build:
> 
> set-freeres.c:87:14: error: the comparison will always evaluate as
> ‘true’ for the address of ‘__libc_getgrgid_freemem_ptr’ will never be
> NULL [-Werror=address]
>    if (&__ptr != NULL)      \
> 
> So add pragma weak for all affected usages.

Agreed, if these work for older gcc we should put prama weak for them.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

 
> Checked on x86_64 and i686 with gcc 6 and 13.
> ---
>  malloc/set-freeres.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/malloc/set-freeres.c b/malloc/set-freeres.c
> index 1aab34b586..41ef9c2dd6 100644
> --- a/malloc/set-freeres.c
> +++ b/malloc/set-freeres.c
> @@ -63,6 +63,52 @@
>  # pragma weak __libpthread_freeres
>  # pragma weak __libc_dlerror_result_free
>  # pragma weak __check_pf_freemem
> +# pragma weak __libc_fgetgrent_freemem_ptr
> +# pragma weak __libc_fgetsgent_freeres_ptr
> +# pragma weak __libc_getnetgrent_freemem_ptr
> +# pragma weak __libc_rcmd_freemem_ptr
> +# pragma weak __libc_rexec_freemem_ptr
> +# pragma weak __libc_mntent_freemem_ptr
> +# pragma weak __libc_fgetpwent_freemem_ptr
> +# pragma weak __libc_getspent_freemem_ptr
> +# pragma weak __libc_resolv_res_hconf_freemem_ptr
> +# pragma weak __libc_fgetspent_freemem_ptr
> +# pragma weak __libc_tzfile_freemem_ptr
> +# pragma weak __libc_getnameinfo_freemem_ptr
> +# pragma weak __libc_getutent_freemem_ptr
> +# pragma weak __libc_getutid_freemem_ptr
> +# pragma weak __libc_getutline_freemem_ptr
> +# pragma weak __libc_reg_printf_freemem_ptr
> +# pragma weak __libc_reg_type_freemem_ptr
> +# pragma weak __libc_getgrgid_freemem_ptr
> +# pragma weak __libc_getgrnam_freemem_ptr
> +# pragma weak __libc_getpwnam_freemem_ptr
> +# pragma weak __libc_getpwuid_freemem_ptr
> +# pragma weak __libc_getspnam_freemem_ptr
> +# pragma weak __libc_getaliasbyname_freemem_ptr
> +# pragma weak __libc_gethostbyaddr_freemem_ptr
> +# pragma weak __libc_gethostbyname_freemem_ptr
> +# pragma weak __libc_gethostbyname2_freemem_ptr
> +# pragma weak __libc_getnetbyaddr_freemem_ptr
> +# pragma weak __libc_getnetbyname_freemem_ptr
> +# pragma weak __libc_getprotobynumber_freemem_ptr
> +# pragma weak __libc_getprotobyname_freemem_ptr
> +# pragma weak __libc_getrpcbyname_freemem_ptr
> +# pragma weak __libc_getrpcbynumber_freemem_ptr
> +# pragma weak __libc_getservbyname_freemem_ptr
> +# pragma weak __libc_getservbyport_freemem_ptr
> +# pragma weak __libc_getgrent_freemem_ptr
> +# pragma weak __libc_getpwent_freemem_ptr
> +# pragma weak __libc_getaliasent_freemem_ptr
> +# pragma weak __libc_gethostent_freemem_ptr
> +# pragma weak __libc_getnetent_freemem_ptr
> +# pragma weak __libc_getprotoent_freemem_ptr
> +# pragma weak __libc_getrpcent_freemem_ptr
> +# pragma weak __libc_getservent_freemem_ptr
> +# pragma weak __libc_efgcvt_freemem_ptr
> +# pragma weak __libc_qefgcvt_freemem_ptr
> +# pragma weak __libc_qefgcvt_freemem_ptr
> +# pragma weak __ttyname_freemem_ptr
>  #endif
>  
>  #ifdef SHARED

-- 
Cheers,
Carlos.

[PATCH 2/3] sunrpc: Fix netname build with older gcc

[Adhemerval Zanella]

The -Wformat-overflow was added by gcc 8.
---
 sunrpc/netname.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sunrpc/netname.c b/sunrpc/netname.c
index d9d82501b0..9e2f78329e 100644
--- a/sunrpc/netname.c
+++ b/sunrpc/netname.c
@@ -52,10 +52,14 @@ user2netname (char netname[MAXNETNAMELEN + 1], const uid_t uid,
   /* GCC with -Os or -O1 warns that sprint might overflow while handling
      dfltdom, however the above test does check if an overflow would
      happen.  */
+#if __GNUC_PREREQ (8, 0)
   DIAG_PUSH_NEEDS_COMMENT;
   DIAG_IGNORE_NEEDS_COMMENT (8, "-Wformat-overflow");
+#endif
   sprintf (netname, "%s.%d@%s", OPSYS, uid, dfltdom);
+#if __GNUC_PREREQ (8, 0)
   DIAG_POP_NEEDS_COMMENT;
+#endif
   i = strlen (netname);
   if (netname[i - 1] == '.')
     netname[i - 1] = '\0';
-- 
2.34.1

Re: [PATCH 2/3] sunrpc: Fix netname build with older gcc

[Carlos O'Donell]

On 7/24/23 13:15, Adhemerval Zanella via Libc-alpha wrote:
> The -Wformat-overflow was added by gcc 8.

LGTM.

These are already approved by the RM to clean these up.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  sunrpc/netname.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/sunrpc/netname.c b/sunrpc/netname.c
> index d9d82501b0..9e2f78329e 100644
> --- a/sunrpc/netname.c
> +++ b/sunrpc/netname.c
> @@ -52,10 +52,14 @@ user2netname (char netname[MAXNETNAMELEN + 1], const uid_t uid,
>    /* GCC with -Os or -O1 warns that sprint might overflow while handling
>       dfltdom, however the above test does check if an overflow would
>       happen.  */
> +#if __GNUC_PREREQ (8, 0)
>    DIAG_PUSH_NEEDS_COMMENT;
>    DIAG_IGNORE_NEEDS_COMMENT (8, "-Wformat-overflow");
> +#endif
>    sprintf (netname, "%s.%d@%s", OPSYS, uid, dfltdom);
> +#if __GNUC_PREREQ (8, 0)
>    DIAG_POP_NEEDS_COMMENT;
> +#endif
>    i = strlen (netname);
>    if (netname[i - 1] == '.')
>      netname[i - 1] = '\0';

-- 
Cheers,
Carlos.

[PATCH 3/3] powerpc: Fix powerpc64 strchrnul build with old gcc

[Adhemerval Zanella]

The compiler might not see that internal definition is an alias
due the libc_ifunc macro, which redefines __strchrnul.  With
gcc 6 it fails with:

In file included from <command-line>:0:0:
./../include/libc-symbols.h:472:33: error: ‘__EI___strchrnul’ aliased to
undefined symbol ‘__GI___strchrnul’
   extern thread __typeof (name) __EI_##name \
                                 ^
./../include/libc-symbols.h:468:3: note: in expansion of macro
‘__hidden_ver2’
   __hidden_ver2 (, local, internal, name)
   ^~~~~~~~~~~~~
./../include/libc-symbols.h:476:29: note: in expansion of macro
‘__hidden_ver1’
 #  define hidden_def(name)  __hidden_ver1(__GI_##name, name, name);
                             ^~~~~~~~~~~~~
./../include/libc-symbols.h:557:32: note: in expansion of macro
‘hidden_def’
 # define libc_hidden_def(name) hidden_def (name)
                                ^~~~~~~~~~
../sysdeps/powerpc/powerpc64/multiarch/strchrnul.c:38:1: note: in
expansion of macro ‘libc_hidden_def’
 libc_hidden_def (__strchrnul)
 ^~~~~~~~~~~~~~~

Use libc_ifunc_hidden as stpcpy.  Checked on powerpc64 with
gcc 6 and gcc 13.
---
 sysdeps/powerpc/powerpc64/multiarch/strchrnul.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/sysdeps/powerpc/powerpc64/multiarch/strchrnul.c b/sysdeps/powerpc/powerpc64/multiarch/strchrnul.c
index 94873507a6..87dd41f72d 100644
--- a/sysdeps/powerpc/powerpc64/multiarch/strchrnul.c
+++ b/sysdeps/powerpc/powerpc64/multiarch/strchrnul.c
@@ -27,13 +27,13 @@ extern __typeof (__strchrnul) __strchrnul_power8 attribute_hidden;
 
 /* Avoid DWARF definition DIE on ifunc symbol so that GDB can handle
    ifunc symbol properly.  */
-libc_ifunc (__strchrnul,
-	    (hwcap2 & PPC_FEATURE2_ARCH_2_07
-	     && hwcap & PPC_FEATURE_HAS_ALTIVEC)
-	    ? __strchrnul_power8 :
-	    (hwcap & PPC_FEATURE_ARCH_2_06)
-            ? __strchrnul_power7
-            : __strchrnul_ppc);
+libc_ifunc_hidden (__strchrnul, __strchrnul,
+		   (hwcap2 & PPC_FEATURE2_ARCH_2_07
+		    && hwcap & PPC_FEATURE_HAS_ALTIVEC)
+		   ? __strchrnul_power8 :
+		   (hwcap & PPC_FEATURE_ARCH_2_06)
+		   ? __strchrnul_power7
+		   : __strchrnul_ppc);
 
 libc_hidden_def (__strchrnul)
 weak_alias (__strchrnul, strchrnul)
-- 
2.34.1

Re: [PATCH 3/3] powerpc: Fix powerpc64 strchrnul build with old gcc

[Carlos O'Donell]

On 7/24/23 13:15, Adhemerval Zanella via Libc-alpha wrote:
> The compiler might not see that internal definition is an alias
> due the libc_ifunc macro, which redefines __strchrnul.  With
> gcc 6 it fails with:
> 
> In file included from <command-line>:0:0:
> ./../include/libc-symbols.h:472:33: error: ‘__EI___strchrnul’ aliased to
> undefined symbol ‘__GI___strchrnul’
>    extern thread __typeof (name) __EI_##name \
>                                  ^
> ./../include/libc-symbols.h:468:3: note: in expansion of macro
> ‘__hidden_ver2’
>    __hidden_ver2 (, local, internal, name)
>    ^~~~~~~~~~~~~
> ./../include/libc-symbols.h:476:29: note: in expansion of macro
> ‘__hidden_ver1’
>  #  define hidden_def(name)  __hidden_ver1(__GI_##name, name, name);
>                              ^~~~~~~~~~~~~
> ./../include/libc-symbols.h:557:32: note: in expansion of macro
> ‘hidden_def’
>  # define libc_hidden_def(name) hidden_def (name)
>                                 ^~~~~~~~~~
> ../sysdeps/powerpc/powerpc64/multiarch/strchrnul.c:38:1: note: in
> expansion of macro ‘libc_hidden_def’
>  libc_hidden_def (__strchrnul)
>  ^~~~~~~~~~~~~~~
> 
> Use libc_ifunc_hidden as stpcpy.  Checked on powerpc64 with
> gcc 6 and gcc 13.

Using the _hidden variant here is OK.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>


> ---
>  sysdeps/powerpc/powerpc64/multiarch/strchrnul.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/strchrnul.c b/sysdeps/powerpc/powerpc64/multiarch/strchrnul.c
> index 94873507a6..87dd41f72d 100644
> --- a/sysdeps/powerpc/powerpc64/multiarch/strchrnul.c
> +++ b/sysdeps/powerpc/powerpc64/multiarch/strchrnul.c
> @@ -27,13 +27,13 @@ extern __typeof (__strchrnul) __strchrnul_power8 attribute_hidden;
>  
>  /* Avoid DWARF definition DIE on ifunc symbol so that GDB can handle
>     ifunc symbol properly.  */
> -libc_ifunc (__strchrnul,
> -	    (hwcap2 & PPC_FEATURE2_ARCH_2_07
> -	     && hwcap & PPC_FEATURE_HAS_ALTIVEC)
> -	    ? __strchrnul_power8 :
> -	    (hwcap & PPC_FEATURE_ARCH_2_06)
> -            ? __strchrnul_power7
> -            : __strchrnul_ppc);
> +libc_ifunc_hidden (__strchrnul, __strchrnul,
> +		   (hwcap2 & PPC_FEATURE2_ARCH_2_07
> +		    && hwcap & PPC_FEATURE_HAS_ALTIVEC)
> +		   ? __strchrnul_power8 :
> +		   (hwcap & PPC_FEATURE_ARCH_2_06)
> +		   ? __strchrnul_power7
> +		   : __strchrnul_ppc);
>  
>  libc_hidden_def (__strchrnul)
>  weak_alias (__strchrnul, strchrnul)

-- 
Cheers,
Carlos.

Re: [PATCH 0/3] Build fixes for older gcc

[Andreas K. Huettel]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

Am Montag, 24. Juli 2023, 19:15:21 CEST schrieb Adhemerval Zanella via Libc-alpha:
> Checking if glibc still builds with the minimum advertised gcc version
> showed some issues, which are easily fixed.  There are some issues that
> are not easily fixed that either would require GCC backports to old
> releases (like ia64 GPREL22 overflows issues again __dso_handle),
> a large backport on old glibc releases (riscv R_RISCV_GPREL_I relocation
> against __ehdr_start that was fixed by a code refactor only presented
> in 2.35), or large compiler backports (like for arc that prevented build
> with gcc older than 9).
> 
> Adhemerval Zanella (3):
>   malloc: Fix set-freeres.c with gcc 6
>   sunrpc: Fix netname build with older gcc
>   powerpc: Fix powerpc64 strchrnul build with old gcc
> 
>  malloc/set-freeres.c                          | 46 +++++++++++++++++++
>  sunrpc/netname.c                              |  4 ++
>  .../powerpc/powerpc64/multiarch/strchrnul.c   | 14 +++---
>  3 files changed, 57 insertions(+), 7 deletions(-)

It would be great to still have this in the release.

That said, I can't technically judge it, so a second OK would be nice.

-- 
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, toolchain, base-system, perl, libreoffice)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 981 bytes --]

Re: [PATCH 0/3] Build fixes for older gcc

[Adhemerval Zanella Netto]

On 25/07/23 17:06, Andreas K. Huettel wrote:
> Am Montag, 24. Juli 2023, 19:15:21 CEST schrieb Adhemerval Zanella via Libc-alpha:
>> Checking if glibc still builds with the minimum advertised gcc version
>> showed some issues, which are easily fixed.  There are some issues that
>> are not easily fixed that either would require GCC backports to old
>> releases (like ia64 GPREL22 overflows issues again __dso_handle),
>> a large backport on old glibc releases (riscv R_RISCV_GPREL_I relocation
>> against __ehdr_start that was fixed by a code refactor only presented
>> in 2.35), or large compiler backports (like for arc that prevented build
>> with gcc older than 9).
>>
>> Adhemerval Zanella (3):
>>   malloc: Fix set-freeres.c with gcc 6
>>   sunrpc: Fix netname build with older gcc
>>   powerpc: Fix powerpc64 strchrnul build with old gcc
>>
>>  malloc/set-freeres.c                          | 46 +++++++++++++++++++
>>  sunrpc/netname.c                              |  4 ++
>>  .../powerpc/powerpc64/multiarch/strchrnul.c   | 14 +++---
>>  3 files changed, 57 insertions(+), 7 deletions(-)
> 
> It would be great to still have this in the release.
> 
> That said, I can't technically judge it, so a second OK would be nice.
> 

We can always backport it since it should only affect old compilers.