[PATCH] correct handling of non-constant width and precision (pr 78521)

[Martin Sebor]

On 12/02/2016 01:31 AM, Rainer Orth wrote:
> Hi Martin,
>
>> PR 78521 notes that the gimple-ssa-sprintf pass doesn't do the right
>> thing (i.e., the -Wformat-length and -fprintf-return-value options
>> behave incorrectly) when a conversion specification includes a width
>> or precision with a non-constant value.  The code treats such cases
>> as if they were not provided which is incorrect and results in
>> the wrong bytes counts in warning messages and in the wrong ranges
>> being generated for such calls (or in the case sprintf(0, 0, ...)
>> for some such calls being eliminated).
>>
>> The attached patch corrects the handling of these cases, plus a couple
>> of other edge cases in the same area: it adjusts the parser to accept
>> precision in the form of just a period with no asterisk or decimal
>> digits after it (this sets the precision to zero), and corrects the
>> handling of zero precision and zero argument in integer directives
>> to produce no bytes on output.
>>
>> Finally, the patch also tightens up the constraint on the upper bound
>> of bounded functions like snprintf to be INT_MAX.  The functions cannot
>> produce output in excess of INT_MAX + 1 bytes and some implementations
>> (e.g., Solaris) fail with EINVAL when the bound is INT_MAX or more.
>> This is the subject of PR 78520.
>
> this patch broke Solaris bootstrap:
>
> /vol/gcc/src/hg/trunk/local/gcc/gimple-ssa-sprintf.c: In function 'void {anonymous}::get_width_and_precision(const {anonymous}::conversion_spec&, long long int*, long long int*)':
> /vol/gcc/src/hg/trunk/local/gcc/gimple-ssa-sprintf.c:777:45: error: call of overloaded 'abs(long long int)' is ambiguous
>   width = abs (tree_to_shwi (spec.star_width));
>                                              ^
> /vol/gcc/src/hg/trunk/local/gcc/gimple-ssa-sprintf.c:777:45: note: candidates are:
> In file included from /usr/include/stdlib.h:12:0,
>                  from /vol/gcc/src/hg/trunk/local/gcc/system.h:258,
>                  from /vol/gcc/src/hg/trunk/local/gcc/gimple-ssa-sprintf.c:49:
> /usr/include/iso/stdlib_iso.h:205:16: note: long int std::abs(long int)
>   inline long   abs(long _l) { return labs(_l); }
>                 ^
> /usr/include/iso/stdlib_iso.h:160:12: note: int std::abs(int)
>  extern int abs(int);
>             ^
>
> The following patch fixed this for me, but I've no idea if it's right.
> It bootstrapped successfully on sparc-sun-solaris2.12,
> i386-pc-solaris2.12, and x86_64-pc-linux-gnu.

Thanks for the heads up!  I just looked at that code yesterday while
analyzing bug 78608, wondering if it was safe.  Now I know it isn't.
I think it might be best to simply hand code the expression instead
of taking a chance on abs.  Let me take care of it today along with 78608.

Martin