expand: Don't depend on warning flags in code generation of strnlen [PR94189]

Hi!

The following testcase FAILs with -O2 -fcompare-debug, but the reason isn't
that we'd emit different code based on -g or non-debug, but rather that
we emit different code depending on whether -w is used or not (or e.g.
-Wno-stringop-overflow or whether some other pass emitted some other warning
already on the call).

Code generation shouldn't depend on whether we emit a warning or not if at
all possible.

The following patch punts (i.e. doesn't optimize the strnlen call to a
constant value) if we would emit the warning if it was enabled.
In the PR there is an alternate patch which does optimize the strnlen call
no matter if we emit the warning or not, though I think I prefer the version
below, e.g. the strnlen call might be crossing field boundaries, which is in
strict reading undefined, but I'd be afraid people do that in the real
world programs.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2020-03-17  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/94189
	* builtins.c (expand_builtin_strnlen): Do return NULL_RTX if we would
	emit a warning if it was enabled and don't depend on TREE_NO_WARNING
	for code-generation.

	* gcc.dg/pr94189.c: New test.


	Jakub

On Tue, Mar 17, 2020 at 9:35 AM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi!
>
> The following testcase FAILs with -O2 -fcompare-debug, but the reason isn't
> that we'd emit different code based on -g or non-debug, but rather that
> we emit different code depending on whether -w is used or not (or e.g.
> -Wno-stringop-overflow or whether some other pass emitted some other warning
> already on the call).
>
> Code generation shouldn't depend on whether we emit a warning or not if at
> all possible.
>
> The following patch punts (i.e. doesn't optimize the strnlen call to a
> constant value) if we would emit the warning if it was enabled.
> In the PR there is an alternate patch which does optimize the strnlen call
> no matter if we emit the warning or not, though I think I prefer the version
> below, e.g. the strnlen call might be crossing field boundaries, which is in
> strict reading undefined, but I'd be afraid people do that in the real
> world programs.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2020-03-17  Jakub Jelinek  <jakub@redhat.com>
>
>         PR middle-end/94189
>         * builtins.c (expand_builtin_strnlen): Do return NULL_RTX if we would
>         emit a warning if it was enabled and don't depend on TREE_NO_WARNING
>         for code-generation.
>
>         * gcc.dg/pr94189.c: New test.
>
> --- gcc/builtins.c.jj   2020-01-12 11:54:36.198416652 +0100
> +++ gcc/builtins.c      2020-03-16 11:39:02.400537378 +0100
> @@ -3142,27 +3142,25 @@ expand_builtin_strnlen (tree exp, rtx ta
>             return NULL_RTX;
>         }
>
> -      if (lendata.decl
> -         && !TREE_NO_WARNING (exp)
> -         && ((tree_int_cst_lt (len, bound))
> -             || !exact))
> +      if (lendata.decl && (tree_int_cst_lt (len, bound) || !exact))
>         {
>           location_t warnloc
>             = expansion_point_location_if_in_system_header (loc);
>
> -         if (warning_at (warnloc, OPT_Wstringop_overflow_,
> -                         exact
> -                         ? G_("%K%qD specified bound %E exceeds the size %E "
> -                              "of unterminated array")
> -                         : G_("%K%qD specified bound %E may exceed the size "
> -                              "of at most %E of unterminated array"),
> -                         exp, func, bound, len))
> +         if (!TREE_NO_WARNING (exp)
> +             && warning_at (warnloc, OPT_Wstringop_overflow_,
> +                            exact
> +                            ? G_("%K%qD specified bound %E exceeds the size "
> +                                 "%E of unterminated array")
> +                            : G_("%K%qD specified bound %E may exceed the "
> +                                 "size of at most %E of unterminated array"),
> +                            exp, func, bound, len))
>             {
>               inform (DECL_SOURCE_LOCATION (lendata.decl),
>                       "referenced argument declared here");
>               TREE_NO_WARNING (exp) = true;
> -             return NULL_RTX;
>             }
> +         return NULL_RTX;
>         }
>
>        if (!len)
> --- gcc/testsuite/gcc.dg/pr94189.c.jj   2020-03-16 11:32:17.785541399 +0100
> +++ gcc/testsuite/gcc.dg/pr94189.c      2020-03-16 11:31:43.257053763 +0100
> @@ -0,0 +1,11 @@
> +/* PR middle-end/94189 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fcompare-debug" } */
> +
> +const char a[] = { 'a', 'b', 'c', 'd' };/* { dg-message "declared here" } */
> +
> +int
> +foo (void)
> +{
> +  return __builtin_strnlen (a, 5);     /* { dg-warning "specified bound 5 exceeds the size 4 of unterminated array" } */
> +}
>
>         Jakub
>