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.edlin...@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.edlin...@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?

Jeff



Reply via email to