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