_dl_exception_create_format: Support %x/%lx/%Zx
Commit Message
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
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, ...)
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
@@ -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]
@@ -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
@@ -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");
new file mode 100644
@@ -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