This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix some bugs in maybe_warn_nonstring_arg (PR middle-end/87099)
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Jeff Law <law at redhat dot com>, gcc-patches at gcc dot gnu dot org, Martin Sebor <msebor at gmail dot com>
- Date: Tue, 28 Aug 2018 11:41:24 +0200 (CEST)
- Subject: Re: [PATCH] Fix some bugs in maybe_warn_nonstring_arg (PR middle-end/87099)
- References: <20180828072340.GN2218@tucnak>
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)