On 08/23/18 01:36, Jeff Law wrote:
> On 08/22/2018 05:22 PM, Bernd Edlinger wrote:
>> On 08/23/18 00:50, Jeff Law wrote:
>>> On 08/22/2018 02:14 AM, Richard Biener wrote:
>>>> On Wed, 22 Aug 2018, Bernd Edlinger wrote:
>>>>
>>>>> On 08/22/18 09:26, Richard Biener wrote:
>>>>>> On Wed, 22 Aug 2018, Bernd Edlinger wrote:
>>>>>>
>>>>>>> On 08/21/18 10:59, 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.
>>>>>>>>
>>>>>>>
>>>>>>> and in this case:
>>>>>>>
>>>>>>> const union
>>>>>>> { struct {
>>>>>>> wchar_t x[4];
>>>>>>> };
>>>>>>> struct {
>>>>>>> char z[8];
>>>>>>> };
>>>>>>> } u = {{L"123"}};
>>>>>>>
>>>>>>> int test()
>>>>>>> {
>>>>>>> return __builtin_strlen(u.z);
>>>>>>> }
>>>>>>>
>>>>>>>
>>>>>>> string_constant works out the initializer for u.x
>>>>>>> which has a different type than u.z
>>>>>>
>>>>>> Yes. That's because it uses ctor-for-folding and friends. It's
>>>>>> a question of the desired semantics of string_constant whether
>>>>>> it should better return NULL_TREE in this case or whether the
>>>>>> caller has to deal with type mismatches.
>>>>>>
>>>>>
>>>>> Yes, absolutely.
>>>>>
>>>>> c_getstr needs to bail out if the string is not zero-terminated
>>>>> within the limits given by the decl, or the string_cst-type or whatever
>>>>> may help.
>>>>>
>>>>> Furthermore I also consider it possible that the byteoffset
>>>>> is not a multiple of eltsize. So fail in that case as well.
>>>>>
>>>>>
>>>>> I am currently boot-strapping a patch for this (pr87053):
>>>>> $ cat u.c
>>>>> const union
>>>>> { struct {
>>>>> char x[4];
>>>>> char y[4];
>>>>> };
>>>>> struct {
>>>>> char z[8];
>>>>> };
>>>>> } u = {{"1234", "567"}};
>>>>>
>>>>> int test()
>>>>> {
>>>>> return __builtin_strlen(u.z);
>>>>> }
>>>>>
>>>>> gets folded to 4.
>>>>>
>>>>> ... but unfortunately it will depend on my pr86714 fix which fixes
>>>>> the mem_size parameter returned from string_constant.
>>>>>
>>>>> Frankly, in the moment I feel like I fell in a deep deep hole. :-O
>>>>
>>>> /me too with > 100 mails in this and related threads unread ;)
>>>>
>>>> I thought Jeff applied the mem_size parameter thing but maybe it
>>>> was something else. *fetches coffee* Ah, Jeff is still "working"
>>>> on it.
>>> The patch that was applied adds a parameter to c_strlen to indicate how
>>> it should count (ie, bytes, 16bit wchars or 32bit wchars).
>>>
>>> There was one hunk that was dependent upon an earlier, more
>>> controversial, patch from Bernd which is still under discussion. That
>>> hunk was twiddled to preserve the overall goal of Bernd's patch which
>>> added the how to count parameter without being dependent upon the older,
>>> more controversial patch.
>>>
>>
>> Which patch do you mean?
> This one:
>
> Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date: Thu Aug 16 22:38:04 2018 +0000
>
> * builtins.c (c_strlen): Add new parameter eltsize. Use it
> for determining how to count the elements.
> * builtins.h (c_strlen): Adjust prototype.
> * expr.c (string_constant): Add new parameter mem_size.
> Set *mem_size appropriately.
> * expr.h (string_constant): Adjust protoype.
> * gimple-fold.c (get_range_strlen): Add new parameter eltsize.
> * gimple-fold.h (get_range_strlen): Adjust prototype.
> * gimple-ssa-sprintf.c (get_string_length): Add new
> parameter eltsize.
> (format_string): Call get_string_length with eltsize.
>
> git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@263607
> 138bc75d-0d04-0410-961f-82ee72b054a4
>
Yes, and which one was the earlier, more controversial patch from me?
Thanks
Bernd.
