fortify: further improve Clang style fortify

  glibc >= v2.41 significatly improved clang fortify support,
so I've rebased the ChromeOS toolchain test suite [1] on
top of the upstream implementation and semantics.

Doing this I noticed some missing upstream diagnostics, so
I've added them to get Clang builds on par with GCC.

In addition to the missing ones I also noticed some mismatch
bugs between __glibc_objsize0 vs __glibc_objsize calls done
via various macros like:
__fortify_clang_warning_only_if_bos0_lt2 versus
__fortify_clang_warning_only_if_bos_lt2

The general rule is that the function argument declaration
must match its uses to ensure check consistency between the
__glibc_objsize0 and __glibc_objsize calls.

For example if the arg is defined like
__fortify_clang_overload_arg (...)
then a form like the following must be used:
__fortify_clang_warning_only_if_bos_lt2(...)

If the arg is defined like
__fortify_clang_overload_arg0 (...)
then a form like the following must be used:
__fortify_clang_warning_only_if_bos0_lt2(...)

This rule ensures consistent use of __glibc_objsize0 vs
__glibc_objsize calls in checks.

While a diagnostic message might be correctly printed
with the above type of mismatch, LLVM/Clang will not be
able to signal and terminate a process despite for eg a
buffer overflow being detected (it will happily continue).

So the end result of this type of bug fix is that processes
will get terimated by proper signals when fortify diagnostics
get triggered.

Based on a patch by George Burges with contributions from
Andrej Shadura. [2]

Link: https://chromium.googlesource.com/chromiumos/platform2/+/refs/heads/main/cros-toolchain/fortify-tests/clang-fortify-tests.cpp [1]
Link: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/staging-infra-release-R135-16209.B/sys-libs/glibc/files/local/glibc-2.39/0005-glibc-add-clang-style-FORTIFY.patch [2]
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 libio/bits/stdio2.h             | 38 ++++++++++++++++++++++-----------
 misc/bits/syslog.h              |  6 ++++--
 posix/bits/unistd.h             |  5 ++++-
 stdlib/bits/stdlib.h            |  5 ++++-
 string/bits/string_fortified.h  | 26 +++++++++++++---------
 string/bits/strings_fortified.h | 11 ++++++----
 wcsmbs/bits/wchar2.h            | 12 +++++------
 7 files changed, 67 insertions(+), 36 deletions(-)
  

On 27/03/2025 15:35, Adrian Ratiu wrote:
> glibc >= v2.41 significatly improved clang fortify support,
> so I've rebased the ChromeOS toolchain test suite [1] on
> top of the upstream implementation and semantics.
> 
> Doing this I noticed some missing upstream diagnostics, so
> I've added them to get Clang builds on par with GCC.
> 
> In addition to the missing ones I also noticed some mismatch
> bugs between __glibc_objsize0 vs __glibc_objsize calls done
> via various macros like:
> __fortify_clang_warning_only_if_bos0_lt2 versus
> __fortify_clang_warning_only_if_bos_lt2
> 
> The general rule is that the function argument declaration
> must match its uses to ensure check consistency between the
> __glibc_objsize0 and __glibc_objsize calls.
> 
> For example if the arg is defined like
> __fortify_clang_overload_arg (...)
> then a form like the following must be used:
> __fortify_clang_warning_only_if_bos_lt2(...)
> 
> If the arg is defined like
> __fortify_clang_overload_arg0 (...)
> then a form like the following must be used:
> __fortify_clang_warning_only_if_bos0_lt2(...)
> 
> This rule ensures consistent use of __glibc_objsize0 vs
> __glibc_objsize calls in checks.
> 
> While a diagnostic message might be correctly printed
> with the above type of mismatch, LLVM/Clang will not be
> able to signal and terminate a process despite for eg a
> buffer overflow being detected (it will happily continue).

despite e.g. having detected a buffer overflow, and the process will 
happily continue.

> 
> So the end result of this type of bug fix is that processes
> will get terimated by proper signals when fortify diagnostics

terminated

> get triggered.
> 
> Based on a patch by George Burges with contributions from
> Andrej Shadura. [2]


> Link: https://chromium.googlesource.com/chromiumos/platform2/+/refs/heads/main/cros-toolchain/fortify-tests/clang-fortify-tests.cpp [1]
> Link: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/staging-infra-release-R135-16209.B/sys-libs/glibc/files/local/glibc-2.39/0005-glibc-add-clang-style-FORTIFY.patch [2]
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>

Thanks!

Acked-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
  

On 27/03/25 11:35, Adrian Ratiu wrote:
> glibc >= v2.41 significatly improved clang fortify support,
> so I've rebased the ChromeOS toolchain test suite [1] on
> top of the upstream implementation and semantics.
> 
> Doing this I noticed some missing upstream diagnostics, so
> I've added them to get Clang builds on par with GCC.
> 
> In addition to the missing ones I also noticed some mismatch
> bugs between __glibc_objsize0 vs __glibc_objsize calls done
> via various macros like:
> __fortify_clang_warning_only_if_bos0_lt2 versus
> __fortify_clang_warning_only_if_bos_lt2
> 
> The general rule is that the function argument declaration
> must match its uses to ensure check consistency between the
> __glibc_objsize0 and __glibc_objsize calls.
> 
> For example if the arg is defined like
> __fortify_clang_overload_arg (...)
> then a form like the following must be used:
> __fortify_clang_warning_only_if_bos_lt2(...)
> 
> If the arg is defined like
> __fortify_clang_overload_arg0 (...)
> then a form like the following must be used:
> __fortify_clang_warning_only_if_bos0_lt2(...)
> 
> This rule ensures consistent use of __glibc_objsize0 vs
> __glibc_objsize calls in checks.
> 
> While a diagnostic message might be correctly printed
> with the above type of mismatch, LLVM/Clang will not be
> able to signal and terminate a process despite for eg a
> buffer overflow being detected (it will happily continue).

These should be considered real bugs and have proper regression testcases.
Do you have any examples of which fortify wrappers are not being triggered
correctly?

Unfortunately we don't have tests for the compiler warning itself,
just for the runtime behavior.  And I recently updated my clang branch [1]
and I don't see any regression with debug fortify tests with clang 18/19/20/main.

I also don't see any issue withg clang 18/19/20/main when used just as test 
compiler (TEST_CC/TEST_CXX).

[1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/clang

> 
> So the end result of this type of bug fix is that processes
> will get terimated by proper signals when fortify diagnostics
> get triggered.
> 
> Based on a patch by George Burges with contributions from
> Andrej Shadura. [2]
> 
> Link: https://chromium.googlesource.com/chromiumos/platform2/+/refs/heads/main/cros-toolchain/fortify-tests/clang-fortify-tests.cpp [1]
> Link: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/staging-infra-release-R135-16209.B/sys-libs/glibc/files/local/glibc-2.39/0005-glibc-add-clang-style-FORTIFY.patch [2]
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
>  libio/bits/stdio2.h             | 38 ++++++++++++++++++++++-----------
>  misc/bits/syslog.h              |  6 ++++--
>  posix/bits/unistd.h             |  5 ++++-
>  stdlib/bits/stdlib.h            |  5 ++++-
>  string/bits/string_fortified.h  | 26 +++++++++++++---------
>  string/bits/strings_fortified.h | 11 ++++++----
>  wcsmbs/bits/wchar2.h            | 12 +++++------
>  7 files changed, 67 insertions(+), 36 deletions(-)
> 
> diff --git a/libio/bits/stdio2.h b/libio/bits/stdio2.h
> index d2ed65371f..0dca01ec83 100644
> --- a/libio/bits/stdio2.h
> +++ b/libio/bits/stdio2.h
> @@ -33,7 +33,8 @@ __NTH (sprintf (char *__restrict __s, const char *__restrict __fmt, ...))
>  }
>  #elif __fortify_use_clang
>  /* clang does not have __va_arg_pack, so defer to va_arg version.  */
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 3))) int

This seems not really related to the described issues in commit message and
not really clang-specific since gcc also supports __format__ attribute.
So I think we should be a different change.

>  __NTH (sprintf (__fortify_clang_overload_arg (char *, __restrict, __s),
>  		const char *__restrict __fmt, ...))
>  {
> @@ -71,9 +72,13 @@ __NTH (snprintf (char *__restrict __s, size_t __n,
>  }
>  # elif __fortify_use_clang
>  /* clang does not have __va_arg_pack, so defer to va_arg version.  */
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 3, 4))) int
>  __NTH (snprintf (__fortify_clang_overload_arg (char *, __restrict, __s),
>  		 size_t __n, const char *__restrict __fmt, ...))
> +     __fortify_clang_warning_only_if_bos_lt(__n, __s,
> +					    "call to snprintf may overflow "
> +					    "the destination buffer")
>  {
>    __gnuc_va_list __fortify_ap;
>    __builtin_va_start (__fortify_ap, __fmt);
> @@ -89,13 +94,14 @@ __NTH (snprintf (__fortify_clang_overload_arg (char *, __restrict, __s),
>  			    __glibc_objsize (str), __VA_ARGS__)
>  # endif
>  
> -__fortify_function __attribute_overloadable__ int
> +__fortify_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 3, 0))) int
>  __NTH (vsnprintf (__fortify_clang_overload_arg (char *, __restrict, __s),
>  		  size_t __n, const char *__restrict __fmt,
>  		  __gnuc_va_list __ap))
> -     __fortify_clang_warning (__fortify_clang_bos_static_lt (__n, __s),
> -			      "call to vsnprintf may overflow the destination "
> -			      "buffer")
> +     __fortify_clang_warning_only_if_bos_lt(__n, __s,
> +					    "call to vsnprintf may overflow "
> +					    "the destination buffer")
>  {
>    return __builtin___vsnprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
>  				    __glibc_objsize (__s), __fmt, __ap);
> @@ -119,7 +125,9 @@ printf (const char *__restrict __fmt, ...)
>  }
>  # elif __fortify_use_clang
>  /* clang does not have __va_arg_pack, so defer to va_arg version.  */
> -__fortify_function_error_function __attribute_overloadable__ __nonnull ((1)) int
> +__fortify_function_error_function __attribute_overloadable__ __nonnull ((1))
> +__attribute__ ((__format__ (__printf__, 2, 3)))
> +int
>  fprintf (__fortify_clang_overload_arg (FILE *, __restrict, __stream),
>  	 const char *__restrict __fmt, ...)
>  {
> @@ -131,7 +139,8 @@ fprintf (__fortify_clang_overload_arg (FILE *, __restrict, __stream),
>    return __r;
>  }
>  
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 1, 2))) int
>  printf (__fortify_clang_overload_arg (const char *, __restrict, __fmt), ...)
>  {
>    __gnuc_va_list __fortify_ap;
> @@ -148,7 +157,8 @@ printf (__fortify_clang_overload_arg (const char *, __restrict, __fmt), ...)
>    __fprintf_chk (stream, __USE_FORTIFY_LEVEL - 1, __VA_ARGS__)
>  # endif
>  
> -__fortify_function __attribute_overloadable__ int
> +__fortify_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 1, 0))) int
>  vprintf (__fortify_clang_overload_arg (const char *, __restrict, __fmt),
>  	 __gnuc_va_list __ap)
>  {
> @@ -175,7 +185,8 @@ dprintf (int __fd, const char *__restrict __fmt, ...)
>  			__va_arg_pack ());
>  }
>  #  elif __fortify_use_clang
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 3))) int
>  dprintf (int __fd, __fortify_clang_overload_arg (const char *, __restrict,
>  						 __fmt), ...)
>  {
> @@ -223,7 +234,9 @@ __NTH (obstack_printf (struct obstack *__restrict __obstack,
>  			       __va_arg_pack ());
>  }
>  #  elif __fortify_use_clang
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 3)))
> +__wur int
>  __NTH (asprintf (__fortify_clang_overload_arg (char **, __restrict, __ptr),
>  		 const char *__restrict __fmt, ...))
>  {
> @@ -247,7 +260,8 @@ __NTH (__asprintf (__fortify_clang_overload_arg (char **, __restrict, __ptr),
>    return __r;
>  }
>  
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 3))) int
>  __NTH (obstack_printf (__fortify_clang_overload_arg (struct obstack *,
>  						     __restrict, __obstack),
>  		       const char *__restrict __fmt, ...))
> diff --git a/misc/bits/syslog.h b/misc/bits/syslog.h
> index 9907a28cbe..ca3132e09c 100644
> --- a/misc/bits/syslog.h
> +++ b/misc/bits/syslog.h
> @@ -37,7 +37,8 @@ syslog (int __pri, const char *__fmt, ...)
>    __syslog_chk (__pri, __USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ());
>  }
>  #elif __fortify_use_clang && defined __USE_MISC
> -__fortify_function_error_function __attribute_overloadable__ void
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 3))) void
>  syslog (int __pri, __fortify_clang_overload_arg (const char *, , __fmt), ...)
>  {
>    __gnuc_va_list __fortify_ap;
> @@ -52,7 +53,8 @@ syslog (int __pri, __fortify_clang_overload_arg (const char *, , __fmt), ...)
>  
>  
>  #ifdef __USE_MISC
> -__fortify_function __attribute_overloadable__ void
> +__fortify_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 0))) void
>  vsyslog (int __pri, __fortify_clang_overload_arg (const char *, ,__fmt),
>  	 __gnuc_va_list __ap)
>  {
> diff --git a/posix/bits/unistd.h b/posix/bits/unistd.h
> index f3428135ec..0af7b5e53c 100644
> --- a/posix/bits/unistd.h
> +++ b/posix/bits/unistd.h
> @@ -79,7 +79,7 @@ pread64 (int __fd, __fortify_clang_overload_arg0 (void *, ,__buf),
>  #if defined __USE_XOPEN_EXTENDED || defined __USE_XOPEN2K
>  __fortify_function __attribute_overloadable__ __nonnull ((1, 2)) __wur ssize_t
>  __NTH (readlink (const char *__restrict __path,
> -		 __fortify_clang_overload_arg0 (char *, __restrict, __buf),
> +		 __fortify_clang_overload_arg (char *, __restrict, __buf),
>  		 size_t __len))
>       __fortify_clang_warning_only_if_bos_lt (__len, __buf,
>  					     "readlink called with bigger length "
> @@ -122,6 +122,9 @@ __NTH (getcwd (__fortify_clang_overload_arg (char *, , __buf), size_t __size))
>  __fortify_function __attribute_overloadable__ __nonnull ((1))
>  __attribute_deprecated__ __wur char *
>  __NTH (getwd (__fortify_clang_overload_arg (char *,, __buf)))
> +     __fortify_clang_warning (__glibc_objsize (__buf) == (size_t) -1,
> +			      "please use getcwd instead, as getwd "
> +			      "doesn't specify buffer size")
>  {
>    if (__glibc_objsize (__buf) != (size_t) -1)
>      return __getwd_chk (__buf, __glibc_objsize (__buf));

Ok.

> diff --git a/stdlib/bits/stdlib.h b/stdlib/bits/stdlib.h
> index 12f9cfd58f..58c51faaf8 100644
> --- a/stdlib/bits/stdlib.h
> +++ b/stdlib/bits/stdlib.h
> @@ -129,7 +129,7 @@ __fortify_function __attribute_overloadable__ size_t
>  __NTH (mbstowcs (__fortify_clang_overload_arg (wchar_t *, __restrict, __dst),
>  		 const char *__restrict __src,
>  		 size_t __len))
> -     __fortify_clang_warning_only_if_bos0_lt2 (__len, __dst, sizeof (wchar_t),
> +     __fortify_clang_warning_only_if_bos_lt2 (__len, __dst, sizeof (wchar_t),
>  					       "mbstowcs called with dst buffer "
>  					       "smaller than len * sizeof (wchar_t)")
>  {

Ok.

> @@ -159,6 +159,9 @@ __fortify_function __attribute_overloadable__ size_t
>  __NTH (wcstombs (__fortify_clang_overload_arg (char *, __restrict, __dst),
>  		 const wchar_t *__restrict __src,
>  		 size_t __len))
> +     __fortify_clang_warning_only_if_bos_lt (__len, __dst,
> +					      "wcstombs called with dst buffer "
> +					      "smaller than len")
>  {
>    return __glibc_fortify (wcstombs, __len, sizeof (char),
>  			  __glibc_objsize (__dst),

Ok.

> diff --git a/string/bits/string_fortified.h b/string/bits/string_fortified.h
> index ae14cfbab0..2512bc0517 100644
> --- a/string/bits/string_fortified.h
> +++ b/string/bits/string_fortified.h
> @@ -22,25 +22,29 @@
>  # error "Never use <bits/string_fortified.h> directly; include <string.h> instead."
>  #endif
>  
> -__fortify_function void *
> -__NTH (memcpy (void *__restrict __dest, const void *__restrict __src,
> -	       size_t __len))
> +__fortify_function __attribute_overloadable__ void *
> +__NTH (memcpy (__fortify_clang_overload_arg0 (void *, __restrict, __dest),
> +	       const void *__restrict __src, size_t __len))
> +     __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
>  {
>    return __builtin___memcpy_chk (__dest, __src, __len,
>  				 __glibc_objsize0 (__dest));
>  }
>  

Ok.

> -__fortify_function void *
> -__NTH (memmove (void *__dest, const void *__src, size_t __len))
> +__fortify_function __attribute_overloadable__ void *
> +__NTH (memmove (__fortify_clang_overload_arg0 (void *, __restrict, __dest),
> +		const void *__src, size_t __len))
> +     __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
>  {
>    return __builtin___memmove_chk (__dest, __src, __len,
>  				  __glibc_objsize0 (__dest));
>  }
>  

Ok.

>  #ifdef __USE_GNU
> -__fortify_function void *
> -__NTH (mempcpy (void *__restrict __dest, const void *__restrict __src,
> -		size_t __len))
> +__fortify_function __attribute_overloadable__ void *
> +__NTH (mempcpy (__fortify_clang_overload_arg0 (void *, __restrict, __dest),
> +                const void *__restrict __src, size_t __len))
> +     __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
>  {
>    return __builtin___mempcpy_chk (__dest, __src, __len,
>  				  __glibc_objsize0 (__dest));

Ok.

> @@ -53,8 +57,10 @@ __NTH (mempcpy (void *__restrict __dest, const void *__restrict __src,
>     especially problematic if the intended fill value is zero.  In this
>     case no work is done at all.  We detect these problems by referring
>     non-existing functions.  */
> -__fortify_function void *
> -__NTH (memset (void *__dest, int __ch, size_t __len))
> +__fortify_function __attribute_overloadable__ void *
> +__NTH (memset (__fortify_clang_overload_arg0 (void *, __restrict, __dest),
> +	       int __ch, size_t __len))
> +     __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
>  {
>    return __builtin___memset_chk (__dest, __ch, __len,
>  				 __glibc_objsize0 (__dest));

Ok.

> diff --git a/string/bits/strings_fortified.h b/string/bits/strings_fortified.h
> index be65d5937d..327288f289 100644
> --- a/string/bits/strings_fortified.h
> +++ b/string/bits/strings_fortified.h
> @@ -19,15 +19,18 @@
>  #ifndef __STRINGS_FORTIFIED
>  # define __STRINGS_FORTIFIED 1
>  
> -__fortify_function void
> -__NTH (bcopy (const void *__src, void *__dest, size_t __len))
> +__fortify_function __attribute_overloadable__ void
> +__NTH (bcopy (const void *__src, __fortify_clang_overload_arg0 (void *, , __dest),
> +	      size_t __len))
> +     __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
>  {
>    (void) __builtin___memmove_chk (__dest, __src, __len,
>  				  __glibc_objsize0 (__dest));
>  }
>  

Ok.

> -__fortify_function void
> -__NTH (bzero (void *__dest, size_t __len))
> +__fortify_function __attribute_overloadable__ void
> +__NTH (bzero (__fortify_clang_overload_arg0 (void *, , __dest), size_t __len))
> +     __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
>  {
>    (void) __builtin___memset_chk (__dest, '\0', __len,
>  				 __glibc_objsize0 (__dest));

Ok.

> diff --git a/wcsmbs/bits/wchar2.h b/wcsmbs/bits/wchar2.h
> index c1a1da8166..609cc76986 100644
> --- a/wcsmbs/bits/wchar2.h
> +++ b/wcsmbs/bits/wchar2.h
> @@ -21,7 +21,7 @@
>  #endif
>  
>  __fortify_function __attribute_overloadable__ wchar_t *
> -__NTH (wmemcpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __s1),
> +__NTH (wmemcpy (__fortify_clang_overload_arg0 (wchar_t *, __restrict, __s1),
>  		const wchar_t *__restrict __s2, size_t __n))
>       __fortify_clang_warning_only_if_bos0_lt2 (__n, __s1, sizeof (wchar_t),
>  					       "wmemcpy called with length bigger "
> @@ -33,7 +33,7 @@ __NTH (wmemcpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __s1),
>  }

Ok.

>  
>  __fortify_function __attribute_overloadable__ wchar_t *
> -__NTH (wmemmove (__fortify_clang_overload_arg (wchar_t *, ,__s1),
> +__NTH (wmemmove (__fortify_clang_overload_arg0 (wchar_t *, ,__s1),
>  		 const wchar_t *__s2, size_t __n))
>       __fortify_clang_warning_only_if_bos0_lt2 (__n, __s1, sizeof (wchar_t),
>  					       "wmemmove called with length bigger "
> @@ -46,7 +46,7 @@ __NTH (wmemmove (__fortify_clang_overload_arg (wchar_t *, ,__s1),
>  
>  #ifdef __USE_GNU
>  __fortify_function __attribute_overloadable__ wchar_t *
> -__NTH (wmempcpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __s1),
> +__NTH (wmempcpy (__fortify_clang_overload_arg0 (wchar_t *, __restrict, __s1),
>  		 const wchar_t *__restrict __s2, size_t __n))
>       __fortify_clang_warning_only_if_bos0_lt2 (__n, __s1, sizeof (wchar_t),
>  					       "wmempcpy called with length bigger "
> @@ -59,7 +59,7 @@ __NTH (wmempcpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __s1),
>  #endif
>  
>  __fortify_function __attribute_overloadable__ wchar_t *
> -__NTH (wmemset (__fortify_clang_overload_arg (wchar_t *, ,__s), wchar_t __c,
> +__NTH (wmemset (__fortify_clang_overload_arg0 (wchar_t *, ,__s), wchar_t __c,
>  		size_t __n))
>       __fortify_clang_warning_only_if_bos0_lt2 (__n, __s, sizeof (wchar_t),
>  					       "wmemset called with length bigger "
> @@ -93,7 +93,7 @@ __NTH (wcpcpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __dest),
>  __fortify_function __attribute_overloadable__ wchar_t *
>  __NTH (wcsncpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __dest),
>  		const wchar_t *__restrict __src, size_t __n))
> -     __fortify_clang_warning_only_if_bos0_lt2 (__n, __dest, sizeof (wchar_t),
> +     __fortify_clang_warning_only_if_bos_lt2 (__n, __dest, sizeof (wchar_t),
>  					       "wcsncpy called with length bigger "
>  					       "than size of destination buffer")
>  {
> @@ -105,7 +105,7 @@ __NTH (wcsncpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __dest),
>  __fortify_function __attribute_overloadable__ wchar_t *
>  __NTH (wcpncpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __dest),
>  		const wchar_t *__restrict __src, size_t __n))
> -     __fortify_clang_warning_only_if_bos0_lt2 (__n, __dest, sizeof (wchar_t),
> +     __fortify_clang_warning_only_if_bos_lt2 (__n, __dest, sizeof (wchar_t),
>  					       "wcpncpy called with length bigger "
>  					       "than size of destination buffer")
>  {

Ok.
  

Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:

> [...]

> Unfortunately we don't have tests for the compiler warning itself,
> just for the runtime behavior.  And I recently updated my clang branch [1]
> and I don't see any regression with debug fortify tests with clang 18/19/20/main.

(FWIW, I don't have much capacity at the moment, but in a few months, if
you want to either send some of that out in batches, or otherwise
coordinate on getting some of that in, let me know, and I'll provide review.)

> [...]

thanks,
sam
  

Hi Adhemerval and thank you for the prompt response!

On Thu, 27 Mar 2025, Adhemerval Zanella Netto 
<adhemerval.zanella@linaro.org> wrote:
> 
> These should be considered real bugs and have proper regression 
> testcases.  Do you have any examples of which fortify wrappers 
> are not being triggered correctly? 

Yes, all the ones affected by the objsize vs objsize0 mismatch 
which I'm touching here.

In version 2, which will be patch series for better clarity, I 
will provide an explicit list. From the top of my head the 
memset/memcpy/mempcpy/memmove & co, including their wcsmbs 
counterparts.

> 
> Unfortunately we don't have tests for the compiler warning 
> itself, just for the runtime behavior.  And I recently updated 
> my clang branch [1] and I don't see any regression with debug 
> fortify tests with clang 18/19/20/main. 
> 
> I also don't see any issue withg clang 18/19/20/main when used 
> just as test  compiler (TEST_CC/TEST_CXX). 
> 
> [1] 
> https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/clang 
> 

I'm testing this by building glibc with GCC (currently 10.2, in 
the process of upgrading to 14.2.1 from Gentoo), then building and 
running the tests linked in the patch description [1] with clang 
20 against those glibc headers on amd64.

I can create minimal reproducing C testcases, to avoid for 
e.g. the CPP GTest suite we use in CrOS.

Also these are not quite regressions due to compiler versions or 
glibc itself, they just never worked and are the last missing 
pieces. :) 

For some fun context, with glibc 2.41 the ChromiumOS downstream 
clang fortify patch went from:
 17 files changed, 1114 insertions(+), 621 deletions(-) 
to 
 7 files changed, 67 insertions(+), 36 deletions(-)

The remaining 67(+), 36(-) is exactly this patch, so sweet! :)
 
> 
> This seems not really related to the described issues in commit 
> message and not really clang-specific since gcc also supports 
> __format__ attribute.  So I think we should be a different 
> change. 
> 

Yes, I'll split this into separate smaller patches, each with 
their own separate reasoning, because indeed there are multiple 
separate issues here.

Thanks again for everything!
  

On 27/03/25 14:43, Adrian Ratiu wrote:
> Hi Adhemerval and thank you for the prompt response!
> 
> On Thu, 27 Mar 2025, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote:
>>
>> These should be considered real bugs and have proper regression testcases.  Do you have any examples of which fortify wrappers are not being triggered correctly? 
> 
> Yes, all the ones affected by the objsize vs objsize0 mismatch which I'm touching here.
> 
> In version 2, which will be patch series for better clarity, I will provide an explicit list. From the top of my head the memset/memcpy/mempcpy/memmove & co, including their wcsmbs counterparts.

Thanks.

> 
>>
>> Unfortunately we don't have tests for the compiler warning itself, just for the runtime behavior.  And I recently updated my clang branch [1] and I don't see any regression with debug fortify tests with clang 18/19/20/main.
>> I also don't see any issue withg clang 18/19/20/main when used just as test  compiler (TEST_CC/TEST_CXX).
>> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/clang
> 
> I'm testing this by building glibc with GCC (currently 10.2, in the process of upgrading to 14.2.1 from Gentoo), then building and running the tests linked in the patch description [1] with clang 20 against those glibc headers on amd64.
> 
> I can create minimal reproducing C testcases, to avoid for e.g. the CPP GTest suite we use in CrOS.

If you could add then either on debug/tst-fortify.c or in an extra extra it would
be helpful.  The tst-fortify.c is the simplest because we test with a lot of
permutation (_FORTIFY_SOURCE equal 1,2,3, C/C++, etc.)

> 
> Also these are not quite regressions due to compiler versions or glibc itself, they just never worked and are the last missing pieces. :)

We do have tests that check for invalid mem* and wmem* on tst-fortify.c, so
they catch missing *_chk calls from compiler/fortify header and also missing
abort exceptions due invalid calls. We don't test invalid calls where the
fortify wrappers will thrown a compile warning.

> For some fun context, with glibc 2.41 the ChromiumOS downstream clang fortify patch went from:
> 17 files changed, 1114 insertions(+), 621 deletions(-) to 7 files changed, 67 insertions(+), 36 deletions(-)
> 
> The remaining 67(+), 36(-) is exactly this patch, so sweet! :)

Nice.

> 
>>
>> This seems not really related to the described issues in commit message and not really clang-specific since gcc also supports __format__ attribute.  So I think we should be a different change.
> 
> Yes, I'll split this into separate smaller patches, each with their own separate reasoning, because indeed there are multiple separate issues here.
> 
> Thanks again for everything!
  

On 27/03/25 13:39, Sam James wrote:
> Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:
> 
>> [...]
> 
>> Unfortunately we don't have tests for the compiler warning itself,
>> just for the runtime behavior.  And I recently updated my clang branch [1]
>> and I don't see any regression with debug fortify tests with clang 18/19/20/main.
> 
> (FWIW, I don't have much capacity at the moment, but in a few months, if
> you want to either send some of that out in batches, or otherwise
> coordinate on getting some of that in, let me know, and I'll provide review.)
> 
>> [...]

Hi Sam,

I finally update my clang branch [1] and cleanup some patches after the H.J
work to support TEST_CC.  It contains now 66 patches, 49 to enable build and
17 to fixes some test regressions.

I used clang versions 18, 19, 20, and master and focused on the main 64 bits
architectures.  In a short:

  * x86_64, aarch64, and riscv builds with expected default options [2] in
    all versions. x86_64 also support --enable-cet.

  * We need either -mno-gnu-attribute for powerpc64le, or adding some Makefile
    hackery to build some objects and strip the .gnu.attributes section.

  * We need -mlong-double-128 for s390x for some compat long double support.

  * --enable-memory-tagging is broken on aarch for clang 18, 19, and 20.  I think
    because __libc_mtag_address_get_tag lacks 'memory' constraint, the optimizer
    passes are not correctly generating the correct code and emitting the 'ldg'
    regardless.  I am checking if adding the constraint fixes it.

  * Both glibc and testsuite were checked using system glibc support libraries
    (libgcc*).  Using compiler-rt ones are possible, but last time I tried it
    triggered a lot of regressions (specially on math tests).

  * I used ld.ldd from each version as the linker, along llvm related tools
    (llvm-ar, llvm-readelf, etc).

The testsuite results are quite good:

# FAIL      x86_64   aarch64    RISCV
clang-18        21        20       66
clang-19        16        20
clang-20        17        20
clang-main      17        16

  * On all architectures I see tgmath conform issues due clang wrapper header.
    I need to check if there is an opened bug report, and/or check how to fix
    it.

  * clang-18 on x86_64 show ifuncmain failures which I almost sure it is code
    generation ones. I am not sure if it is worth to dig into, since clang-19
    does not show the regressions.

  * All the math tests failure are for float128 and/or long double.  I need to
    check exactly what is happening.

  * aarch64 gmon is broken due a ABI mismatch on how clang and gcc generated the
    _mcount funtion call.  I think I have a patch somewhere, I need to double
    check if I fixed on glibc or clang.

  * There are multiple tests that uses __builtin_*_overflow that disable by
    335ba9b6c1db7030264 because it tests for __GNUC_PREREQ instead of a proper
    configure tests if the compiler supports the builtin.  I need to add a
    patch to fix it.

  * I tests RISCV on qemu using NFS and cross-test-ssh.sh.  For some reason
    I am not sure the containers tests fails because it can not obtain the
    lock, so I expect the real result for RISCV on a more streamlined
    environment would be better.

So if you could help I can start to send the patched.  Most of them are just to
handle -Wall -Werror with clang, where some patches disable some compiler flags,
while other enable/disable the warnings.  There are some to fix the code for
clang where some gccsism are assumed.


[1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/clang
[2] --enable-stack-protector=all, --enable-bind-now=yes, --enable-profile=yes, and
    --enable-fortify-source=2