On Tue, 28 Aug 2018, Jakub Jelinek wrote: > Hi! > > The following patch fixes some bugs in maybe_warn_nonstring_arg. > The testcase ICEs, because lenrng[1] is a PLUS_EXPR, but the code assumes > without checking that it must be INTEGER_CST and feeds it into > tree_int_cst_lt. If the upper bound is NULL or is some expression > other than INTEGER_CST, we can't do anything useful with it, so should > just treat it as unknown bound (but e.g. if the first argument doesn't > have a bound or has a complex expression as bound, but the second one > has INTEGER_CST upper bound, we can use that). > Additionally, the code doesn't ever care about lenrng[0] (lower bound), > only lenrng[1] (upper bound), so I've changed the code to check that that > bound is non-NULL and INTEGER_CST. > Because nothing uses lenrng[0], it is wasted work to add 1 to it using > const_binop and the whole function is about -Wstringop-overflow{,=} warning, > so if !warn_stringop_overflow, there is no reason to do all of it and we can > bail out early. > I've also noticed that it can call get_range_strlen on the length argument > of strncmp/strncasecmp, that doesn't make sense either. > And last, just a formatting fix to put constants on rhs of comparison. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK. Thanks, Richard. > 2018-08-27 Jakub Jelinek <ja...@redhat.com> > > PR middle-end/87099 > * calls.c (maybe_warn_nonstring_arg): Punt early if > warn_stringop_overflow is zero. Don't call get_range_strlen > on 3rd argument, keep iterating until lenrng[1] is INTEGER_CST. > Swap comparison operands to have constants on rhs. Only use > lenrng[1] if non-NULL and INTEGER_CST. Don't uselessly > increment lenrng[0]. > > * gcc.dg/pr87099.c: New test. > > --- gcc/calls.c.jj 2018-08-26 22:42:22.525779600 +0200 > +++ gcc/calls.c 2018-08-27 14:34:07.959235490 +0200 > @@ -1545,7 +1545,7 @@ maybe_warn_nonstring_arg (tree fndecl, t > if (!fndecl || DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL) > return; > > - if (TREE_NO_WARNING (exp)) > + if (TREE_NO_WARNING (exp) || !warn_stringop_overflow) > return; > > unsigned nargs = call_expr_nargs (exp); > @@ -1573,7 +1573,9 @@ maybe_warn_nonstring_arg (tree fndecl, t > the range of their known or possible lengths and use it > conservatively as the bound for the unbounded function, > and to adjust the range of the bound of the bounded ones. */ > - for (unsigned argno = 0; argno < nargs && !*lenrng; argno ++) > + for (unsigned argno = 0; > + argno < MIN (nargs, 2) > + && !(lenrng[1] && TREE_CODE (lenrng[1]) == INTEGER_CST); argno++) > { > tree arg = CALL_EXPR_ARG (exp, argno); > if (!get_attr_nonstring_decl (arg)) > @@ -1585,12 +1587,12 @@ maybe_warn_nonstring_arg (tree fndecl, t > case BUILT_IN_STRNCAT: > case BUILT_IN_STPNCPY: > case BUILT_IN_STRNCPY: > - if (2 < nargs) > + if (nargs > 2) > bound = CALL_EXPR_ARG (exp, 2); > break; > > case BUILT_IN_STRNDUP: > - if (1 < nargs) > + if (nargs > 1) > bound = CALL_EXPR_ARG (exp, 1); > break; > > @@ -1600,7 +1602,7 @@ maybe_warn_nonstring_arg (tree fndecl, t > if (!get_attr_nonstring_decl (arg)) > get_range_strlen (arg, lenrng); > > - if (1 < nargs) > + if (nargs > 1) > bound = CALL_EXPR_ARG (exp, 1); > break; > } > @@ -1640,11 +1642,9 @@ maybe_warn_nonstring_arg (tree fndecl, t > } > } > > - if (*lenrng) > + if (lenrng[1] && TREE_CODE (lenrng[1]) == INTEGER_CST) > { > /* Add one for the nul. */ > - lenrng[0] = const_binop (PLUS_EXPR, TREE_TYPE (lenrng[0]), > - lenrng[0], size_one_node); > lenrng[1] = const_binop (PLUS_EXPR, TREE_TYPE (lenrng[1]), > lenrng[1], size_one_node); > > --- gcc/testsuite/gcc.dg/pr87099.c.jj 2018-08-27 14:36:30.220856712 +0200 > +++ gcc/testsuite/gcc.dg/pr87099.c 2018-08-27 14:36:06.475253767 +0200 > @@ -0,0 +1,21 @@ > +/* PR middle-end/87099 */ > +/* { dg-do compile } */ > +/* { dg-options "-Wstringop-overflow" } */ > + > +void bar (char *); > + > +int > +foo (int n) > +{ > + char v[n]; > + bar (v); > + return __builtin_strncmp (&v[1], "aaa", 3); > +} > + > +int > +baz (int n, char *s) > +{ > + char v[n]; > + bar (v); > + return __builtin_strncmp (&v[1], s, 3); > +} > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)