[v2] assert: Refactor assert/assert_perror

  It now calls __libc_assert_message, which contains similar logic. The
assert now does not require any dynamic memory allocation, so
test-assert2.c is adapted to handle it.

It also removes the fxprintf from assert/assert_perror; although it
is not 100% backwards-compatible (write message only if there is a
file descriptor associated with the stderr) it nows write bytes
directly to without going through the wide stream state.

Checked on aarch64-linux-gnu.
--
Changes from v1:
* Setup the VMA properly.
---
 assert/__libc_assert_fail.c |   4 +-
 assert/assert-perr.c        |  22 ++++++-
 assert/assert.c             | 112 +++++-------------------------------
 assert/test-assert-2.c      |  18 ++----
 include/stdio.h             |  51 ++++++++++++----
 sysdeps/posix/libc_fatal.c  |   4 +-
 6 files changed, 82 insertions(+), 129 deletions(-)
  

Ping.

On 31/03/25 14:30, Adhemerval Zanella wrote:
> It now calls __libc_assert_message, which contains similar logic. The
> assert now does not require any dynamic memory allocation, so
> test-assert2.c is adapted to handle it.
> 
> It also removes the fxprintf from assert/assert_perror; although it
> is not 100% backwards-compatible (write message only if there is a
> file descriptor associated with the stderr) it nows write bytes
> directly to without going through the wide stream state.
> 
> Checked on aarch64-linux-gnu.
> --
> Changes from v1:
> * Setup the VMA properly.
> ---
>  assert/__libc_assert_fail.c |   4 +-
>  assert/assert-perr.c        |  22 ++++++-
>  assert/assert.c             | 112 +++++-------------------------------
>  assert/test-assert-2.c      |  18 ++----
>  include/stdio.h             |  51 ++++++++++++----
>  sysdeps/posix/libc_fatal.c  |   4 +-
>  6 files changed, 82 insertions(+), 129 deletions(-)
> 
> diff --git a/assert/__libc_assert_fail.c b/assert/__libc_assert_fail.c
> index b50637a893..2d4654bd41 100644
> --- a/assert/__libc_assert_fail.c
> +++ b/assert/__libc_assert_fail.c
> @@ -28,6 +28,6 @@ __libc_assert_fail (const char *assertion, const char *file, unsigned int line,
>    char linebuf[INT_BUFSIZE_BOUND (unsigned int)];
>    array_end (linebuf)[-1] = '\0';
>    char *linestr = _itoa_word (line, array_end (linebuf) - 1, 10, 0);
> -  __libc_message ("Fatal glibc error: %s:%s (%s): assertion failed: %s\n",
> -                  file, linestr, function, assertion);
> +  __libc_assert_message ("Fatal glibc error: %s:%s (%s): assertion failed: %s\n",
> +			 file, linestr, function, assertion);
>  }
> diff --git a/assert/assert-perr.c b/assert/assert-perr.c
> index 83f0b3a76f..1de96c8790 100644
> --- a/assert/assert-perr.c
> +++ b/assert/assert-perr.c
> @@ -15,10 +15,15 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +#include <_itoa.h>
> +#include <array_length.h>
>  #include <assert.h>
> +#include <intprops.h>
>  #include <libintl.h>
>  #include <string.h>
> +#include <stdio.h>
>  
> +extern const char *__progname;
>  
>  /* This function, when passed an error number, a filename, and a line
>     number, prints a message on the standard error stream of the form:
> @@ -31,8 +36,19 @@ __assert_perror_fail (int errnum,
>  {
>    char errbuf[1024];
>  
> -  char *e = __strerror_r (errnum, errbuf, sizeof errbuf);
> -  __assert_fail_base (_("%s%s%s:%u: %s%sUnexpected error: %s.\n"),
> -		      e, file, line, function);
> +  const char *e = __strerror_r (errnum, errbuf, sizeof errbuf);
> +
> +  char linebuf[INT_BUFSIZE_BOUND (unsigned int)];
> +  array_end (linebuf)[-1] = '\0';
> +  char *linestr = _itoa_word (line, array_end (linebuf) - 1, 10, 0);
> +
> +  __libc_assert_message (_("%s%s%s:%s: %s%sUnexpected error: %s.\n"),
> +			 __progname,
> +			 __progname[0] ? ": " : "",
> +			 file,
> +			 linestr,
> +			 function ? function : "",
> +			 function ? ": " : "",
> +			 e);
>  }
>  libc_hidden_def (__assert_perror_fail)
> diff --git a/assert/assert.c b/assert/assert.c
> index d21d76fa62..095266e45d 100644
> --- a/assert/assert.c
> +++ b/assert/assert.c
> @@ -15,115 +15,31 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +#include <array_length.h>
>  #include <intprops.h>
>  #include <ldsodefs.h>
>  #include <libc-pointer-arith.h>
>  #include <libintl.h>
> -#include <libio/iolibio.h>
> -#include <setvmaname.h>
> -#include <sys/uio.h>
> -#include <unistd.h>
> +#include <stdio.h>
>  
>  
>  extern const char *__progname;
>  
> -#define fflush(s) _IO_fflush (s)
> -
> -/* This function, when passed a string containing an asserted
> -   expression, a filename, and a line number, prints a message
> -   on the standard error stream of the form:
> -	a.c:10: foobar: Assertion `a == b' failed.
> -   It then aborts program execution via a call to `abort'.  */
> -
> -#ifdef FATAL_PREPARE_INCLUDE
> -# include FATAL_PREPARE_INCLUDE
> -#endif
> -
> -
> -void
> -__assert_fail_base (const char *fmt, const char *assertion, const char *file,
> -		    unsigned int line, const char *function)
> -{
> -  char *str;
> -
> -#ifdef FATAL_PREPARE
> -  FATAL_PREPARE;
> -#endif
> -
> -  int total = __asprintf (&str, fmt,
> -			  __progname, __progname[0] ? ": " : "",
> -			  file, line,
> -			  function ? function : "", function ? ": " : "",
> -			  assertion);
> -  if (total >= 0)
> -    {
> -      /* Print the message.  */
> -      (void) __fxprintf (NULL, "%s", str);
> -      (void) fflush (stderr);
> -
> -      total = ALIGN_UP (total + sizeof (struct abort_msg_s) + 1,
> -			GLRO(dl_pagesize));
> -      struct abort_msg_s *buf = __mmap (NULL, total, PROT_READ | PROT_WRITE,
> -					MAP_ANON | MAP_PRIVATE, -1, 0);
> -      if (__glibc_likely (buf != MAP_FAILED))
> -	{
> -	  buf->size = total;
> -	  strcpy (buf->msg, str);
> -	  __set_vma_name (buf, total, " glibc: assert");
> -
> -	  /* We have to free the old buffer since the application might
> -	     catch the SIGABRT signal.  */
> -	  struct abort_msg_s *old = atomic_exchange_acquire (&__abort_msg, buf);
> -
> -	  if (old != NULL)
> -	    __munmap (old, old->size);
> -	}
> -
> -      free (str);
> -    }
> -  else
> -    {
> -      /* At least print a minimal message.  */
> -      char linebuf[INT_STRLEN_BOUND (int) + sizeof ":: "];
> -      struct iovec v[9];
> -      int i = 0;
> -
> -#define WS(s) (v[i].iov_len = strlen (v[i].iov_base = (void *) (s)), i++)
> -
> -      if (__progname)
> -	{
> -	  WS (__progname);
> -	  WS (": ");
> -	}
> -
> -      WS (file);
> -      v[i++] = (struct iovec) {.iov_base = linebuf,
> -	.iov_len = sprintf (linebuf, ":%d: ", line)};
> -
> -      if (function)
> -	{
> -	  WS (function);
> -	  WS (": ");
> -	}
> -
> -      WS ("Assertion `");
> -      WS (assertion);
> -      /* We omit the '.' here so that the assert tests can tell when
> -         this code path is taken.  */
> -      WS ("' failed\n");
> -
> -      (void) __writev (STDERR_FILENO, v, i);
> -    }
> -
> -  abort ();
> -}
> -
> -
>  #undef __assert_fail
>  void
>  __assert_fail (const char *assertion, const char *file, unsigned int line,
>  	       const char *function)
>  {
> -  __assert_fail_base (_("%s%s%s:%u: %s%sAssertion `%s' failed.\n"),
> -		      assertion, file, line, function);
> +  char linebuf[INT_BUFSIZE_BOUND (unsigned int)];
> +  array_end (linebuf)[-1] = '\0';
> +  char *linestr = _itoa_word (line, array_end (linebuf) - 1, 10, 0);
> +
> +  __libc_assert_message (_("%s%s%s:%s: %s%sAssertion `%s' failed.\n"),
> +			 __progname,
> +			 __progname[0] ? ": " : "",
> +			 file,
> +			 linestr,
> +			 function ? function : "",
> +			 function ? ": " : "",
> +			 assertion);
>  }
> diff --git a/assert/test-assert-2.c b/assert/test-assert-2.c
> index 6d54ef9ba6..9997c98d1a 100644
> --- a/assert/test-assert-2.c
> +++ b/assert/test-assert-2.c
> @@ -127,7 +127,7 @@ check_posix (const char *buf, int rv, int line,
>  }
>  
>  static void
> -one_test (void (*func)(void *), int which_path)
> +one_test (void (*func)(void *))
>  {
>    struct support_capture_subprocess sp;
>    int rv;
> @@ -140,17 +140,7 @@ one_test (void (*func)(void *), int which_path)
>  
>    check_posix (sp.err.buffer, rv, do_lineno, do_funcname, "1 == 2");
>  
> -  /* Look for intentional subtle differences in messages to verify
> -     that the intended code path was taken.  */
> -  switch (which_path)
> -    {
> -    case 0:
> -      TEST_VERIFY (strstr (sp.err.buffer, "failed.\n") != NULL);
> -      break;
> -    case 1:
> -      TEST_VERIFY (strstr (sp.err.buffer, "failed\n") != NULL);
> -      break;
> -    }
> +  TEST_VERIFY (strstr (sp.err.buffer, "failed.\n") != NULL);
>  
>    support_capture_subprocess_free (&sp);
>  }
> @@ -158,8 +148,8 @@ one_test (void (*func)(void *), int which_path)
>  static int
>  do_test(void)
>  {
> -  one_test (test_assert_normal, 0);
> -  one_test (test_assert_nomalloc, 1);
> +  one_test (test_assert_normal);
> +  one_test (test_assert_nomalloc);
>  
>    return 0;
>  }
> diff --git a/include/stdio.h b/include/stdio.h
> index e48d709919..683b212d14 100644
> --- a/include/stdio.h
> +++ b/include/stdio.h
> @@ -171,33 +171,64 @@ extern void __fortify_fail (const char *msg) __attribute__ ((__noreturn__));
>  libc_hidden_proto (__fortify_fail)
>  
>  /* The maximum number of varargs allowed in a __libc_message format string */
> -#define LIBC_MESSAGE_MAX_ARGS 4
> +#define LIBC_MESSAGE_MAX_ARGS 7
>  
> -_Noreturn void __libc_message_impl (const char *__fnt, ...) attribute_hidden
> -     __attribute__ ((__format__ (__printf__, 1, 2)));
> +_Noreturn void __libc_message_impl (const char *__vmaname, const char *__fmt,
> +				    ...) attribute_hidden
> +     __attribute__ ((__format__ (__printf__, 2, 3)));
> +
> +#define __libc_fatal_vma_name  "glibc: fatal"
> +#define __libc_assert_vma_name "glibc: assert"
>  
>  #define __libc_message0(fmt) \
> -   __libc_message_impl (fmt)
> +   __libc_message_impl (__libc_fatal_vma_name, fmt)
>  #define __libc_message1(fmt, a1) \
> -   __libc_message_impl (fmt, a1)
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1)
>  #define __libc_message2(fmt, a1, a2) \
> -   __libc_message_impl (fmt, a1, a2)
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2)
>  #define __libc_message3(fmt, a1, a2, a3) \
> -   __libc_message_impl (fmt, a1, a2, a3)
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3)
>  #define __libc_message4(fmt, a1, a2, a3, a4) \
> -   __libc_message_impl (fmt, a1, a2, a3, a4)
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4)
> +#define __libc_message5(fmt, a1, a2, a3, a4, a5) \
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4, a5)
> +#define __libc_message6(fmt, a1, a2, a3, a4, a5, a6) \
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4, a5, a6)
> +#define __libc_message7(fmt, a1, a2, a3, a4, a5, a6, a7) \
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4, a5, a6, a7)
> +
> +#define __libc_assert_message0(fmt) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt)
> +#define __libc_assert_message1(fmt, a1) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1)
> +#define __libc_assert_message2(fmt, a1, a2) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2)
> +#define __libc_assert_message3(fmt, a1, a2, a3) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3)
> +#define __libc_assert_message4(fmt, a1, a2, a3, a4) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4)
> +#define __libc_assert_message5(fmt, a1, a2, a3, a4, a5) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4, a5)
> +#define __libc_assert_message6(fmt, a1, a2, a3, a4, a5, a6) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4, a5, a6)
> +#define __libc_assert_message7(fmt, a1, a2, a3, a4, a5, a6, a7) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4, a5, a6, a7)
>  
>  #define __libc_message_concat_x(a,b)  a##b
>  #define __libc_message_concat(a,b)    __libc_message_concat_x (a, b)
>  
> -#define __libc_message_nargs_x(a0,a1,a2,a3,a4,a5,a6,...) a6
> +#define __libc_message_nargs_x(a0,a1,a2,a3,a4,a5,a6,a7,...) a7
>  #define __libc_message_nargs(b, ...) \
> -   __libc_message_nargs_x (__VA_ARGS__,6,5,4,3,2,1,0,)
> +   __libc_message_nargs_x (__VA_ARGS__,7,6,5,4,3,2,1,0,)
>  #define __libc_message_disp(b, ...) \
>     __libc_message_concat (b, __libc_message_nargs (__VA_ARGS__))(__VA_ARGS__)
> +
>  #define __libc_message(...) \
>     __libc_message_disp (__libc_message, __VA_ARGS__)
>  
> +#define __libc_assert_message(...) \
> +   __libc_message_disp (__libc_assert_message, __VA_ARGS__)
> +
>  /* Acquire ownership of STREAM.  */
>  extern void __flockfile (FILE *__stream) attribute_hidden;
>  
> diff --git a/sysdeps/posix/libc_fatal.c b/sysdeps/posix/libc_fatal.c
> index d90cc6c681..818f911675 100644
> --- a/sysdeps/posix/libc_fatal.c
> +++ b/sysdeps/posix/libc_fatal.c
> @@ -49,7 +49,7 @@ writev_for_fatal (int fd, const struct iovec *iov, size_t niov, size_t total)
>  
>  /* Abort with an error message.  */
>  void
> -__libc_message_impl (const char *fmt, ...)
> +__libc_message_impl (const char *vma_name, const char *fmt, ...)
>  {
>    va_list ap;
>    int fd = -1;
> @@ -119,7 +119,7 @@ __libc_message_impl (const char *fmt, ...)
>  	    wp = mempcpy (wp, iov[cnt].iov_base, iov[cnt].iov_len);
>  	  *wp = '\0';
>  
> -	  __set_vma_name (buf, total, " glibc: fatal");
> +	  __set_vma_name (buf, total, vma_name);
>  
>  	  /* We have to free the old buffer since the application might
>  	     catch the SIGABRT signal.  */
  

* Adhemerval Zanella:

> It now calls __libc_assert_message, which contains similar logic. The
> assert now does not require any dynamic memory allocation, so
> test-assert2.c is adapted to handle it.
>
> It also removes the fxprintf from assert/assert_perror; although it
> is not 100% backwards-compatible (write message only if there is a
> file descriptor associated with the stderr) it nows write bytes
> directly to without going through the wide stream state.
>
> Checked on aarch64-linux-gnu.
> --
> Changes from v1:
> * Setup the VMA properly.
> ---
>  assert/__libc_assert_fail.c |   4 +-
>  assert/assert-perr.c        |  22 ++++++-
>  assert/assert.c             | 112 +++++-------------------------------
>  assert/test-assert-2.c      |  18 ++----
>  include/stdio.h             |  51 ++++++++++++----
>  sysdeps/posix/libc_fatal.c  |   4 +-
>  6 files changed, 82 insertions(+), 129 deletions(-)
>
> diff --git a/assert/__libc_assert_fail.c b/assert/__libc_assert_fail.c
> index b50637a893..2d4654bd41 100644
> --- a/assert/__libc_assert_fail.c
> +++ b/assert/__libc_assert_fail.c
> @@ -28,6 +28,6 @@ __libc_assert_fail (const char *assertion, const char *file, unsigned int line,
>    char linebuf[INT_BUFSIZE_BOUND (unsigned int)];
>    array_end (linebuf)[-1] = '\0';
>    char *linestr = _itoa_word (line, array_end (linebuf) - 1, 10, 0);
> -  __libc_message ("Fatal glibc error: %s:%s (%s): assertion failed: %s\n",
> -                  file, linestr, function, assertion);
> +  __libc_assert_message ("Fatal glibc error: %s:%s (%s): assertion failed: %s\n",
> +			 file, linestr, function, assertion);
>  }

I think we are aiming for 79 characters at maximum per line.

> diff --git a/include/stdio.h b/include/stdio.h
> index e48d709919..683b212d14 100644
> --- a/include/stdio.h
> +++ b/include/stdio.h
> @@ -171,33 +171,64 @@ extern void __fortify_fail (const char *msg) __attribute__ ((__noreturn__));
>  libc_hidden_proto (__fortify_fail)
>  
>  /* The maximum number of varargs allowed in a __libc_message format string */
> -#define LIBC_MESSAGE_MAX_ARGS 4
> +#define LIBC_MESSAGE_MAX_ARGS 7
>  
> -_Noreturn void __libc_message_impl (const char *__fnt, ...) attribute_hidden
> -     __attribute__ ((__format__ (__printf__, 1, 2)));
> +_Noreturn void __libc_message_impl (const char *__vmaname, const char *__fmt,
> +				    ...) attribute_hidden
> +     __attribute__ ((__format__ (__printf__, 2, 3)));
> +
> +#define __libc_fatal_vma_name  "glibc: fatal"
> +#define __libc_assert_vma_name "glibc: assert"
>  
>  #define __libc_message0(fmt) \
> -   __libc_message_impl (fmt)
> +   __libc_message_impl (__libc_fatal_vma_name, fmt)
>  #define __libc_message1(fmt, a1) \
> -   __libc_message_impl (fmt, a1)
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1)
>  #define __libc_message2(fmt, a1, a2) \
> -   __libc_message_impl (fmt, a1, a2)
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2)
>  #define __libc_message3(fmt, a1, a2, a3) \
> -   __libc_message_impl (fmt, a1, a2, a3)
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3)
>  #define __libc_message4(fmt, a1, a2, a3, a4) \
> -   __libc_message_impl (fmt, a1, a2, a3, a4)
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4)
> +#define __libc_message5(fmt, a1, a2, a3, a4, a5) \
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4, a5)
> +#define __libc_message6(fmt, a1, a2, a3, a4, a5, a6) \
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4, a5, a6)
> +#define __libc_message7(fmt, a1, a2, a3, a4, a5, a6, a7) \
> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4, a5, a6, a7)
> +
> +#define __libc_assert_message0(fmt) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt)
> +#define __libc_assert_message1(fmt, a1) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1)
> +#define __libc_assert_message2(fmt, a1, a2) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2)
> +#define __libc_assert_message3(fmt, a1, a2, a3) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3)
> +#define __libc_assert_message4(fmt, a1, a2, a3, a4) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4)
> +#define __libc_assert_message5(fmt, a1, a2, a3, a4, a5) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4, a5)
> +#define __libc_assert_message6(fmt, a1, a2, a3, a4, a5, a6) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4, a5, a6)
> +#define __libc_assert_message7(fmt, a1, a2, a3, a4, a5, a6, a7) \
> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4, a5, a6, a7)
>  
>  #define __libc_message_concat_x(a,b)  a##b
>  #define __libc_message_concat(a,b)    __libc_message_concat_x (a, b)
>  
> -#define __libc_message_nargs_x(a0,a1,a2,a3,a4,a5,a6,...) a6
> +#define __libc_message_nargs_x(a0,a1,a2,a3,a4,a5,a6,a7,...) a7
>  #define __libc_message_nargs(b, ...) \
> -   __libc_message_nargs_x (__VA_ARGS__,6,5,4,3,2,1,0,)
> +   __libc_message_nargs_x (__VA_ARGS__,7,6,5,4,3,2,1,0,)
>  #define __libc_message_disp(b, ...) \
>     __libc_message_concat (b, __libc_message_nargs (__VA_ARGS__))(__VA_ARGS__)

Why do we need __libc_message_nargs when things go into a varargs
function without an explicit argument count in the end anyway?

The patch no longer applies to the main branch.

Thanks,
Florian
  

On 18/08/25 11:01, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> It now calls __libc_assert_message, which contains similar logic. The
>> assert now does not require any dynamic memory allocation, so
>> test-assert2.c is adapted to handle it.
>>
>> It also removes the fxprintf from assert/assert_perror; although it
>> is not 100% backwards-compatible (write message only if there is a
>> file descriptor associated with the stderr) it nows write bytes
>> directly to without going through the wide stream state.
>>
>> Checked on aarch64-linux-gnu.
>> --
>> Changes from v1:
>> * Setup the VMA properly.
>> ---
>>  assert/__libc_assert_fail.c |   4 +-
>>  assert/assert-perr.c        |  22 ++++++-
>>  assert/assert.c             | 112 +++++-------------------------------
>>  assert/test-assert-2.c      |  18 ++----
>>  include/stdio.h             |  51 ++++++++++++----
>>  sysdeps/posix/libc_fatal.c  |   4 +-
>>  6 files changed, 82 insertions(+), 129 deletions(-)
>>
>> diff --git a/assert/__libc_assert_fail.c b/assert/__libc_assert_fail.c
>> index b50637a893..2d4654bd41 100644
>> --- a/assert/__libc_assert_fail.c
>> +++ b/assert/__libc_assert_fail.c
>> @@ -28,6 +28,6 @@ __libc_assert_fail (const char *assertion, const char *file, unsigned int line,
>>    char linebuf[INT_BUFSIZE_BOUND (unsigned int)];
>>    array_end (linebuf)[-1] = '\0';
>>    char *linestr = _itoa_word (line, array_end (linebuf) - 1, 10, 0);
>> -  __libc_message ("Fatal glibc error: %s:%s (%s): assertion failed: %s\n",
>> -                  file, linestr, function, assertion);
>> +  __libc_assert_message ("Fatal glibc error: %s:%s (%s): assertion failed: %s\n",
>> +			 file, linestr, function, assertion);
>>  }
> 
> I think we are aiming for 79 characters at maximum per line.

Ack.

> 
>> diff --git a/include/stdio.h b/include/stdio.h
>> index e48d709919..683b212d14 100644
>> --- a/include/stdio.h
>> +++ b/include/stdio.h
>> @@ -171,33 +171,64 @@ extern void __fortify_fail (const char *msg) __attribute__ ((__noreturn__));
>>  libc_hidden_proto (__fortify_fail)
>>  
>>  /* The maximum number of varargs allowed in a __libc_message format string */
>> -#define LIBC_MESSAGE_MAX_ARGS 4
>> +#define LIBC_MESSAGE_MAX_ARGS 7
>>  
>> -_Noreturn void __libc_message_impl (const char *__fnt, ...) attribute_hidden
>> -     __attribute__ ((__format__ (__printf__, 1, 2)));
>> +_Noreturn void __libc_message_impl (const char *__vmaname, const char *__fmt,
>> +				    ...) attribute_hidden
>> +     __attribute__ ((__format__ (__printf__, 2, 3)));
>> +
>> +#define __libc_fatal_vma_name  "glibc: fatal"
>> +#define __libc_assert_vma_name "glibc: assert"
>>  
>>  #define __libc_message0(fmt) \
>> -   __libc_message_impl (fmt)
>> +   __libc_message_impl (__libc_fatal_vma_name, fmt)
>>  #define __libc_message1(fmt, a1) \
>> -   __libc_message_impl (fmt, a1)
>> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1)
>>  #define __libc_message2(fmt, a1, a2) \
>> -   __libc_message_impl (fmt, a1, a2)
>> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2)
>>  #define __libc_message3(fmt, a1, a2, a3) \
>> -   __libc_message_impl (fmt, a1, a2, a3)
>> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3)
>>  #define __libc_message4(fmt, a1, a2, a3, a4) \
>> -   __libc_message_impl (fmt, a1, a2, a3, a4)
>> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4)
>> +#define __libc_message5(fmt, a1, a2, a3, a4, a5) \
>> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4, a5)
>> +#define __libc_message6(fmt, a1, a2, a3, a4, a5, a6) \
>> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4, a5, a6)
>> +#define __libc_message7(fmt, a1, a2, a3, a4, a5, a6, a7) \
>> +   __libc_message_impl (__libc_fatal_vma_name, fmt, a1, a2, a3, a4, a5, a6, a7)
>> +
>> +#define __libc_assert_message0(fmt) \
>> +   __libc_message_impl (__libc_assert_vma_name, fmt)
>> +#define __libc_assert_message1(fmt, a1) \
>> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1)
>> +#define __libc_assert_message2(fmt, a1, a2) \
>> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2)
>> +#define __libc_assert_message3(fmt, a1, a2, a3) \
>> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3)
>> +#define __libc_assert_message4(fmt, a1, a2, a3, a4) \
>> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4)
>> +#define __libc_assert_message5(fmt, a1, a2, a3, a4, a5) \
>> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4, a5)
>> +#define __libc_assert_message6(fmt, a1, a2, a3, a4, a5, a6) \
>> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4, a5, a6)
>> +#define __libc_assert_message7(fmt, a1, a2, a3, a4, a5, a6, a7) \
>> +   __libc_message_impl (__libc_assert_vma_name, fmt, a1, a2, a3, a4, a5, a6, a7)
>>  
>>  #define __libc_message_concat_x(a,b)  a##b
>>  #define __libc_message_concat(a,b)    __libc_message_concat_x (a, b)
>>  
>> -#define __libc_message_nargs_x(a0,a1,a2,a3,a4,a5,a6,...) a6
>> +#define __libc_message_nargs_x(a0,a1,a2,a3,a4,a5,a6,a7,...) a7
>>  #define __libc_message_nargs(b, ...) \
>> -   __libc_message_nargs_x (__VA_ARGS__,6,5,4,3,2,1,0,)
>> +   __libc_message_nargs_x (__VA_ARGS__,7,6,5,4,3,2,1,0,)
>>  #define __libc_message_disp(b, ...) \
>>     __libc_message_concat (b, __libc_message_nargs (__VA_ARGS__))(__VA_ARGS__)
> 
> Why do we need __libc_message_nargs when things go into a varargs
> function without an explicit argument count in the end anyway?

This is the same logic from sysdep/unix/syscall.h, used on INLINE_SYSCALL_CALL,
and it is required to correctly paste the argument is the expected order.  I
think maybe there is a better way to accomplish it.

> 
> The patch no longer applies to the main branch.

Ack, I need to handle the __libc_message_impl testcase as well.

> 
> Thanks,
> Florian
>
  

* Adhemerval Zanella Netto:

>> Why do we need __libc_message_nargs when things go into a varargs
>> function without an explicit argument count in the end anyway?
>
> This is the same logic from sysdep/unix/syscall.h, used on INLINE_SYSCALL_CALL,
> and it is required to correctly paste the argument is the expected order.  I
> think maybe there is a better way to accomplish it.

This is used for INLINE_SYSCALL_CALL etc. because the final target is
not variadic, but has a fixed number of arguments (loading only the
registers that are really required).  This isn't the case here as far as
I can see: the final function call is still variadic.

Thanks,
Florian
  

On 19/08/25 18:32, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
>>> Why do we need __libc_message_nargs when things go into a varargs
>>> function without an explicit argument count in the end anyway?
>>
>> This is the same logic from sysdep/unix/syscall.h, used on INLINE_SYSCALL_CALL,
>> and it is required to correctly paste the argument is the expected order.  I
>> think maybe there is a better way to accomplish it.
> 
> This is used for INLINE_SYSCALL_CALL etc. because the final target is
> not variadic, but has a fixed number of arguments (loading only the
> registers that are really required).  This isn't the case here as far as
> I can see: the final function call is still variadic.

I used the same strategy to try catch misuse with more arguments than
supported at compile time, but since eeb7b079d5d8785e760ca08c3692ffa579bbb737
we also have a runtime check so this is not strictly required. I still
think this is a nice think to have.
  

* Adhemerval Zanella Netto:

> On 19/08/25 18:32, Florian Weimer wrote:
>> * Adhemerval Zanella Netto:
>> 
>>>> Why do we need __libc_message_nargs when things go into a varargs
>>>> function without an explicit argument count in the end anyway?
>>>
>>> This is the same logic from sysdep/unix/syscall.h, used on INLINE_SYSCALL_CALL,
>>> and it is required to correctly paste the argument is the expected order.  I
>>> think maybe there is a better way to accomplish it.
>> 
>> This is used for INLINE_SYSCALL_CALL etc. because the final target is
>> not variadic, but has a fixed number of arguments (loading only the
>> registers that are really required).  This isn't the case here as far as
>> I can see: the final function call is still variadic.
>
> I used the same strategy to try catch misuse with more arguments than
> supported at compile time, but since eeb7b079d5d8785e760ca08c3692ffa579bbb737
> we also have a runtime check so this is not strictly required. I still
> think this is a nice think to have.

You could use __builtin_va_arg_pack_len for that, at least when building
with GCC.  I think it's much clearer.

Thanks,
Florian
  

On 20/08/25 08:12, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
>> On 19/08/25 18:32, Florian Weimer wrote:
>>> * Adhemerval Zanella Netto:
>>>
>>>>> Why do we need __libc_message_nargs when things go into a varargs
>>>>> function without an explicit argument count in the end anyway?
>>>>
>>>> This is the same logic from sysdep/unix/syscall.h, used on INLINE_SYSCALL_CALL,
>>>> and it is required to correctly paste the argument is the expected order.  I
>>>> think maybe there is a better way to accomplish it.
>>>
>>> This is used for INLINE_SYSCALL_CALL etc. because the final target is
>>> not variadic, but has a fixed number of arguments (loading only the
>>> registers that are really required).  This isn't the case here as far as
>>> I can see: the final function call is still variadic.
>>
>> I used the same strategy to try catch misuse with more arguments than
>> supported at compile time, but since eeb7b079d5d8785e760ca08c3692ffa579bbb737
>> we also have a runtime check so this is not strictly required. I still
>> think this is a nice think to have.
> 
> You could use __builtin_va_arg_pack_len for that, at least when building
> with GCC.  I think it's much clearer.

I think this is a long shot, but I do want to eventually resubmit the clang
build patches and relying on this macro will an extra work to disable with
clang.