libiberty: Fix up libiberty_vprintf_buffer_size
Checks
Commit Message
Hi!
When writing the HOST_SIZE_T_PRINT_UNSIGNED incremental patch,
my first bootstrap failed on i686-linux. That is because I've also had
@@ -1344,8 +1344,10 @@ adjust_field_rtx_def (type_p t, options_
}
subfields = create_field (subfields, t,
- xasprintf (".fld[%lu].%s",
- (unsigned long) aindex,
+ xasprintf (".fld["
+ HOST_SIZE_T_PRINT_UNSIGNED
+ "].%s",
+ (fmt_size_t) aindex,
subname));
subfields->opt = nodot;
if (t == note_union_tp)
hunk in gengtype.cc. While sprintf obviously can print in this case %llu
with fmt_size_t being unsigned long long (that is another bug I'll fix
incrementally), seems libiberty_vprintf_buffer_size
can't deal with that, it ignores h, hh, l, ll and L modifiers and
unconditionally, estimates 30 chars as upper bounds for integers (that is
fine) and then uses (void) va_arg (ap, int); to skip over the argument
regardless if it was %d, %ld, %lld, %hd, %hhd etc.
Now, on x86_64 that happens to work fine probably for all of those,
on ia32 for everything but %lld, because it then skips just one half
of the long long argument; now as there is %s after it, it will try to
compute strlen not from the pointer argument corresponding to %s, but
from the most significant half of the previous long long argument.
So, the following patch attempts not to completely ignore the modifiers,
but figure out from them whether to va_arg an int (used for h and hh as
well), or long, or long long, or size_t, or ptrdiff_t - added support for
z and t there, plus for Windows I64. And also %Lf etc. for long double.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2024-02-12 Jakub Jelinek <jakub@redhat.com>
* vprintf-support.c (libiberty_vprintf_buffer_size): Handle
properly l, ll, z, t or on _WIN32 I64 modifiers for diouxX
and L modifier for fFgGeE.
Jakub
Comments
> Am 12.02.2024 um 18:10 schrieb Jakub Jelinek <jakub@redhat.com>:
>
> Hi!
>
> When writing the HOST_SIZE_T_PRINT_UNSIGNED incremental patch,
> my first bootstrap failed on i686-linux. That is because I've also had
> @@ -1344,8 +1344,10 @@ adjust_field_rtx_def (type_p t, options_
> }
>
> subfields = create_field (subfields, t,
> - xasprintf (".fld[%lu].%s",
> - (unsigned long) aindex,
> + xasprintf (".fld["
> + HOST_SIZE_T_PRINT_UNSIGNED
> + "].%s",
> + (fmt_size_t) aindex,
> subname));
> subfields->opt = nodot;
> if (t == note_union_tp)
> hunk in gengtype.cc. While sprintf obviously can print in this case %llu
> with fmt_size_t being unsigned long long (that is another bug I'll fix
> incrementally), seems libiberty_vprintf_buffer_size
> can't deal with that, it ignores h, hh, l, ll and L modifiers and
> unconditionally, estimates 30 chars as upper bounds for integers (that is
> fine) and then uses (void) va_arg (ap, int); to skip over the argument
> regardless if it was %d, %ld, %lld, %hd, %hhd etc.
> Now, on x86_64 that happens to work fine probably for all of those,
> on ia32 for everything but %lld, because it then skips just one half
> of the long long argument; now as there is %s after it, it will try to
> compute strlen not from the pointer argument corresponding to %s, but
> from the most significant half of the previous long long argument.
>
> So, the following patch attempts not to completely ignore the modifiers,
> but figure out from them whether to va_arg an int (used for h and hh as
> well), or long, or long long, or size_t, or ptrdiff_t - added support for
> z and t there, plus for Windows I64. And also %Lf etc. for long double.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok
Richard
> 2024-02-12 Jakub Jelinek <jakub@redhat.com>
>
> * vprintf-support.c (libiberty_vprintf_buffer_size): Handle
> properly l, ll, z, t or on _WIN32 I64 modifiers for diouxX
> and L modifier for fFgGeE.
>
> --- libiberty/vprintf-support.c.jj 2024-01-03 12:07:48.464085595 +0100
> +++ libiberty/vprintf-support.c 2024-02-12 09:29:35.265651819 +0100
> @@ -56,6 +56,7 @@ libiberty_vprintf_buffer_size (const cha
> {
> if (*p++ == '%')
> {
> + int prec = 0;
> while (strchr ("-+ #0", *p))
> ++p;
> if (*p == '*')
> @@ -76,8 +77,43 @@ libiberty_vprintf_buffer_size (const cha
> else
> total_width += strtoul (p, (char **) &p, 10);
> }
> - while (strchr ("hlL", *p))
> - ++p;
> + do
> + {
> + switch (*p)
> + {
> + case 'h':
> + ++p;
> + continue;
> + case 'l':
> + case 'L':
> + ++prec;
> + ++p;
> + continue;
> + case 'z':
> + prec = 3;
> + ++p;
> + continue;
> + case 't':
> + prec = 4;
> + ++p;
> + continue;
> +#ifdef _WIN32
> + case 'I':
> + if (p[1] == '6' && p[2] == '4')
> + {
> + prec = 2;
> + p += 3;
> + continue;
> + }
> + break;
> +#endif
> + default:
> + break;
> + }
> + break;
> + }
> + while (1);
> +
> /* Should be big enough for any format specifier except %s and floats. */
> total_width += 30;
> switch (*p)
> @@ -88,6 +124,15 @@ libiberty_vprintf_buffer_size (const cha
> case 'u':
> case 'x':
> case 'X':
> + switch (prec)
> + {
> + case 0: (void) va_arg (ap, int); break;
> + case 1: (void) va_arg (ap, long int); break;
> + case 2: (void) va_arg (ap, long long int); break;
> + case 3: (void) va_arg (ap, size_t); break;
> + case 4: (void) va_arg (ap, ptrdiff_t); break;
> + }
> + break;
> case 'c':
> (void) va_arg (ap, int);
> break;
> @@ -96,10 +141,18 @@ libiberty_vprintf_buffer_size (const cha
> case 'E':
> case 'g':
> case 'G':
> - (void) va_arg (ap, double);
> - /* Since an ieee double can have an exponent of 307, we'll
> - make the buffer wide enough to cover the gross case. */
> - total_width += 307;
> + if (!prec)
> + {
> + (void) va_arg (ap, double);
> + /* Since an ieee double can have an exponent of 308, we'll
> + make the buffer wide enough to cover the gross case. */
> + total_width += 308;
> + }
> + else
> + {
> + (void) va_arg (ap, long double);
> + total_width += 4932;
> + }
> break;
> case 's':
> total_width += strlen (va_arg (ap, char *));
>
>
> Jakub
>
@@ -56,6 +56,7 @@ libiberty_vprintf_buffer_size (const cha
{
if (*p++ == '%')
{
+ int prec = 0;
while (strchr ("-+ #0", *p))
++p;
if (*p == '*')
@@ -76,8 +77,43 @@ libiberty_vprintf_buffer_size (const cha
else
total_width += strtoul (p, (char **) &p, 10);
}
- while (strchr ("hlL", *p))
- ++p;
+ do
+ {
+ switch (*p)
+ {
+ case 'h':
+ ++p;
+ continue;
+ case 'l':
+ case 'L':
+ ++prec;
+ ++p;
+ continue;
+ case 'z':
+ prec = 3;
+ ++p;
+ continue;
+ case 't':
+ prec = 4;
+ ++p;
+ continue;
+#ifdef _WIN32
+ case 'I':
+ if (p[1] == '6' && p[2] == '4')
+ {
+ prec = 2;
+ p += 3;
+ continue;
+ }
+ break;
+#endif
+ default:
+ break;
+ }
+ break;
+ }
+ while (1);
+
/* Should be big enough for any format specifier except %s and floats. */
total_width += 30;
switch (*p)
@@ -88,6 +124,15 @@ libiberty_vprintf_buffer_size (const cha
case 'u':
case 'x':
case 'X':
+ switch (prec)
+ {
+ case 0: (void) va_arg (ap, int); break;
+ case 1: (void) va_arg (ap, long int); break;
+ case 2: (void) va_arg (ap, long long int); break;
+ case 3: (void) va_arg (ap, size_t); break;
+ case 4: (void) va_arg (ap, ptrdiff_t); break;
+ }
+ break;
case 'c':
(void) va_arg (ap, int);
break;
@@ -96,10 +141,18 @@ libiberty_vprintf_buffer_size (const cha
case 'E':
case 'g':
case 'G':
- (void) va_arg (ap, double);
- /* Since an ieee double can have an exponent of 307, we'll
- make the buffer wide enough to cover the gross case. */
- total_width += 307;
+ if (!prec)
+ {
+ (void) va_arg (ap, double);
+ /* Since an ieee double can have an exponent of 308, we'll
+ make the buffer wide enough to cover the gross case. */
+ total_width += 308;
+ }
+ else
+ {
+ (void) va_arg (ap, long double);
+ total_width += 4932;
+ }
break;
case 's':
total_width += strlen (va_arg (ap, char *));