_dl_exception_create_format: Support %x/%lx/%Zx

Message ID 20181122173812.9025-1-hjl.tools@gmail.com
State New
Headers show
Series
  • _dl_exception_create_format: Support %x/%lx/%Zx
Related show

Commit Message

H.J. Lu Nov. 22, 2018, 5:38 p.m.
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.
---
 elf/dl-exception.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

-- 
2.19.1

Comments

Szabolcs Nagy Nov. 23, 2018, 10:53 a.m. | #1
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."
H.J. Lu Nov. 23, 2018, 4:18 p.m. | #2
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.
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.
---
 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");
Adhemerval Zanella Nov. 27, 2018, 3:48 p.m. | #3
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>
---
H.J. Lu Nov. 27, 2018, 9:33 p.m. | #4
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.

Thanks.

-- 
H.J.
From e39a9750c19a4101bd6bfdfe121257de1b4487e6 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.

2018-11-27  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.
---
 elf/Makefile             |   3 +-
 elf/dl-exception.c       |  40 ++++++++++++++++
 elf/tst-create_format1.c | 101 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 elf/tst-create_format1.c

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..cae23e1824
--- /dev/null
+++ b/elf/tst-create_format1.c
@@ -0,0 +1,101 @@
+/* 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);
+}
+
+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>
H.J. Lu Nov. 29, 2018, 10:48 p.m. | #5
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.

-- 
H.J.
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>
Joseph Myers Nov. 30, 2018, 12:02 a.m. | #6
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, ...)

-- 
Joseph S. Myers
joseph@codesourcery.com
H.J. Lu Nov. 30, 2018, 2:48 a.m. | #7
On Thu, Nov 29, 2018 at 4:02 PM Joseph Myers <joseph@codesourcery.com> wrote:
>

> 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, ...)

>


I am testing this patch now.


-- 
H.J.
---
diff --git a/elf/dl-exception.c b/elf/dl-exception.c
index 1e41d89a7d..3e8e0ba3f1 100644
--- a/elf/dl-exception.c
+++ b/elf/dl-exception.c
@@ -23,6 +23,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <unistd.h>
+#include <_itoa.h>

 /* This message we return as a last resort.  We define the string in a
    variable since we have to avoid freeing it and so have to enable

Patch

diff --git a/elf/dl-exception.c b/elf/dl-exception.c
index 1c63e4a3a6..2144998261 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");