diff mbox

_dl_exception_create_format: Support %x/%lx/%Zx

Message ID CAMe9rOojP1NpHVWURc+YPR_jAug2QrYdiXdPt0t9w96bNsnUTg@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu Nov. 29, 2018, 10:48 p.m. UTC
On Tue, Nov 27, 2018 at 1:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Nov 27, 2018 at 7:49 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
> >
> >
> >
> > On 23/11/2018 14:18, H.J. Lu wrote:
> > > On Fri, Nov 23, 2018 at 2:53 AM Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote:
> > >> On 22/11/18 17:38, H.J. Lu wrote:
> > >>> Add support for %x, %lx and %Zx to _dl_exception_create_format and pad
> > >>> to the full width with 0.
> > >>>
> > >>>       * elf/dl-exception.c (_dl_exception_create_format): Support
> > >>>       %x, %lx and %Zx.
> > >> why Z and not z?  the glibc man says
> > >>
> > >> " Z   A nonstandard synonym for z that predates the appearance of z.
> > >>   Do not use in new code."
> > > Like this?
> > >
> > > -- H.J.
> > >
> > >
> > > 0001-_dl_exception_create_format-Support-x-lx-zx.patch
> > >
> > > From d937bf4bf3b92897b1ff60406ca065e2968bcdbe Mon Sep 17 00:00:00 2001
> > > From: "H.J. Lu" <hjl.tools@gmail.com>
> > > Date: Thu, 22 Nov 2018 09:02:40 -0800
> > > Subject: [PATCH] _dl_exception_create_format: Support %x/%lx/%zx
> > >
> > > Add support for %x, %lx and %zx to _dl_exception_create_format and pad
> > > to the full width with 0.
> > >
> > >       * elf/dl-exception.c (_dl_exception_create_format): Support
> > >       %x, %lx and %zx.
> >
> > LGTM.
> >
> > > ---
> > >  elf/dl-exception.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 40 insertions(+)
> > >
> > > diff --git a/elf/dl-exception.c b/elf/dl-exception.c
> > > index 1c63e4a3a6..1e41d89a7d 100644
> > > --- a/elf/dl-exception.c
> > > +++ b/elf/dl-exception.c
> > > @@ -111,6 +111,20 @@ _dl_exception_create_format (struct dl_exception *exception, const char *objname
> > >              case 's':
> > >                length += strlen (va_arg (ap, const char *));
> > >                break;
> > > +           /* Recognize the l modifier.  It is only important on some
> > > +              platforms where long and int have a different size.  We
> > > +              can use the same code for size_t.  */
> > > +         case 'l':
> > > +         case 'z':
> > > +           if (p[1] == 'x')
> > > +             {
> > > +               length += LONG_WIDTH / 4;
> > > +               ++p;
> > > +               break;
> > > +             }
> > > +         case 'x':
> > > +           length += INT_WIDTH / 4;
> > > +           break;
> > >              default:
> > >                /* Assumed to be '%'.  */
> > >                ++length;
> > > @@ -167,6 +181,32 @@ _dl_exception_create_format (struct dl_exception *exception, const char *objname
> > >                *wptr = '%';
> > >                ++wptr;
> > >                break;
> > > +         case 'x':
> > > +           {
> > > +             unsigned long int num = va_arg (ap, unsigned int);
> > > +             char *start = wptr;
> > > +             wptr += INT_WIDTH / 4;
> > > +             char *cp = _itoa (num, wptr, 16, 0);
> > > +             /* Pad to the full width with 0.  */
> > > +             while (cp != start)
> > > +               *--cp = '0';
> > > +           }
> > > +           break;
> > > +         case 'l':
> > > +         case 'z':
> > > +           if (p[1] == 'x')
> > > +             {
> > > +               unsigned long int num = va_arg (ap, unsigned long int);
> > > +               char *start = wptr;
> > > +               wptr += LONG_WIDTH / 4;
> > > +               char *cp = _itoa (num, wptr, 16, 0);
> > > +               /* Pad to the full width with 0.  */
> > > +               while (cp != start)
> > > +                 *--cp = '0';
> > > +               ++p;
> > > +               break;
> > > +             }
> > > +            /* FALLTHROUGH */
> > >              default:
> > >                _dl_fatal_printf ("Fatal error:"
> > >                                  " invalid format in exception string\n");
> > > -- 2.19.1
> > >
> >
> > What about adding a test for _dl_exception_create_format, such as:
> >
> > ---
> > #include <ldsodefs.h>
> > #include <array_length.h>
> >
> > #include <support/check.h>
> > #include <support/xunistd.h>
> > #include <support/capture_subprocess.h>
> >
> > #define TEST(es, objn, fmt, ...)                                        \
> >   ({                                                                    \
> >     struct dl_exception exception;                                      \
> >     _dl_exception_create_format (&exception, objn, fmt, __VA_ARGS__);   \
> >     TEST_COMPARE_STRING (exception.objname, objn == NULL ? "" : objn);  \
> >     TEST_COMPARE_STRING (exception.errstring, es);                      \
> >     _dl_exception_free (&exception);                                    \
> >   })
> >
> > static void
> > do_test_invalid_conversion (void *closure)
> > {
> >   TEST ("(null)", NULL, "%p", NULL);
> > }
> >
> > /* Exit status after abnormal termination.  */
> > static int invalid_status;
> >
> > static void
> > init_invalid_status (void)
> > {
> >   pid_t pid = xfork ();
> >   if (pid == 0)
> >     _exit (127);
> >   xwaitpid (pid, &invalid_status, 0);
> > }
> >
> > static int
> > do_test (void)
> > {
> >   init_invalid_status ();
> >
> >   TEST ("test",      NULL,   "%s",      "test");
> >   TEST ("test-test", NULL,   "%s-test", "test");
> >   TEST ("test",      "test", "%s",      "test");
> >   TEST ("test-test", "test", "%s-test", "test");
> >
> >   TEST ("test%",      NULL,   "%s%%",      "test");
> >   TEST ("test%-test", NULL,   "%s%%-test", "test");
> >   TEST ("test%",      "test", "%s%%",      "test");
> >   TEST ("test%-test", "test", "%s%%-test", "test");
> >
> >   TEST ("0000007b",      NULL,   "%x",      123);
> >   TEST ("0000007b-test", NULL,   "%x-test", 123);
> >   TEST ("0000007b",      "test", "%x",      123);
> >   TEST ("0000007b-test", "test", "%x-test", 123);
> >
> > #define TEST_LONG(es, objn, fmt, ...)                                   \
> >   ({                                                                    \
> >     if (sizeof (int) == sizeof (long int))                              \
> >       TEST (es, objn, fmt, __VA_ARGS__);                                \
> >     else                                                                \
> >       TEST ("ffffffff" es, objn, fmt, __VA_ARGS__);                     \
> >   })
> >
> >   TEST_LONG ("fffffffd",      NULL,   "%lx",      (long int)~2ul);
> >   TEST_LONG ("fffffffd-test", NULL,   "%lx-test", (long int)~2ul);
> >   TEST_LONG ("fffffffd",      "test", "%lx",      (long int)~2ul);
> >   TEST_LONG ("fffffffd-test", "test", "%lx-test", (long int)~2ul);
> >
> >   TEST_LONG ("fffffffe",      NULL,   "%zx",      (size_t)~1ul);
> >   TEST_LONG ("fffffffe-test", NULL,   "%zx-test", (size_t)~1ul);
> >   TEST_LONG ("fffffffe",      "test", "%zx",      (size_t)~1ul);
> >   TEST_LONG ("fffffffe-test", "test", "%zx-test", (size_t)~1ul);
> >
> >   {
> >     struct support_capture_subprocess result;
> >     result = support_capture_subprocess (do_test_invalid_conversion, NULL);
> >     support_capture_subprocess_check (&result, "dl-exception", invalid_status,
> >                                       sc_allow_stderr);
> >     TEST_COMPARE_STRING (result.err.buffer,
> >                          "Fatal error: invalid format in exception string\n");
> >   }
> >
> >   return 0;
> > }
> >
> > #include <support/test-driver.c>
> > ---
>
> This is what I am going to check in.
>

This is what I actually checked in.

Comments

Joseph Myers Nov. 30, 2018, 12:02 a.m. UTC | #1
On Thu, 29 Nov 2018, H.J. Lu wrote:

> This is what I actually checked in.

Your commit message doesn't say anything about how the commit was tested 
(please include that information in commit messages).  It causes a lot of 
build failures of the form:

dl-exception.c: In function '_dl_exception_create_format':
dl-exception.c:189:14: error: implicit declaration of function '_itoa' 
[-Werror=implicit-function-declaration]
   char *cp = _itoa (num, wptr, 16, 0);
              ^~~~~
dl-exception.c:189:14: error: initialization of 'char *' from 'int' makes 
pointer from integer without a cast [-Werror=int-conversion]
dl-exception.c:202:16: error: initialization of 'char *' from 'int' makes 
pointer from integer without a cast [-Werror=int-conversion]
     char *cp = _itoa (num, wptr, 16, 0);
                ^~~~~

(on aarch64, arm, hppa, i486, m68k, mips, ...)
diff mbox

Patch

From a5275ba5378c9256d18e582572b4315e8edfcbfb Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 29 Nov 2018 14:15:01 -0800
Subject: [PATCH] _dl_exception_create_format: Support %x/%lx/%zx

Add support for %x, %lx and %zx to _dl_exception_create_format and pad
to the full width with 0.

	* elf/Makefile (tests-internal): Add tst-create_format1.
	* elf/dl-exception.c (_dl_exception_create_format): Support
	%x, %lx and %zx.
	* elf/tst-create_format1.c: New file.
---
 ChangeLog                |   8 +++
 elf/Makefile             |   3 +-
 elf/dl-exception.c       |  40 +++++++++++++++
 elf/tst-create_format1.c | 103 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 153 insertions(+), 1 deletion(-)
 create mode 100644 elf/tst-create_format1.c

diff --git a/ChangeLog b/ChangeLog
index 1551dc0ec3..f852a8b299 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@ 
+2018-11-29  H.J. Lu  <hongjiu.lu@intel.com>
+	    Adhemerval Zanella  <adhemerval.zanella@linaro.org>
+
+	* elf/Makefile (tests-internal): Add tst-create_format1.
+	* elf/dl-exception.c (_dl_exception_create_format): Support
+	%x, %lx and %zx.
+	* elf/tst-create_format1.c: New file.
+
 2018-11-29  Charles-Antoine Couret  <charles-antoine.couret@essensium.com>
 
 	* argp/argp-fmtstream.c (__argp_fmtstream_update): Use [_LIBC]
diff --git a/elf/Makefile b/elf/Makefile
index d72e7b67f6..71f5896a6d 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -192,7 +192,8 @@  tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
 	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
-	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym
+	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \
+	 tst-create_format1
 ifeq ($(build-hardcoded-path-in-tests),yes)
 tests += tst-dlopen-aout
 tst-dlopen-aout-no-pie = yes
diff --git a/elf/dl-exception.c b/elf/dl-exception.c
index 1c63e4a3a6..1e41d89a7d 100644
--- a/elf/dl-exception.c
+++ b/elf/dl-exception.c
@@ -111,6 +111,20 @@  _dl_exception_create_format (struct dl_exception *exception, const char *objname
             case 's':
               length += strlen (va_arg (ap, const char *));
               break;
+	      /* Recognize the l modifier.  It is only important on some
+		 platforms where long and int have a different size.  We
+		 can use the same code for size_t.  */
+	    case 'l':
+	    case 'z':
+	      if (p[1] == 'x')
+		{
+		  length += LONG_WIDTH / 4;
+		  ++p;
+		  break;
+		}
+	    case 'x':
+	      length += INT_WIDTH / 4;
+	      break;
             default:
               /* Assumed to be '%'.  */
               ++length;
@@ -167,6 +181,32 @@  _dl_exception_create_format (struct dl_exception *exception, const char *objname
               *wptr = '%';
               ++wptr;
               break;
+	    case 'x':
+	      {
+		unsigned long int num = va_arg (ap, unsigned int);
+		char *start = wptr;
+		wptr += INT_WIDTH / 4;
+		char *cp = _itoa (num, wptr, 16, 0);
+		/* Pad to the full width with 0.  */
+		while (cp != start)
+		  *--cp = '0';
+	      }
+	      break;
+	    case 'l':
+	    case 'z':
+	      if (p[1] == 'x')
+		{
+		  unsigned long int num = va_arg (ap, unsigned long int);
+		  char *start = wptr;
+		  wptr += LONG_WIDTH / 4;
+		  char *cp = _itoa (num, wptr, 16, 0);
+		  /* Pad to the full width with 0.  */
+		  while (cp != start)
+		    *--cp = '0';
+		  ++p;
+		  break;
+		}
+	       /* FALLTHROUGH */
             default:
               _dl_fatal_printf ("Fatal error:"
                                 " invalid format in exception string\n");
diff --git a/elf/tst-create_format1.c b/elf/tst-create_format1.c
new file mode 100644
index 0000000000..8b9edfdc69
--- /dev/null
+++ b/elf/tst-create_format1.c
@@ -0,0 +1,103 @@ 
+/* Check _dl_exception_create_format.
+   Copyright (C) 2018 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <ldsodefs.h>
+#include <array_length.h>
+
+#include <support/check.h>
+#include <support/xunistd.h>
+#include <support/capture_subprocess.h>
+
+#define TEST(es, objn, fmt, ...)					\
+  ({									\
+     struct dl_exception exception;					\
+     _dl_exception_create_format (&exception, objn, fmt, __VA_ARGS__);	\
+     TEST_COMPARE_STRING (exception.objname, objn == NULL ? "" : objn);	\
+     TEST_COMPARE_STRING (exception.errstring, es);			\
+     _dl_exception_free (&exception);					\
+   })
+
+static void
+do_test_invalid_conversion (void *closure)
+{
+  TEST ("(null)", NULL, "%p", NULL);
+}
+
+/* Exit status after abnormal termination.  */
+static int invalid_status;
+
+static void
+init_invalid_status (void)
+{
+  pid_t pid = xfork ();
+  if (pid == 0)
+    _exit (127);
+  xwaitpid (pid, &invalid_status, 0);
+  if (WIFEXITED (invalid_status))
+    invalid_status = WEXITSTATUS (invalid_status);
+}
+
+static int
+do_test (void)
+{
+  init_invalid_status ();
+
+  TEST ("test",      NULL,   "%s",      "test");
+  TEST ("test-test", NULL,   "%s-test", "test");
+  TEST ("test",      "test", "%s",      "test");
+  TEST ("test-test", "test", "%s-test", "test");
+
+  TEST ("test%",      NULL,   "%s%%",      "test");
+  TEST ("test%-test", NULL,   "%s%%-test", "test");
+  TEST ("test%",      "test", "%s%%",      "test");
+  TEST ("test%-test", "test", "%s%%-test", "test");
+
+  TEST ("0000007b",      NULL,   "%x",      123);
+  TEST ("0000007b-test", NULL,   "%x-test", 123);
+  TEST ("0000007b",      "test", "%x",      123);
+  TEST ("0000007b-test", "test", "%x-test", 123);
+
+#define TEST_LONG(es, objn, fmt, ...)				\
+  ({								\
+     if (sizeof (int) == sizeof (long int))			\
+       TEST (es, objn, fmt, __VA_ARGS__);			\
+     else							\
+       TEST ("ffffffff" es, objn, fmt, __VA_ARGS__);		\
+   })
+
+  TEST_LONG ("fffffffd",      NULL,   "%lx",      (long int)~2ul);
+  TEST_LONG ("fffffffd-test", NULL,   "%lx-test", (long int)~2ul);
+  TEST_LONG ("fffffffd",      "test", "%lx",      (long int)~2ul);
+  TEST_LONG ("fffffffd-test", "test", "%lx-test", (long int)~2ul);
+
+  TEST_LONG ("fffffffe",      NULL,   "%zx",      (size_t)~1ul);
+  TEST_LONG ("fffffffe-test", NULL,   "%zx-test", (size_t)~1ul);
+  TEST_LONG ("fffffffe",      "test", "%zx",      (size_t)~1ul);
+  TEST_LONG ("fffffffe-test", "test", "%zx-test", (size_t)~1ul);
+
+  struct support_capture_subprocess result;
+  result = support_capture_subprocess (do_test_invalid_conversion, NULL);
+  support_capture_subprocess_check (&result, "dl-exception",
+				    invalid_status, sc_allow_stderr);
+  TEST_COMPARE_STRING (result.err.buffer,
+		       "Fatal error: invalid format in exception string\n");
+
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.19.2