Re: [PATCH] have -Wformat-overflow handle -fexec-charset (PR 80503)

[Martin Sebor <msebor@gmail.com>]

On 01/31/2018 10:36 AM, Renlin Li wrote:
> Hi there,
>
> I have a patch to fix to regressions we observed in armhf native
> environment.
>
> To effectively check out of range for format string, a target type
> should be
> used. And according to the standard, int type is used for "width" and
> "precision"
> field of format string.
>
> Here target_strtol10 is rename to target_strtoi10, and fixed to use
> target_int_max () which is target dependent.
>
> The value returned by target_strtol10 is (target_int_max () + 1) when it
> exceeds the range.
> This is used to indicate its value exceeds target INT_MAX for the later
> warning.

Sorry for not responding to your original email. It's still
in my inbox, just buried under a pile of stuff.

Using LONG_MAX is not ideal but unless I missed something
(it's been a while) I think using INT_MAX for the target would
be even less optimal.  Unless specified by the asterisk, width
and precision can be arbitrarily large. It seems to me that
constraining either to INT_MAX would trigger warnings on LP64
targets for safe calls like:

   // INT_MAX is 2147483647
   sprintf (d, "%.2147483648s", "");

I think we want to use the maximum value of an unsigned type
with the greatest precision on the host.  It will still warn
for directives with precisions in excess of HOST_WIDE_INT_MAX
but short of using something like offset_int it's probably as
good as it's going to get.  (It has been suggested that
the whole pass would benefit from using offset_int to track
large numbers so if/when it's enhanced to make this change
it should also make the code behave more consistently across
different hosts.)

Martin

>
> Is it Okay to commit?
>
> Regards,
> Renlin
>
> gcc/ChangeLog:
>
> 2018-01-31  Renlin Li  <renlin.li@arm.com>
>
>     * gimple-ssa-sprintf.c (target_strtol10): Use target integer to check
>     the range. Rename it into ....
>     (target_strtoi10): This.
>     (parse_directive): Compare with target int max instead of LONG_MAX.
>
>
> On 20/06/17 12:00, Renlin Li wrote:
>> Hi Martin,
>>
>> I did a little investigation into this. Please correct me if I missed
>> anything.
>>
>> I build a native arm-linux-gnueabihf toolchain in armhf hardware.
>> It's ILP32. So in this situation:
>>
>> HOST_WIDE_INT is long, which is 32-bit.
>> integer type 32-bit as well, so target_int_max () == LONG_MAX
>>
>>
>> gimple-ssa-sprintf.c line 2887
>>>   /* Has the likely and maximum directive output exceeded INT_MAX?  */
>>>   bool likelyximax = *dir.beg && res->range.likely > target_int_max ();
>>
>> likelyximax will be false as the latter expression is always false.
>> res->range.likely is truncated to LONG_MAX (in target_strtol10 function)
>>
>> I have checked in cross build environment (host x86_64), this variable
>> is true.
>>
>> Regards,
>> Renlin
>>
>>
>> On 13/06/17 09:16, Renlin Li wrote:
>>> Hi Martin,
>>>
>>> On 04/06/17 23:24, Martin Sebor wrote:
>>>> On 06/02/2017 09:38 AM, Renlin Li wrote:
>>>>> Hi Martin,
>>>>>
>>>>> After r247444, I saw the following two regressions in
>>>>> arm-linux-gnueabihf environment:
>>>>>
>>>>> FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c  (test for warnings,
>>>>> line 119)
>>>>> PASS: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c  (test for warnings,
>>>>> line 121)
>>>>> FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c  (test for warnings,
>>>>> line 121)
>>>>>
>>>>> The warning message related to those two lines are:
>>>>> testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c:119:3: warning:
>>>>> '%9223372036854775808i' directive width out of range
>>>>> [-Wformat-overflow=]
>>>>>
>>>>> testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c:121:3: warning:
>>>>> '%.9223372036854775808i' directive precision out of range
>>>>> [-Wformat-overflow=]
>>>>>
>>>>> testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c:121:3: warning:
>>>>> '%.9223372036854775808i' directive precision out of range
>>>>> [-Wformat-overflow=]
>>>>>
>>>>> Did you notice similar things from your test environment, Christophe?
>>>>
>>>> Looks like you're missing a couple of warnings.  I see the following
>>>> output with both my arm-linux-gnueabihf cross compiler and my native
>>>> x86_64 GCC, both in 32-bit and 64-bit modes, as expected by the test,
>>>> so I don't see the same issue in my environment.
>>>
>>> Yes, it happens on arm-linux-gnueabihf native environment. the
>>> warnings with "INT_MAX"
>>> line are missing. I don't know if the host environment will cause the
>>> difference.
>>>
>>> Regards,
>>> Renlin