From patchwork Thu Nov 29 22:48:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 30407 Received: (qmail 31158 invoked by alias); 29 Nov 2018 22:49:31 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 30530 invoked by uid 89); 29 Nov 2018 22:49:14 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.4 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SEM_URI, SEM_URIRED, SPF_PASS autolearn=ham version=3.3.2 spammy=Szabolcs, szabolcs, predates, modifier X-HELO: mail-ot1-f66.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=EkmkrXKiDeMbbELSOY/ZVRDPe0b6lR79xFVh3fwx14k=; b=EREDn8Gf4jXmyeBXxxp6uJLfGZnC8rkev0U97gwbSaYAa3rR/x2gTCR0b+JBnm4W5n ClD7TWEz+gvi7IaB/X9ZbmyMD4zT8RqnS0uW6hjrd9b0nHNaevZ9jLcCrN8R8qqG/SaI 38rCOcUHXxKZMqR1826V9xPc8DUy55lsEJLDgBNm5tTONZPgI9+px9VGIEmf48JwmZEI jnSJJxEtuq0S6hLCmE45usOh1TGMwvwfug6PJfWQhBQfMmisYiZC3v8E7TKCT7HJYb6F YwduRXLjTTshSc+3xwlZAYBY/BfLRdOUG70ZLgYsgShUoAebbdeLf8Hdrq5bcPD6UJdQ T+bA== MIME-Version: 1.0 References: <20181122173812.9025-1-hjl.tools@gmail.com> <7794985c-9c57-58ce-91af-e737b8bc82e2@arm.com> <15c39c60-150e-fe2a-a511-fb735b152d9b@linaro.org> In-Reply-To: From: "H.J. Lu" Date: Thu, 29 Nov 2018 14:48:31 -0800 Message-ID: Subject: Re: [PATCH] _dl_exception_create_format: Support %x/%lx/%Zx To: Adhemerval Zanella Cc: GNU C Library On Tue, Nov 27, 2018 at 1:33 PM H.J. Lu wrote: > > On Tue, Nov 27, 2018 at 7:49 AM Adhemerval Zanella > wrote: > > > > > > > > On 23/11/2018 14:18, H.J. Lu wrote: > > > On Fri, Nov 23, 2018 at 2:53 AM Szabolcs Nagy 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" > > > 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 > > #include > > > > #include > > #include > > #include > > > > #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 > > --- > > This is what I am going to check in. > This is what I actually checked in. From a5275ba5378c9256d18e582572b4315e8edfcbfb Mon Sep 17 00:00:00 2001 From: "H.J. Lu" 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 + Adhemerval Zanella + + * 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 * 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 + . */ + +#include +#include + +#include +#include +#include + +#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 -- 2.19.2