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] Fix some bugs in maybe_warn_nonstring_arg (PR middle-end/87099)


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  <jakub@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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)


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