On 08/26/18 07:44, Jeff Law wrote:
> On 08/21/2018 10:35 AM, Martin Sebor wrote:
>> On 08/21/2018 09:59 AM, Jeff Law wrote:
>>> On 08/21/2018 09:57 AM, Martin Sebor wrote:
>>>> On 08/21/2018 02:59 AM, Richard Biener wrote:
>>>>> On Tue, 21 Aug 2018, Bernd Edlinger wrote:
>>>>>
>>>>>> gcc -S -O2 -Wall -Wformat-overflow -ftrack-macro-expansion=0
>>>>>> -fshort-wchar builtin-sprintf-warn-20.c
>>>>>> builtin-sprintf-warn-20.c: In function 'test':
>>>>>> builtin-sprintf-warn-20.c:19:39: warning: hex escape sequence out of
>>>>>> range
>>>>>> 19 | ? (char*)L"\x4142\x4344" : (char*)L"\x41424344\x45464748";
>>>>>> | ^~~~~~~~~~~~~~~~~~~~~~~
>>>>>>
>>>>>> Hmm, this test might create some noise on short-wchar targets.
>>>>>>
>>>>>> I would prefer a warning here, about the wrong type of the parameter.
>>>>>> The buffer overflow is only a secondary thing.
>>>>>>
>>>>>> For constant objects like those, the GIMPLE type is still guaranteed
>>>>>> to be reliable,
>>>>>> right?
>>>>>
>>>>> TREE_TYPE of tcc_declaration and tcc_constant trees should more-or-less
>>>>> (minus qualifications not affecting semantics) be those set by
>>>>> frontends.
>>>>
>>>> A warning for these cases should be relatively straightforward
>>>> to add to the sprintf pass. It would require c_strlen() to return
>>>> the type of the string argument to the caller. That way sprintf's
>>>> format_string() function could compare the string type to the
>>>> expected type of the directive.
>>> Umm, why would c_strlen need to return that? We should be able to get
>>> to it directly from the argument?!? What am I missing here?
>>
>> The sprintf pass sees the type of the argument. In the literal
>> case above the cast is folded away and so sprintf does see
>> the type of the argument. In more interesting cases like
>> the one below it sees the type of the argument (i.e., wchar_t*)
>> but c_strlen() has the smarts to get at the underlying object:
>>
>> const char a[] = "1234";
>>
>> int f (int i)
>> {
>> __WCHAR_TYPE__ *p = a;
>>
>> return __builtin_snprintf (0, 0, "%ls", &p[i]);
> Presumably it'd have to be bubbled up from string_constant, right?
>
I think for the warning, the type may also be interesting if a
CONSTRUCTOR is not constant.
Bernd.
