Bug 106900 - Regression after memchr optimization
Summary: Regression after memchr optimization
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 13.0
: P3 normal
Target Milestone: 14.0
Assignee: Andrew Pinski
URL: https://gcc.gnu.org/pipermail/gcc-pat...
Keywords: diagnostic, patch
Depends on:
Blocks: Warray-bounds
  Show dependency treegraph
 
Reported: 2022-09-10 05:36 UTC by Jan-Benedict Glaw
Modified: 2023-05-18 15:47 UTC (History)
1 user (show)

See Also:
Host:
Target: avr-elf, pru-elf, rl78-elf
Build:
Known to work:
Known to fail:
Last reconfirmed: 2023-05-16 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jan-Benedict Glaw 2022-09-10 05:36:13 UTC
Hi!

The optimization prepared in PR103798 ("memchr with a (small) constant string should be expanded inline", c6cf555a88f8ffb8ec85c2b94fe656ab217260ea) caused a regression for avr-elf, pru-elf and rl78-elf:

.../gcc/configure --prefix=... --enable-werror-always --enable-languages=all --disable-gcov --disable-shared --disable-threads --target=pru-elf 
--without-headers                                                                                                                                 
[...]                                                                                                                                           
make V=1 all-gcc                                                                                                                                
[...]                                                                                                                                           
/usr/lib/gcc-snapshot/bin/g++  -fno-PIE -c   -g -O2   -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -W
cast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libcody  -I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/gcc/../libbacktrace   -o tree-ssa-forwprop.o -MT tree-ssa-forwprop.o -MMD -MP -MF ./.deps/tree-ssa-forwprop.TPo ../../gcc/gcc/tree-ssa-forwprop.cc
../../gcc/gcc/tree-ssa-forwprop.cc: In function 'bool simplify_builtin_call(gimple_stmt_iterator*, tree)':
../../gcc/gcc/tree-ssa-forwprop.cc:1258:42: error: array subscript 1 is outside array bounds of 'tree_node* [1]' [-Werror=array-bounds]
 1258 |             op[i - 1] = fold_convert_loc (loc, boolean_type_node,
      |                         ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
 1259 |                                           fold_build2_loc (loc,
      |                                           ~~~~~~~~~~~~~~~~~~~~~
 1260 |                                                            BIT_IOR_EXPR,
      |                                                            ~~~~~~~~~~~~~
 1261 |                                                            boolean_type_node,
      |                                                            ~~~~~~~~~~~~~~~~~~
 1262 |                                                            op[i - 1],
      |                                                            ~~~~~~~~~~
 1263 |                                                            op[i]));
      |                                                            ~~~~~~~
In file included from ../../gcc/gcc/system.h:707,
                 from ../../gcc/gcc/tree-ssa-forwprop.cc:21:
../../gcc/gcc/../include/libiberty.h:733:36: note: at offset 8 into object of size [0, 8] allocated by '__builtin_alloca'
  733 | # define alloca(x) __builtin_alloca(x)
      |                    ~~~~~~~~~~~~~~~~^~~
../../gcc/gcc/../include/libiberty.h:365:40: note: in expansion of macro 'alloca'
  365 | #define XALLOCAVEC(T, N)        ((T *) alloca (sizeof (T) * (N)))
      |                                        ^~~~~~
../../gcc/gcc/tree-ssa-forwprop.cc:1250:22: note: in expansion of macro 'XALLOCAVEC'
 1250 |           tree *op = XALLOCAVEC (tree, isize);
      |                      ^~~~~~~~~~
cc1plus: all warnings being treated as errors
make[1]: *** [Makefile:1146: tree-ssa-forwprop.o] Error 1


(Last confirmed at c2c3e4f6698925c8c969d8525677fbfe98f78909.)
Comment 1 Andrew Pinski 2022-09-10 05:47:21 UTC
Looks like gcc can't figure out that isize can't be 0 even if there was a check for integer_zerop (size) before hand.
There must be a way to add an assert there to allow gcc to figure that out.
Comment 2 Andrew Pinski 2022-09-10 05:48:09 UTC
Obviously this only happens because you use --enable-werror-always
Comment 3 Jan-Benedict Glaw 2023-02-25 19:33:21 UTC
Sure, just a warning, but still there (as of 31303c9b5bab200754cdb7ef8cd91ae4918f3018) and affecting three targets. Maybe we get that understood/fixed?
Comment 4 Andrew Pinski 2023-05-16 21:33:44 UTC
I am just going to test:
diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc
index 06f19868ade..0326e6733e8 100644
--- a/gcc/tree-ssa-forwprop.cc
+++ b/gcc/tree-ssa-forwprop.cc
@@ -1231,14 +1231,14 @@ simplify_builtin_call (gimple_stmt_iterator *gsi_p, tree callee2)
          tree size = gimple_call_arg (stmt2, 2);
          /* Size must be a constant which is <= UNITS_PER_WORD and
             <= the string length.  */
-         if (TREE_CODE (size) != INTEGER_CST || integer_zerop (size))
+         if (TREE_CODE (size) != INTEGER_CST)
            break;

          if (!tree_fits_uhwi_p (size))
            break;

          unsigned HOST_WIDE_INT sz = tree_to_uhwi (size);
-         if (sz > UNITS_PER_WORD || sz >= slen)
+         if (sz == 0 || sz > UNITS_PER_WORD || sz >= slen)
            break;

          tree ch = gimple_call_arg (stmt2, 1);


This does not change the behavior of the code at all, just makes it obvious sz cannot be 0 and therefore isize won't be 0 either.
Comment 5 Andrew Pinski 2023-05-17 04:24:05 UTC
Patch posted:
https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618762.html
Comment 6 GCC Commits 2023-05-17 15:03:22 UTC
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:f65af1eeef670f2c249b1896726ef57bbf65fe2f

commit r14-937-gf65af1eeef670f2c249b1896726ef57bbf65fe2f
Author: Andrew Pinski <apinski@marvell.com>
Date:   Tue May 16 14:34:05 2023 -0700

    Fix PR 106900: array-bounds warning inside simplify_builtin_call
    
    The problem here is that VRP cannot figure out isize could not be 0
    due to using integer_zerop. This patch removes the use of integer_zerop
    and instead checks for 0 directly after converting the tree to
    an unsigned HOST_WIDE_INT. This allows VRP to figure out isize is not 0
    and `isize - 1` will always be >= 0.
    
    This patch is just to avoid the warning that GCC could produce sometimes
    and does not change any code generation or even VRP.
    
    OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
    
    gcc/ChangeLog:
    
            * tree-ssa-forwprop.cc (simplify_builtin_call): Check
            against 0 instead of calling integer_zerop.
Comment 7 Andrew Pinski 2023-05-17 15:05:06 UTC
Fixed on the trunk; not really worth backporting since it is only an issue with --enable-werror-always which almost nobody uses.
Comment 8 Jan-Benedict Glaw 2023-05-17 18:40:06 UTC
Thanks a lot!  I scheduled builds for the three affected targets (from my target list.) The box is quite loaded right now (and a few jobs a before those three), so I guess it'll take a few hours.
Comment 9 Jan-Benedict Glaw 2023-05-18 15:47:02 UTC
All three target configurations reported a successful build. Thanks!