Message ID | ZcpQuw5ndmIpXrdc@tucnak |
---|---|
State | New |
Headers | show |
Series | libiberty: Fix up libiberty_vprintf_buffer_size | expand |
> 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 >
--- 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 *));