This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 08/15/18 18:25, Jeff Law wrote:
> On 08/14/2018 04:25 AM, Bernd Edlinger wrote:
>> Hi,
>>
>> this patch is a follow-up to my patch here:
>> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01800.html
>>
>> Since most calls of c_strlen and get_range_strlen expect
>> a string length in bytes of a zero-terminated string, there is
>> a need for a new parameter eltsize, which is per default 1,
>> but can be used in gimple-ssa-sprintf.c to specify the
>> expected character size.
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
>>
>> patch-fix-c-strlen.diff
>>
>>
>> 2018-08-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* builtins.c (c_strlen): Add new parameter eltsize.
>> 	* builtins.h (c_strlen): Adjust prototype.
>> 	* expr.c (string_constant): Add new parameter mem_size.
>> 	* 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.
>>
>> 2018-08-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* gcc.dg/strlenopt-49.c: Adjust test case.
>> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-18.c: Likewise.
> 
> Your patch appears to pass in an ELTSIZE which is used to determine how
> to count.  This has some advantages in that the routine can be used to
> count bytes or other elements such as 2 or 4 byte wide characters.
> 
> Your testsuite change appears to remove the xfails in strlenopt-49 which
> is usually OK.  So no concerns there.
> 
> The other testsuite change shows that the issued warning has changed.
> But we're still issuing a diagnostic the the construct, so it looks
> reasonable to me.
> 
> 
> This seems conceptually independent from the rest of the stuff we need
> to work through.  Though there definitely a potential for
> implementation/merge conflicts.
> 
> My concern is the sprintf changes -- for the wide char case we'll be
> getting a count of wide chars in the string.  I'm not offhand sure if
> the rest of the sprintf warning bits are prepared to count that way.
> 
> Martin?
> 

I think the %S issue is an independet thing, I have seen
a patch where %S sets the FMT_LEN_l, so I left that one alone.

What I have no solution for, is which wide char-type to assume.
I used 4, but that is only correct for some O/S. Others have 2.


Bernd.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]