assert: Refactor assert/assert_perror

It now calls __libc_assert, which contains similar logic. The assert
call only requires memory allocation for the message translation, 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 now writes bytes
directly without going through the wide stream state.

Checked on aarch64-linux-gnu.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

diff --git a/assert/__libc_assert_fail.c b/assert/__libc_assert_fail.c
index b50637a8936d340dccc6e71d985091f55891f763..d9fc37dc963196cee3ea0ab0c63c5a2b88a87259 100644 (file)
--- a/assert/__libc_assert_fail.c
+++ b/assert/__libc_assert_fail.c
@@ -28,6 +28,7 @@ __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 (
+      "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 83f0b3a76fd2e39b2399cdb9d60ad6689e23ccab..04f890bcabf78ae21ae398da0c63e014eeabb30d 100644 (file)
--- 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 d21d76fa62607d2f6a96591447309f2b2aa8772b..796f6bdec10e80695da6b3518a4d785fbe4a58c4 100644 (file)
--- 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 (_("%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 6d54ef9ba6fb5ee200ece3891d23c0b93f507d57..9997c98d1ae42f71f5613cdf94f36cbf1cb2b0fd 100644 (file)
--- 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 3d917dba5baf96b072e772196604373ea499c3dd..17f5516c2b477fdb61c4b35295af80d683812ab0 100644 (file)
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -171,36 +171,35 @@ 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
 
 #define IOVEC_MAX_ERR_MSG "Fatal glibc error: Internal " \
                          "__libc_message error. Too many arguments.\n"
 #define IOVEC_MAX_ERR_MSG_LEN (sizeof (IOVEC_MAX_ERR_MSG) - 1)
 
-_Noreturn void __libc_message_impl (const char *__fnt, ...) attribute_hidden
-     __attribute__ ((__format__ (__printf__, 1, 2)));
-
-#define __libc_message0(fmt) \
-   __libc_message_impl (fmt)
-#define __libc_message1(fmt, a1) \
-   __libc_message_impl (fmt, a1)
-#define __libc_message2(fmt, a1, a2) \
-   __libc_message_impl (fmt, a1, a2)
-#define __libc_message3(fmt, a1, a2, a3) \
-   __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_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(b, ...) \
-   __libc_message_nargs_x (__VA_ARGS__,6,5,4,3,2,1,0,)
-#define __libc_message_disp(b, ...) \
-   __libc_message_concat (b, __libc_message_nargs (__VA_ARGS__))(__VA_ARGS__)
+_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"
+
+static inline _Noreturn void __libc_message_wrapper (const char *vmaname,
+                                                    const char *fmt, ...)
+{
+#ifdef __va_arg_pack_len
+  if (__va_arg_pack_len () > LIBC_MESSAGE_MAX_ARGS)
+    {
+      __errordecl (__libc_message_error, "invalid number of arguments");
+      __libc_message_error ();
+    }
+#endif
+  __libc_message_impl (vmaname, fmt, __va_arg_pack ());
+}
 #define __libc_message(...) \
-   __libc_message_disp (__libc_message, __VA_ARGS__)
+   __libc_message_wrapper (__libc_fatal_vma_name, __VA_ARGS__)
+#define __libc_assert(...) \
+   __libc_message_wrapper (__libc_assert_vma_name, __VA_ARGS__)
 
 /* Acquire ownership of STREAM.  */
 extern void __flockfile (FILE *__stream) attribute_hidden;

diff --git a/posix/tst-libc-message.c b/posix/tst-libc-message.c
index b85195e338bd147bb9fb0f8fac4aaba752912f89..a76243936bab6a448edade81f4a83465c9a847b3 100644 (file)
--- a/posix/tst-libc-message.c
+++ b/posix/tst-libc-message.c
@@ -25,8 +25,11 @@
 static _Noreturn void
 run_libc_message (void *closure)
 {
-  /* We only support 4 arguments.  Call with 5 to trigger failure.  */
-  __libc_message_impl ("%s %s %s %s %s\n", "1", "2", "3", "4", "5");
+  /* We only support 7 (LIBC_MESSAGE_MAX_ARGS) arguments.  Call with 8 to
+     trigger failure.  */
+  __libc_message_impl ("glibc: test",
+                      "%s %s %s %s %s %s %s %s\n",
+                      "1", "2", "3", "4", "5", "6", "7", "8");
   __builtin_unreachable ();
 }
 

diff --git a/sysdeps/posix/libc_fatal.c b/sysdeps/posix/libc_fatal.c
index 6f75197e96629c4b3ee4b2df3ee512cda3e920b4..f9e5318d92c3fdbbbd5089e98b41143bdb9fa3d2 100644 (file)
--- a/sysdeps/posix/libc_fatal.c
+++ b/sysdeps/posix/libc_fatal.c
@@ -43,7 +43,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;
@@ -123,7 +123,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.  */