Re: [PATCH] Add a character size parameter to c_strlen/get_range_strlen

[Richard Biener <rguenther at suse dot de>]

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.

Richard.