[PATCH 0/4] Fix i386 build with --enable-fortify-source

[PATCH 0/4] Fix i386 build with --enable-fortify-source

[Adhemerval Zanella]

Building i686-linux-gnu with --enable-fortify=1,2,3 on gcc from 6
to 13 triggers some issues:

  - gcc 6 issues an warning building string/tester.c where strncat
    fortify builtin might overflow the destination buffer.

  - gcc 12 and 13 with --enable-fortify=3 uncovered an issue with
    static linking, where multiple internal chk symbols for memcpy,
    memmove, and mempcpy are being provided.

A clean build/check for gcc 6 requires my previous set [1].  With
both applied I see to build/check regression for all supported
--enable-fortify with all supported gcc on i686-linux-gnu.

[1] https://patchwork.sourceware.org/project/glibc/list/?series=22751.


Adhemerval Zanella (4):
  i386: Fix build with --enable-fortify=3
  i386: Remove memset_chk-nonshared.S
  linux: Fix i686 with gcc6
  string: Fix tester build with fortify enable with gcc 6

 string/tester.c                               |  9 ++++++++
 sysdeps/i386/i686/memset.S                    |  2 +-
 sysdeps/i386/i686/multiarch/Makefile          |  5 -----
 .../i686/multiarch/memcpy_chk-nonshared.S     | 21 -------------------
 .../i686/multiarch/memmove_chk-nonshared.S    | 21 -------------------
 .../i686/multiarch/mempcpy_chk-nonshared.S    | 21 -------------------
 sysdeps/i386/i686/multiarch/memset-ia32.S     |  9 ++++----
 .../i686/multiarch/memset_chk-nonshared.S     | 21 -------------------
 .../unix/sysv/linux/convert_scm_timestamps.c  |  9 ++++++++
 9 files changed, 24 insertions(+), 94 deletions(-)
 delete mode 100644 sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S
 delete mode 100644 sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S
 delete mode 100644 sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S
 delete mode 100644 sysdeps/i386/i686/multiarch/memset_chk-nonshared.S

-- 
2.34.1

[PATCH 1/4] i386: Fix build with --enable-fortify=3

[Adhemerval Zanella]

The i386 string routines provide multiple internal definitions
for memcpy, memmove, and mempcpy chk routines:

  $ objdump -t libc.a | grep __memcpy_chk
  00000000 g     F .text  0000000e __memcpy_chk
  00000000 g     F .text  00000013 __memcpy_chk
  $ objdump -t libc.a | grep __mempcpy_chk
  00000000 g     F .text  0000000e __mempcpy_chk
  00000000 g     F .text  00000013 __mempcpy_chk
  $ objdump -t libc.a | grep __memmove_chk
  00000000 g     F .text  0000000e __memmove_chk
  00000000 g     F .text  00000013 __memmove_chk

Although is not an issue for normal static builds, with fortify=3
glibc itself might use the fortify chk functions and thus static
build might fail with multiple definitions.  For instance:

x86_64-glibc-linux-gnu-gcc -m32 -march=i686 -o [...]math/test-signgam-uchar-static -nostdlib -nostartfiles -static -static-pie [...]
x86_64-glibc-linux-gnu/bin/ld: [...]/libc.a(mempcpy-ia32.o):
in function `__mempcpy_chk': [...]/glibc-git/string/../sysdeps/i386/i686/mempcpy.S:32: multiple definition of `__mempcpy_chk';
[...]/libc.a(mempcpy_chk-nonshared.o):[...]/debug/../sysdeps/i386/mempcpy_chk.S:28: first defined here
collect2: error: ld returned 1 exit status
make[2]: *** [../Rules:298:

There is no need for mem*-nonshared.S, the __mem*_chk routines
are already provided by the assembly routines.

Checked on i686-linux-gnu with gcc 13 built with fortify=1,2,3 and
without fortify.
---
 sysdeps/i386/i686/multiarch/Makefile          |  3 +--
 .../i686/multiarch/memcpy_chk-nonshared.S     | 21 -------------------
 .../i686/multiarch/memmove_chk-nonshared.S    | 21 -------------------
 .../i686/multiarch/mempcpy_chk-nonshared.S    | 21 -------------------
 4 files changed, 1 insertion(+), 65 deletions(-)
 delete mode 100644 sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S
 delete mode 100644 sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S
 delete mode 100644 sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S

diff --git a/sysdeps/i386/i686/multiarch/Makefile b/sysdeps/i386/i686/multiarch/Makefile
index 9fe5ea8639..f48b06741f 100644
--- a/sysdeps/i386/i686/multiarch/Makefile
+++ b/sysdeps/i386/i686/multiarch/Makefile
@@ -46,6 +46,5 @@ CFLAGS-s_fmaf-fma.c += -mavx -mfpmath=sse
 endif
 
 ifeq ($(subdir),debug)
-sysdep_routines += memcpy_chk-nonshared mempcpy_chk-nonshared \
-		   memmove_chk-nonshared memset_chk-nonshared
+sysdep_routines += memset_chk-nonshared
 endif
diff --git a/sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S b/sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S
deleted file mode 100644
index 6801893b72..0000000000
--- a/sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Non-shared version of memcpy_chk for i686.
-   Copyright (C) 2017-2023 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#if IS_IN (libc) && !defined SHARED
-# include <sysdeps/i386/memcpy_chk.S>
-#endif
diff --git a/sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S b/sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S
deleted file mode 100644
index c5de28d4a8..0000000000
--- a/sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Non-shared version of memmmove_chk for i686.
-   Copyright (C) 2017-2023 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#if IS_IN (libc) && !defined SHARED
-# include <sysdeps/i386/memmove_chk.S>
-#endif
diff --git a/sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S b/sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S
deleted file mode 100644
index c4f89e156b..0000000000
--- a/sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Non-shared version of mempcpy_chk for i686.
-   Copyright (C) 2017-2023 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#if IS_IN (libc) && !defined SHARED
-# include <sysdeps/i386/mempcpy_chk.S>
-#endif
-- 
2.34.1

[PATCH 2/4] i386: Remove memset_chk-nonshared.S

[Adhemerval Zanella]

Similar to memcpy, mempcpy, and memmove there is no need for an
specific memset_chk-nonshared.S.  It can be provided by
memset-ia32.S itself for static library.

Checked on i686-linux-gnu.
---
 sysdeps/i386/i686/memset.S                    |  2 +-
 sysdeps/i386/i686/multiarch/Makefile          |  4 ----
 sysdeps/i386/i686/multiarch/memset-ia32.S     |  9 ++++----
 .../i686/multiarch/memset_chk-nonshared.S     | 21 -------------------
 4 files changed, 6 insertions(+), 30 deletions(-)
 delete mode 100644 sysdeps/i386/i686/multiarch/memset_chk-nonshared.S

diff --git a/sysdeps/i386/i686/memset.S b/sysdeps/i386/i686/memset.S
index 0b5e671d83..b84dc3fbe9 100644
--- a/sysdeps/i386/i686/memset.S
+++ b/sysdeps/i386/i686/memset.S
@@ -27,7 +27,7 @@
 #define LEN	CHR+4
 
         .text
-#if defined SHARED && IS_IN (libc)
+#if defined PIC && IS_IN (libc)
 ENTRY_CHK (__memset_chk)
 	movl	12(%esp), %eax
 	cmpl	%eax, 16(%esp)
diff --git a/sysdeps/i386/i686/multiarch/Makefile b/sysdeps/i386/i686/multiarch/Makefile
index f48b06741f..f86e69de55 100644
--- a/sysdeps/i386/i686/multiarch/Makefile
+++ b/sysdeps/i386/i686/multiarch/Makefile
@@ -44,7 +44,3 @@ libm-sysdep_routines += s_fma-fma s_fmaf-fma
 CFLAGS-s_fma-fma.c += -mavx -mfpmath=sse
 CFLAGS-s_fmaf-fma.c += -mavx -mfpmath=sse
 endif
-
-ifeq ($(subdir),debug)
-sysdep_routines += memset_chk-nonshared
-endif
diff --git a/sysdeps/i386/i686/multiarch/memset-ia32.S b/sysdeps/i386/i686/multiarch/memset-ia32.S
index ac57e456d4..a1b3919a19 100644
--- a/sysdeps/i386/i686/multiarch/memset-ia32.S
+++ b/sysdeps/i386/i686/multiarch/memset-ia32.S
@@ -18,16 +18,17 @@
 
 #if IS_IN (libc)
 # define memset __memset_ia32
-# define __memset_chk __memset_chk_ia32
 
 # ifdef SHARED
-#  undef libc_hidden_builtin_def
+#  define __memset_chk __memset_chk_ia32
+# endif
+
+# undef libc_hidden_builtin_def
 /* IFUNC doesn't work with the hidden functions in shared library since
    they will be called without setting up EBX needed for PLT which is
    used by IFUNC.  */
-#  define libc_hidden_builtin_def(name) \
+# define libc_hidden_builtin_def(name) \
 	.globl __GI_memset; __GI_memset = memset
-# endif
 #endif
 
 #include <sysdeps/i386/i686/memset.S>
diff --git a/sysdeps/i386/i686/multiarch/memset_chk-nonshared.S b/sysdeps/i386/i686/multiarch/memset_chk-nonshared.S
deleted file mode 100644
index e68c6e43b3..0000000000
--- a/sysdeps/i386/i686/multiarch/memset_chk-nonshared.S
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Non-shared version of memset_chk for i686.
-   Copyright (C) 2017-2023 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#if IS_IN (libc) && !defined SHARED
-# include <sysdeps/i386/memset_chk.S>
-#endif
-- 
2.34.1

[PATCH 3/4] linux: Fix i686 with gcc6

[Adhemerval Zanella]

On __convert_scm_timestamps GCC 6 issues an warning that tvts[0]/tvts[1]
maybe be used uninitialized, however it would be used if type is set to a
value different than 0 (done by either COMPAT_SO_TIMESTAMP_OLD or
COMPAT_SO_TIMESTAMPNS_OLD) which will fallthrough to 'common' label.

It does not show with gcc 7 or more recent versions.

Checked on i686-linux-gnu.
---
 sysdeps/unix/sysv/linux/convert_scm_timestamps.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
index 42f9613416..06c8adeee1 100644
--- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
+++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
@@ -23,6 +23,7 @@
 # include <string.h>
 # include <sys/socket.h>
 # include <socket-constants-time64.h>
+# include <libc-diag.h>
 
 /* It converts the first SO_TIMESTAMP or SO_TIMESTAMPNS with 32-bit time and
    appends it to the control buffer.  The 32-bit time field is kept as-is.
@@ -44,7 +45,15 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
      'struct __kernel_sock_timeval' while for SO_TIMESTAMPNS_NEW is a
      'struct __kernel_timespec'.  In either case it is two uint64_t
      members.  */
+
+  /* GCC 6 issues an warning that tvts[0]/tvts[1] maybe be used uninitialized,
+     however it would be used if type is set to a value different than 0
+     (done by either COMPAT_SO_TIMESTAMP_OLD or COMPAT_SO_TIMESTAMPNS_OLD)
+     which will fallthrough to 'common' label.  */
+  DIAG_PUSH_NEEDS_COMMENT;
+  DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
   int64_t tvts[2];
+  DIAG_POP_NEEDS_COMMENT;
   int32_t tmp[2];
 
   struct cmsghdr *cmsg, *last = NULL;
-- 
2.34.1

[PATCH 4/4] string: Fix tester build with fortify enable with gcc 6

[Adhemerval Zanella]

When building with fortify enabled, GCC 6 issues an warning the fortify
wrapper might overflow the destination buffer.  However, GCC does not
provide a specific flag to disable the warning (the failure is tied to
-Werror).  So to avoid disable all errors, only enable the check for
GCC 7 or newer.

Checked on i686-linux-gnu.
---
 string/tester.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/string/tester.c b/string/tester.c
index da42c72141..f7d4bac5a8 100644
--- a/string/tester.c
+++ b/string/tester.c
@@ -385,8 +385,17 @@ test_strncat (void)
 
   (void) strcpy (one, "gh");
   (void) strcpy (two, "ef");
+  /* When building with fortify enabled, GCC 6 issues an warning the fortify
+     wrapper might overflow the destination buffer.  However, GCC does not
+     provide a specific flag to disable the warning (the failure is tied to
+     -Werror).  So to avoid disable all errors, only enable the check for
+     GCC 7 or newer.  */
+#if __GNUC_PREREQ (7, 0)
   (void) strncat (one, two, 99);
   equal (one, "ghef", 5);			/* Basic test encore. */
+#else
+  equal (one, "gh", 2);
+#endif
   equal (two, "ef", 6);			/* Stomped on source? */
 
   (void) strcpy (one, "");
-- 
2.34.1

Re: [PATCH 1/4] i386: Fix build with --enable-fortify=3

[Carlos O'Donell]

On 7/25/23 11:16, Adhemerval Zanella via Libc-alpha wrote:
> The i386 string routines provide multiple internal definitions
> for memcpy, memmove, and mempcpy chk routines:
> 
>   $ objdump -t libc.a | grep __memcpy_chk
>   00000000 g     F .text  0000000e __memcpy_chk
>   00000000 g     F .text  00000013 __memcpy_chk
>   $ objdump -t libc.a | grep __mempcpy_chk
>   00000000 g     F .text  0000000e __mempcpy_chk
>   00000000 g     F .text  00000013 __mempcpy_chk
>   $ objdump -t libc.a | grep __memmove_chk
>   00000000 g     F .text  0000000e __memmove_chk
>   00000000 g     F .text  00000013 __memmove_chk
> 
> Although is not an issue for normal static builds, with fortify=3
> glibc itself might use the fortify chk functions and thus static
> build might fail with multiple definitions.  For instance:
> 
> x86_64-glibc-linux-gnu-gcc -m32 -march=i686 -o [...]math/test-signgam-uchar-static -nostdlib -nostartfiles -static -static-pie [...]
> x86_64-glibc-linux-gnu/bin/ld: [...]/libc.a(mempcpy-ia32.o):
> in function `__mempcpy_chk': [...]/glibc-git/string/../sysdeps/i386/i686/mempcpy.S:32: multiple definition of `__mempcpy_chk';
> [...]/libc.a(mempcpy_chk-nonshared.o):[...]/debug/../sysdeps/i386/mempcpy_chk.S:28: first defined here
> collect2: error: ld returned 1 exit status
> make[2]: *** [../Rules:298:
> 
> There is no need for mem*-nonshared.S, the __mem*_chk routines
> are already provided by the assembly routines.
> 
> Checked on i686-linux-gnu with gcc 13 built with fortify=1,2,3 and
> without fortify.

I agree this is probably the best fix for glibc 2.38.

I think we should commit this for 2.38 and I'll re-test with i686.

Andreas, As the RM is this OK to push?

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

> ---
>  sysdeps/i386/i686/multiarch/Makefile          |  3 +--
>  .../i686/multiarch/memcpy_chk-nonshared.S     | 21 -------------------
>  .../i686/multiarch/memmove_chk-nonshared.S    | 21 -------------------
>  .../i686/multiarch/mempcpy_chk-nonshared.S    | 21 -------------------
>  4 files changed, 1 insertion(+), 65 deletions(-)
>  delete mode 100644 sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S
>  delete mode 100644 sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S
>  delete mode 100644 sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S
> 
> diff --git a/sysdeps/i386/i686/multiarch/Makefile b/sysdeps/i386/i686/multiarch/Makefile
> index 9fe5ea8639..f48b06741f 100644
> --- a/sysdeps/i386/i686/multiarch/Makefile
> +++ b/sysdeps/i386/i686/multiarch/Makefile
> @@ -46,6 +46,5 @@ CFLAGS-s_fmaf-fma.c += -mavx -mfpmath=sse
>  endif
>  
>  ifeq ($(subdir),debug)
> -sysdep_routines += memcpy_chk-nonshared mempcpy_chk-nonshared \
> -		   memmove_chk-nonshared memset_chk-nonshared
> +sysdep_routines += memset_chk-nonshared
>  endif
> diff --git a/sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S b/sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S
> deleted file mode 100644
> index 6801893b72..0000000000
> --- a/sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -/* Non-shared version of memcpy_chk for i686.
> -   Copyright (C) 2017-2023 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#if IS_IN (libc) && !defined SHARED
> -# include <sysdeps/i386/memcpy_chk.S>
> -#endif
> diff --git a/sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S b/sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S
> deleted file mode 100644
> index c5de28d4a8..0000000000
> --- a/sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -/* Non-shared version of memmmove_chk for i686.
> -   Copyright (C) 2017-2023 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#if IS_IN (libc) && !defined SHARED
> -# include <sysdeps/i386/memmove_chk.S>
> -#endif
> diff --git a/sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S b/sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S
> deleted file mode 100644
> index c4f89e156b..0000000000
> --- a/sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -/* Non-shared version of mempcpy_chk for i686.
> -   Copyright (C) 2017-2023 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#if IS_IN (libc) && !defined SHARED
> -# include <sysdeps/i386/mempcpy_chk.S>
> -#endif

-- 
Cheers,
Carlos.

Re: [PATCH 2/4] i386: Remove memset_chk-nonshared.S

[Carlos O'Donell]

On 7/25/23 11:16, Adhemerval Zanella via Libc-alpha wrote:
> Similar to memcpy, mempcpy, and memmove there is no need for an
> specific memset_chk-nonshared.S.  It can be provided by
> memset-ia32.S itself for static library.

Agreed. LGTM. I'll retest i686 after this goes in.

Andreas, As the RM is this OK to push?

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

> Checked on i686-linux-gnu.
> ---
>  sysdeps/i386/i686/memset.S                    |  2 +-
>  sysdeps/i386/i686/multiarch/Makefile          |  4 ----
>  sysdeps/i386/i686/multiarch/memset-ia32.S     |  9 ++++----
>  .../i686/multiarch/memset_chk-nonshared.S     | 21 -------------------
>  4 files changed, 6 insertions(+), 30 deletions(-)
>  delete mode 100644 sysdeps/i386/i686/multiarch/memset_chk-nonshared.S
> 
> diff --git a/sysdeps/i386/i686/memset.S b/sysdeps/i386/i686/memset.S
> index 0b5e671d83..b84dc3fbe9 100644
> --- a/sysdeps/i386/i686/memset.S
> +++ b/sysdeps/i386/i686/memset.S
> @@ -27,7 +27,7 @@
>  #define LEN	CHR+4
>  
>          .text
> -#if defined SHARED && IS_IN (libc)
> +#if defined PIC && IS_IN (libc)
>  ENTRY_CHK (__memset_chk)
>  	movl	12(%esp), %eax
>  	cmpl	%eax, 16(%esp)
> diff --git a/sysdeps/i386/i686/multiarch/Makefile b/sysdeps/i386/i686/multiarch/Makefile
> index f48b06741f..f86e69de55 100644
> --- a/sysdeps/i386/i686/multiarch/Makefile
> +++ b/sysdeps/i386/i686/multiarch/Makefile
> @@ -44,7 +44,3 @@ libm-sysdep_routines += s_fma-fma s_fmaf-fma
>  CFLAGS-s_fma-fma.c += -mavx -mfpmath=sse
>  CFLAGS-s_fmaf-fma.c += -mavx -mfpmath=sse
>  endif
> -
> -ifeq ($(subdir),debug)
> -sysdep_routines += memset_chk-nonshared
> -endif
> diff --git a/sysdeps/i386/i686/multiarch/memset-ia32.S b/sysdeps/i386/i686/multiarch/memset-ia32.S
> index ac57e456d4..a1b3919a19 100644
> --- a/sysdeps/i386/i686/multiarch/memset-ia32.S
> +++ b/sysdeps/i386/i686/multiarch/memset-ia32.S
> @@ -18,16 +18,17 @@
>  
>  #if IS_IN (libc)
>  # define memset __memset_ia32
> -# define __memset_chk __memset_chk_ia32
>  
>  # ifdef SHARED
> -#  undef libc_hidden_builtin_def
> +#  define __memset_chk __memset_chk_ia32

OK.

> +# endif
> +
> +# undef libc_hidden_builtin_def
>  /* IFUNC doesn't work with the hidden functions in shared library since
>     they will be called without setting up EBX needed for PLT which is
>     used by IFUNC.  */
> -#  define libc_hidden_builtin_def(name) \
> +# define libc_hidden_builtin_def(name) \
>  	.globl __GI_memset; __GI_memset = memset
> -# endif
>  #endif
>  
>  #include <sysdeps/i386/i686/memset.S>
> diff --git a/sysdeps/i386/i686/multiarch/memset_chk-nonshared.S b/sysdeps/i386/i686/multiarch/memset_chk-nonshared.S
> deleted file mode 100644
> index e68c6e43b3..0000000000
> --- a/sysdeps/i386/i686/multiarch/memset_chk-nonshared.S
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -/* Non-shared version of memset_chk for i686.
> -   Copyright (C) 2017-2023 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#if IS_IN (libc) && !defined SHARED
> -# include <sysdeps/i386/memset_chk.S>
> -#endif

OK.

-- 
Cheers,
Carlos.

Re: [PATCH 3/4] linux: Fix i686 with gcc6

[Carlos O'Donell]

On 7/25/23 11:16, Adhemerval Zanella via Libc-alpha wrote:
> On __convert_scm_timestamps GCC 6 issues an warning that tvts[0]/tvts[1]
> maybe be used uninitialized, however it would be used if type is set to a
> value different than 0 (done by either COMPAT_SO_TIMESTAMP_OLD or
> COMPAT_SO_TIMESTAMPNS_OLD) which will fallthrough to 'common' label.

LGTM.

Andreas, As the RM is this OK to push?

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


> It does not show with gcc 7 or more recent versions.
> 
> Checked on i686-linux-gnu.
> ---
>  sysdeps/unix/sysv/linux/convert_scm_timestamps.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> index 42f9613416..06c8adeee1 100644
> --- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> +++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> @@ -23,6 +23,7 @@
>  # include <string.h>
>  # include <sys/socket.h>
>  # include <socket-constants-time64.h>
> +# include <libc-diag.h>

OK.

>  
>  /* It converts the first SO_TIMESTAMP or SO_TIMESTAMPNS with 32-bit time and
>     appends it to the control buffer.  The 32-bit time field is kept as-is.
> @@ -44,7 +45,15 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
>       'struct __kernel_sock_timeval' while for SO_TIMESTAMPNS_NEW is a
>       'struct __kernel_timespec'.  In either case it is two uint64_t
>       members.  */
> +
> +  /* GCC 6 issues an warning that tvts[0]/tvts[1] maybe be used uninitialized,
> +     however it would be used if type is set to a value different than 0
> +     (done by either COMPAT_SO_TIMESTAMP_OLD or COMPAT_SO_TIMESTAMPNS_OLD)
> +     which will fallthrough to 'common' label.  */
> +  DIAG_PUSH_NEEDS_COMMENT;
> +  DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");

OK.

>    int64_t tvts[2];
> +  DIAG_POP_NEEDS_COMMENT;
>    int32_t tmp[2];
>  
>    struct cmsghdr *cmsg, *last = NULL;

-- 
Cheers,
Carlos.

Re: [PATCH 4/4] string: Fix tester build with fortify enable with gcc 6

[Carlos O'Donell]

On 7/25/23 11:16, Adhemerval Zanella via Libc-alpha wrote:
> When building with fortify enabled, GCC 6 issues an warning the fortify
> wrapper might overflow the destination buffer.  However, GCC does not
> provide a specific flag to disable the warning (the failure is tied to
> -Werror).  So to avoid disable all errors, only enable the check for
> GCC 7 or newer.

It's not normal to disable a part of a test depending on the gcc version.

However, keeping the test is better for coverage rather than moving to UNSUPPORTED.

Andreas, As the RM is this OK to push?

LGTM.

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

> 
> Checked on i686-linux-gnu.
> ---
>  string/tester.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/string/tester.c b/string/tester.c
> index da42c72141..f7d4bac5a8 100644
> --- a/string/tester.c
> +++ b/string/tester.c
> @@ -385,8 +385,17 @@ test_strncat (void)
>  
>    (void) strcpy (one, "gh");
>    (void) strcpy (two, "ef");
> +  /* When building with fortify enabled, GCC 6 issues an warning the fortify
> +     wrapper might overflow the destination buffer.  However, GCC does not
> +     provide a specific flag to disable the warning (the failure is tied to
> +     -Werror).  So to avoid disable all errors, only enable the check for
> +     GCC 7 or newer.  */
> +#if __GNUC_PREREQ (7, 0)

OK. Disabled test for < GCC 7. This works.

>    (void) strncat (one, two, 99);
>    equal (one, "ghef", 5);			/* Basic test encore. */
> +#else
> +  equal (one, "gh", 2);
> +#endif
>    equal (two, "ef", 6);			/* Stomped on source? */
>  
>    (void) strcpy (one, "");

-- 
Cheers,
Carlos.

Re: [PATCH 1/4] i386: Fix build with --enable-fortify=3

[Andreas K. Huettel]

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

Am Mittwoch, 26. Juli 2023, 03:51:31 CEST schrieb Carlos O'Donell:
> On 7/25/23 11:16, Adhemerval Zanella via Libc-alpha wrote:
> > The i386 string routines provide multiple internal definitions
> > for memcpy, memmove, and mempcpy chk routines:
> > 
> >   $ objdump -t libc.a | grep __memcpy_chk
> >   00000000 g     F .text  0000000e __memcpy_chk
> >   00000000 g     F .text  00000013 __memcpy_chk
> >   $ objdump -t libc.a | grep __mempcpy_chk
> >   00000000 g     F .text  0000000e __mempcpy_chk
> >   00000000 g     F .text  00000013 __mempcpy_chk
> >   $ objdump -t libc.a | grep __memmove_chk
> >   00000000 g     F .text  0000000e __memmove_chk
> >   00000000 g     F .text  00000013 __memmove_chk
> > 
> > Although is not an issue for normal static builds, with fortify=3
> > glibc itself might use the fortify chk functions and thus static
> > build might fail with multiple definitions.  For instance:
> > 
> > x86_64-glibc-linux-gnu-gcc -m32 -march=i686 -o [...]math/test-signgam-uchar-static -nostdlib -nostartfiles -static -static-pie [...]
> > x86_64-glibc-linux-gnu/bin/ld: [...]/libc.a(mempcpy-ia32.o):
> > in function `__mempcpy_chk': [...]/glibc-git/string/../sysdeps/i386/i686/mempcpy.S:32: multiple definition of `__mempcpy_chk';
> > [...]/libc.a(mempcpy_chk-nonshared.o):[...]/debug/../sysdeps/i386/mempcpy_chk.S:28: first defined here
> > collect2: error: ld returned 1 exit status
> > make[2]: *** [../Rules:298:
> > 
> > There is no need for mem*-nonshared.S, the __mem*_chk routines
> > are already provided by the assembly routines.
> > 
> > Checked on i686-linux-gnu with gcc 13 built with fortify=1,2,3 and
> > without fortify.
> 
> I agree this is probably the best fix for glibc 2.38.
> 
> I think we should commit this for 2.38 and I'll re-test with i686.
> 
> Andreas, As the RM is this OK to push?

Yes please go ahead.

> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> 
> > ---
> >  sysdeps/i386/i686/multiarch/Makefile          |  3 +--
> >  .../i686/multiarch/memcpy_chk-nonshared.S     | 21 -------------------
> >  .../i686/multiarch/memmove_chk-nonshared.S    | 21 -------------------
> >  .../i686/multiarch/mempcpy_chk-nonshared.S    | 21 -------------------
> >  4 files changed, 1 insertion(+), 65 deletions(-)
> >  delete mode 100644 sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S
> >  delete mode 100644 sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S
> >  delete mode 100644 sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S
> > 
> > diff --git a/sysdeps/i386/i686/multiarch/Makefile b/sysdeps/i386/i686/multiarch/Makefile
> > index 9fe5ea8639..f48b06741f 100644
> > --- a/sysdeps/i386/i686/multiarch/Makefile
> > +++ b/sysdeps/i386/i686/multiarch/Makefile
> > @@ -46,6 +46,5 @@ CFLAGS-s_fmaf-fma.c += -mavx -mfpmath=sse
> >  endif
> >  
> >  ifeq ($(subdir),debug)
> > -sysdep_routines += memcpy_chk-nonshared mempcpy_chk-nonshared \
> > -		   memmove_chk-nonshared memset_chk-nonshared
> > +sysdep_routines += memset_chk-nonshared
> >  endif
> > diff --git a/sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S b/sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S
> > deleted file mode 100644
> > index 6801893b72..0000000000
> > --- a/sysdeps/i386/i686/multiarch/memcpy_chk-nonshared.S
> > +++ /dev/null
> > @@ -1,21 +0,0 @@
> > -/* Non-shared version of memcpy_chk for i686.
> > -   Copyright (C) 2017-2023 Free Software Foundation, Inc.
> > -   This file is part of the GNU C Library.
> > -
> > -   The GNU C Library is free software; you can redistribute it and/or
> > -   modify it under the terms of the GNU Lesser General Public
> > -   License as published by the Free Software Foundation; either
> > -   version 2.1 of the License, or (at your option) any later version.
> > -
> > -   The GNU C Library is distributed in the hope that it will be useful,
> > -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > -   Lesser General Public License for more details.
> > -
> > -   You should have received a copy of the GNU Lesser General Public
> > -   License along with the GNU C Library; if not, see
> > -   <https://www.gnu.org/licenses/>.  */
> > -
> > -#if IS_IN (libc) && !defined SHARED
> > -# include <sysdeps/i386/memcpy_chk.S>
> > -#endif
> > diff --git a/sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S b/sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S
> > deleted file mode 100644
> > index c5de28d4a8..0000000000
> > --- a/sysdeps/i386/i686/multiarch/memmove_chk-nonshared.S
> > +++ /dev/null
> > @@ -1,21 +0,0 @@
> > -/* Non-shared version of memmmove_chk for i686.
> > -   Copyright (C) 2017-2023 Free Software Foundation, Inc.
> > -   This file is part of the GNU C Library.
> > -
> > -   The GNU C Library is free software; you can redistribute it and/or
> > -   modify it under the terms of the GNU Lesser General Public
> > -   License as published by the Free Software Foundation; either
> > -   version 2.1 of the License, or (at your option) any later version.
> > -
> > -   The GNU C Library is distributed in the hope that it will be useful,
> > -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > -   Lesser General Public License for more details.
> > -
> > -   You should have received a copy of the GNU Lesser General Public
> > -   License along with the GNU C Library; if not, see
> > -   <https://www.gnu.org/licenses/>.  */
> > -
> > -#if IS_IN (libc) && !defined SHARED
> > -# include <sysdeps/i386/memmove_chk.S>
> > -#endif
> > diff --git a/sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S b/sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S
> > deleted file mode 100644
> > index c4f89e156b..0000000000
> > --- a/sysdeps/i386/i686/multiarch/mempcpy_chk-nonshared.S
> > +++ /dev/null
> > @@ -1,21 +0,0 @@
> > -/* Non-shared version of mempcpy_chk for i686.
> > -   Copyright (C) 2017-2023 Free Software Foundation, Inc.
> > -   This file is part of the GNU C Library.
> > -
> > -   The GNU C Library is free software; you can redistribute it and/or
> > -   modify it under the terms of the GNU Lesser General Public
> > -   License as published by the Free Software Foundation; either
> > -   version 2.1 of the License, or (at your option) any later version.
> > -
> > -   The GNU C Library is distributed in the hope that it will be useful,
> > -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > -   Lesser General Public License for more details.
> > -
> > -   You should have received a copy of the GNU Lesser General Public
> > -   License along with the GNU C Library; if not, see
> > -   <https://www.gnu.org/licenses/>.  */
> > -
> > -#if IS_IN (libc) && !defined SHARED
> > -# include <sysdeps/i386/mempcpy_chk.S>
> > -#endif
> 
> 


-- 
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 2/4] i386: Remove memset_chk-nonshared.S

[Andreas K. Huettel]

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

Am Mittwoch, 26. Juli 2023, 03:51:37 CEST schrieb Carlos O'Donell:
> On 7/25/23 11:16, Adhemerval Zanella via Libc-alpha wrote:
> > Similar to memcpy, mempcpy, and memmove there is no need for an
> > specific memset_chk-nonshared.S.  It can be provided by
> > memset-ia32.S itself for static library.
> 
> Agreed. LGTM. I'll retest i686 after this goes in.
> 
> Andreas, As the RM is this OK to push?
> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Yes please go ahead.

> 
> > Checked on i686-linux-gnu.
> > ---
> >  sysdeps/i386/i686/memset.S                    |  2 +-
> >  sysdeps/i386/i686/multiarch/Makefile          |  4 ----
> >  sysdeps/i386/i686/multiarch/memset-ia32.S     |  9 ++++----
> >  .../i686/multiarch/memset_chk-nonshared.S     | 21 -------------------
> >  4 files changed, 6 insertions(+), 30 deletions(-)
> >  delete mode 100644 sysdeps/i386/i686/multiarch/memset_chk-nonshared.S
> > 
> > diff --git a/sysdeps/i386/i686/memset.S b/sysdeps/i386/i686/memset.S
> > index 0b5e671d83..b84dc3fbe9 100644
> > --- a/sysdeps/i386/i686/memset.S
> > +++ b/sysdeps/i386/i686/memset.S
> > @@ -27,7 +27,7 @@
> >  #define LEN	CHR+4
> >  
> >          .text
> > -#if defined SHARED && IS_IN (libc)
> > +#if defined PIC && IS_IN (libc)
> >  ENTRY_CHK (__memset_chk)
> >  	movl	12(%esp), %eax
> >  	cmpl	%eax, 16(%esp)
> > diff --git a/sysdeps/i386/i686/multiarch/Makefile b/sysdeps/i386/i686/multiarch/Makefile
> > index f48b06741f..f86e69de55 100644
> > --- a/sysdeps/i386/i686/multiarch/Makefile
> > +++ b/sysdeps/i386/i686/multiarch/Makefile
> > @@ -44,7 +44,3 @@ libm-sysdep_routines += s_fma-fma s_fmaf-fma
> >  CFLAGS-s_fma-fma.c += -mavx -mfpmath=sse
> >  CFLAGS-s_fmaf-fma.c += -mavx -mfpmath=sse
> >  endif
> > -
> > -ifeq ($(subdir),debug)
> > -sysdep_routines += memset_chk-nonshared
> > -endif
> > diff --git a/sysdeps/i386/i686/multiarch/memset-ia32.S b/sysdeps/i386/i686/multiarch/memset-ia32.S
> > index ac57e456d4..a1b3919a19 100644
> > --- a/sysdeps/i386/i686/multiarch/memset-ia32.S
> > +++ b/sysdeps/i386/i686/multiarch/memset-ia32.S
> > @@ -18,16 +18,17 @@
> >  
> >  #if IS_IN (libc)
> >  # define memset __memset_ia32
> > -# define __memset_chk __memset_chk_ia32
> >  
> >  # ifdef SHARED
> > -#  undef libc_hidden_builtin_def
> > +#  define __memset_chk __memset_chk_ia32
> 
> OK.
> 
> > +# endif
> > +
> > +# undef libc_hidden_builtin_def
> >  /* IFUNC doesn't work with the hidden functions in shared library since
> >     they will be called without setting up EBX needed for PLT which is
> >     used by IFUNC.  */
> > -#  define libc_hidden_builtin_def(name) \
> > +# define libc_hidden_builtin_def(name) \
> >  	.globl __GI_memset; __GI_memset = memset
> > -# endif
> >  #endif
> >  
> >  #include <sysdeps/i386/i686/memset.S>
> > diff --git a/sysdeps/i386/i686/multiarch/memset_chk-nonshared.S b/sysdeps/i386/i686/multiarch/memset_chk-nonshared.S
> > deleted file mode 100644
> > index e68c6e43b3..0000000000
> > --- a/sysdeps/i386/i686/multiarch/memset_chk-nonshared.S
> > +++ /dev/null
> > @@ -1,21 +0,0 @@
> > -/* Non-shared version of memset_chk for i686.
> > -   Copyright (C) 2017-2023 Free Software Foundation, Inc.
> > -   This file is part of the GNU C Library.
> > -
> > -   The GNU C Library is free software; you can redistribute it and/or
> > -   modify it under the terms of the GNU Lesser General Public
> > -   License as published by the Free Software Foundation; either
> > -   version 2.1 of the License, or (at your option) any later version.
> > -
> > -   The GNU C Library is distributed in the hope that it will be useful,
> > -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > -   Lesser General Public License for more details.
> > -
> > -   You should have received a copy of the GNU Lesser General Public
> > -   License along with the GNU C Library; if not, see
> > -   <https://www.gnu.org/licenses/>.  */
> > -
> > -#if IS_IN (libc) && !defined SHARED
> > -# include <sysdeps/i386/memset_chk.S>
> > -#endif
> 
> OK.
> 
> 


-- 
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 3/4] linux: Fix i686 with gcc6

[Andreas K. Huettel]

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

Am Mittwoch, 26. Juli 2023, 03:51:41 CEST schrieb Carlos O'Donell:
> On 7/25/23 11:16, Adhemerval Zanella via Libc-alpha wrote:
> > On __convert_scm_timestamps GCC 6 issues an warning that tvts[0]/tvts[1]
> > maybe be used uninitialized, however it would be used if type is set to a
> > value different than 0 (done by either COMPAT_SO_TIMESTAMP_OLD or
> > COMPAT_SO_TIMESTAMPNS_OLD) which will fallthrough to 'common' label.
> 
> LGTM.
> 
> Andreas, As the RM is this OK to push?
> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Yes please go ahead.

> 
> 
> > It does not show with gcc 7 or more recent versions.
> > 
> > Checked on i686-linux-gnu.
> > ---
> >  sysdeps/unix/sysv/linux/convert_scm_timestamps.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> > index 42f9613416..06c8adeee1 100644
> > --- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> > +++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> > @@ -23,6 +23,7 @@
> >  # include <string.h>
> >  # include <sys/socket.h>
> >  # include <socket-constants-time64.h>
> > +# include <libc-diag.h>
> 
> OK.
> 
> >  
> >  /* It converts the first SO_TIMESTAMP or SO_TIMESTAMPNS with 32-bit time and
> >     appends it to the control buffer.  The 32-bit time field is kept as-is.
> > @@ -44,7 +45,15 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
> >       'struct __kernel_sock_timeval' while for SO_TIMESTAMPNS_NEW is a
> >       'struct __kernel_timespec'.  In either case it is two uint64_t
> >       members.  */
> > +
> > +  /* GCC 6 issues an warning that tvts[0]/tvts[1] maybe be used uninitialized,
> > +     however it would be used if type is set to a value different than 0
> > +     (done by either COMPAT_SO_TIMESTAMP_OLD or COMPAT_SO_TIMESTAMPNS_OLD)
> > +     which will fallthrough to 'common' label.  */
> > +  DIAG_PUSH_NEEDS_COMMENT;
> > +  DIAG_IGNORE_NEEDS_COMMENT (6, "-Wmaybe-uninitialized");
> 
> OK.
> 
> >    int64_t tvts[2];
> > +  DIAG_POP_NEEDS_COMMENT;
> >    int32_t tmp[2];
> >  
> >    struct cmsghdr *cmsg, *last = NULL;
> 
> 


-- 
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 4/4] string: Fix tester build with fortify enable with gcc6

[Andreas K. Huettel]

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

Am Mittwoch, 26. Juli 2023, 03:51:45 CEST schrieb Carlos O'Donell:
> On 7/25/23 11:16, Adhemerval Zanella via Libc-alpha wrote:
> > When building with fortify enabled, GCC 6 issues an warning the fortify
> > wrapper might overflow the destination buffer.  However, GCC does not
> > provide a specific flag to disable the warning (the failure is tied to
> > -Werror).  So to avoid disable all errors, only enable the check for
> > GCC 7 or newer.
> 
> It's not normal to disable a part of a test depending on the gcc version.
> 
> However, keeping the test is better for coverage rather than moving to UNSUPPORTED.
> 
> Andreas, As the RM is this OK to push?
> 
> LGTM.
> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Yes please go ahead.

> 
> > 
> > Checked on i686-linux-gnu.
> > ---
> >  string/tester.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/string/tester.c b/string/tester.c
> > index da42c72141..f7d4bac5a8 100644
> > --- a/string/tester.c
> > +++ b/string/tester.c
> > @@ -385,8 +385,17 @@ test_strncat (void)
> >  
> >    (void) strcpy (one, "gh");
> >    (void) strcpy (two, "ef");
> > +  /* When building with fortify enabled, GCC 6 issues an warning the fortify
> > +     wrapper might overflow the destination buffer.  However, GCC does not
> > +     provide a specific flag to disable the warning (the failure is tied to
> > +     -Werror).  So to avoid disable all errors, only enable the check for
> > +     GCC 7 or newer.  */
> > +#if __GNUC_PREREQ (7, 0)
> 
> OK. Disabled test for < GCC 7. This works.
> 
> >    (void) strncat (one, two, 99);
> >    equal (one, "ghef", 5);			/* Basic test encore. */
> > +#else
> > +  equal (one, "gh", 2);
> > +#endif
> >    equal (two, "ef", 6);			/* Stomped on source? */
> >  
> >    (void) strcpy (one, "");
> 
> 


-- 
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 --]