[PATCH] suppress unhelpful -Wformat-truncation=2 INT_MAX warning (PR 79448)

[Martin Sebor]

On 02/13/2017 04:33 PM, Jeff Law wrote:
> On 02/10/2017 10:55 AM, Martin Sebor wrote:
>> The recent Fedora mass rebuild revealed that the Wformat-truncation=2
>> checker is still a bit too aggressive and complains about potentially
>> unbounded strings causing subsequent directives t exceed the INT_MAX
>> limit.  (It's unclear how the build ended up enabling level 2 of
>> the warning.)
>>
>> This is because for the purposes of the return value optimization
>> the pass must assume that such strings really are potentially unbounded
>> and result in as many as INT_MAX bytes (or more).  That doesn't mean
>> that it should warn on such cases.
>>
>> The attached patch relaxes the checker to avoid the warning in this
>> case.  Since there's no easy way for a user to suppress the warning,
>> is this change okay for trunk at this stage?
>>
>> Martin
>>
>> gcc-79448.diff
>>
>>
>> PR middle-end/79448 - unhelpful -Wformat-truncation=2 warning
>>
>> gcc/testsuite/ChangeLog:
>>
>>     PR middle-end/79448
>>     * gcc.dg/tree-ssa/builtin-snprintf-warn-3.c: New test.
>>     * gcc.dg/tree-ssa/pr79448-2.c: New test.
>>     * gcc.dg/tree-ssa/pr79448.c: New test.
>>
>> gcc/ChangeLog:
>>
>>     PR middle-end/79448
>>     * gimple-ssa-sprintf.c (format_directive): Avoid issuing INT_MAX
>>       warning for strings of unknown length.
>>
>> diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
>> index e6cc31d..bf76162 100644
>> --- a/gcc/gimple-ssa-sprintf.c
>> +++ b/gcc/gimple-ssa-sprintf.c
>> @@ -2561,11 +2561,15 @@ format_directive (const
>> pass_sprintf_length::call_info &info,
>>    /* Raise the total unlikely maximum by the larger of the maximum
>>       and the unlikely maximum.  It doesn't matter if the unlikely
>>       maximum overflows.  */
>> +  unsigned HOST_WIDE_INT save = res->range.unlikely;
>>    if (fmtres.range.max < fmtres.range.unlikely)
>>      res->range.unlikely += fmtres.range.unlikely;
>>    else
>>      res->range.unlikely += fmtres.range.max;
>>
>> +  if (res->range.unlikely < save)
>> +    res->range.unlikely = HOST_WIDE_INT_M1U;
>> +
> So this looks like you're doing an overflow check -- yet earlier your
> comment says "It doesnt' matter if the unlikely maximum overflows". ISTM
> that comment needs updating -- if it doesn't matter, then why check for
> it and clamp the value?
>
>
>>    res->range.min += fmtres.range.min;
>>    res->range.likely += fmtres.range.likely;
>>
>> @@ -2616,7 +2620,12 @@ format_directive (const
>> pass_sprintf_length::call_info &info,
>>
>>    /* Has the likely and maximum directive output exceeded INT_MAX?  */
>>    bool likelyximax = *dir.beg && res->range.likely > target_int_max ();
>> -  bool maxximax = *dir.beg && res->range.max > target_int_max ();
>> +  /* Don't consider the maximum to be in excess when it's the result
>> +     of a string of unknown length (i.e., whose maximum has been set
>> +     to HOST_WIDE_INT_M1U.  */
>> +  bool maxximax = (*dir.beg
>> +           && res->range.max > target_int_max ()
>> +           && res->range.max < HOST_WIDE_INT_MAX);
> So your comment mentions HOST_WIDE_INT_M1U as the key for a string of
> unknown length.  But that doesn't obviously correspond to what the code
> checks.
>
> Can you please fix up the two comments.  With the comments fixed, this
> is OK.

Sure, I updated the comments.

The code alternately uses HOST_WIDE_INT_M1U and HOST_WIDE_INT_MAX as
a stand-in for either a "can't happen" or "unbounded/unknown" size.
It's not fully consistent and should be cleaned up and the uses of
HOST_WIDE_INT should be replaced by a class like wide_int as someone
suggested in the past.  If I get to some of the enhancements I'd like
to make in stage 1 (e.g., integrating the pass with tree-ssa-strlen)
I'll see about cleaning this up.

Martin