_dl_exception_create_format: Support %x/%lx/%Zx

  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?
  

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>
---