assert: Refactor assert/assert_perror

It now calls __libc_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.
---
 assert/assert-perr.c   |  22 ++++++--
 assert/assert.c        | 112 ++++++-----------------------------------
 assert/test-assert-2.c |  18 ++-----
 include/stdio.h        |  12 +++--
 4 files changed, 46 insertions(+), 118 deletions(-)

Ping.

On 31/01/25 16:10, Adhemerval Zanella wrote:
> It now calls __libc_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.
> ---
>  assert/assert-perr.c   |  22 ++++++--
>  assert/assert.c        | 112 ++++++-----------------------------------
>  assert/test-assert-2.c |  18 ++-----
>  include/stdio.h        |  12 +++--
>  4 files changed, 46 insertions(+), 118 deletions(-)
> 
> diff --git a/assert/assert-perr.c b/assert/assert-perr.c
> index 83f0b3a76f..04f890bcab 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_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..499a90f4c6 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_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..b3d9ed0d7a 100644
> --- a/include/stdio.h
> +++ b/include/stdio.h
> @@ -171,7 +171,7 @@ 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)));
> @@ -186,13 +186,19 @@ _Noreturn void __libc_message_impl (const char *__fnt, ...) attribute_hidden
>     __libc_message_impl (fmt, a1, a2, a3)
>  #define __libc_message4(fmt, a1, a2, a3, a4) \
>     __libc_message_impl (fmt, a1, a2, a3, a4)
> +#define __libc_message5(fmt, a1, a2, a3, a4, a5) \
> +   __libc_message_impl (fmt, a1, a2, a3, a4, a5)
> +#define __libc_message6(fmt, a1, a2, a3, a4, a5, a6) \
> +   __libc_message_impl (fmt, a1, a2, a3, a4, a5, a6)
> +#define __libc_message7(fmt, a1, a2, a3, a4, a5, a6, a7) \
> +   __libc_message_impl (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(...) \

On Jan 31 2025, Adhemerval Zanella wrote:

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

Though __libc_message uses "glibc: fatal" for the VMA name instead of
"glibc: assert".

On 31/03/25 11:37, Andreas Schwab wrote:
> On Jan 31 2025, Adhemerval Zanella wrote:
> 
>> It now calls __libc_message, which contains similar logic. The assert
>> now does not require any dynamic memory allocation, so test-assert2.c
>> is adapted to handle it.
> 
> Though __libc_message uses "glibc: fatal" for the VMA name instead of
> "glibc: assert".
> 

Indeed, I will fix it.