From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 9C2093851C22; Mon, 14 Jun 2021 07:35:54 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9C2093851C22 From: "rguenth at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug tree-optimization/101031] [12 Regression] wrong code at -O2 on x86_64-linux-gnu Date: Mon, 14 Jun 2021 07:35:54 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: tree-optimization X-Bugzilla-Version: 12.0 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: rguenth at gcc dot gnu.org X-Bugzilla-Status: ASSIGNED X-Bugzilla-Resolution: X-Bugzilla-Priority: P1 X-Bugzilla-Assigned-To: rguenth at gcc dot gnu.org X-Bugzilla-Target-Milestone: 12.0 X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: gcc-bugs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-bugs mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Jun 2021 07:35:54 -0000 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D101031 --- Comment #4 from Richard Biener --- Interestingly the bogus store removal happens in the strlen pass. > diff -u b/a-t3.c.190t.dom3 b/a-t3.c.191t.strlen1 --- b/a-t3.c.190t.dom3 2021-06-14 08:59:33.483709029 +0200 +++ b/a-t3.c.191t.strlen1 2021-06-14 08:59:33.487709082 +0200 @@ -78,7 +80,6 @@ _25 =3D (long int) _24; d =3D _25; *c.0_14 =3D 1; - b =3D 0; f (); return; in #0 gsi_remove (i=3D0x7fffffffd830, remove_permanently=3Dtrue) at /home/rguenther/src/gcc2/gcc/gimple-iterator.c:558 #1 0x0000000001619e63 in handle_store (gsi=3D0x7fffffffd830,=20 zero_write=3D0x7fffffffd7df, ptr_qry=3D...) at /home/rguenther/src/gcc2/gcc/tree-ssa-strlen.c:4852 #2 0x000000000161bb9a in check_and_optimize_stmt (gsi=3D0x7fffffffd830,=20 cleanup_eh=3D0x7fffffffd89f, ptr_qry=3D...) at /home/rguenther/src/gcc2/gcc/tree-ssa-strlen.c:5439 #3 0x000000000161c30c in strlen_dom_walker::before_dom_children ( this=3D0x7fffffffd970, bb=3D) at /home/rguenther/src/gcc2/gcc/tree-ssa-strlen.c:5635 #4 0x000000000228e9b6 in dom_walker::walk (this=3D0x7fffffffd970,=20 bb=3D) at /home/rguenther/src/gcc2/gcc/domwalk.c:309 the *c stores have string info index 1 and the 'b' store have index 2. Interestingly the first *c store is handled by handle_store but the second is not and we return false from check_and_optimize_stmt. In particular we run into if (store_before_nul[1] > 0 && storing_nonzero_p && lenrange[0] =3D=3D lenrange[1] && lenrange[0] =3D=3D lenrange[2] && TREE_CODE (TREE_TYPE (rhs)) =3D=3D INTEGER_TYPE) { /* Handle a store of one or more non-nul characters that ends before the terminating nul of the destination and so does not affect its length If si->nonzero_chars > OFFSET, we aren't overwriting '\0', and if we aren't storing '\0', we know that the length of the string and any other zero terminated string in memory remains the same. In that case we move to the next gimple statement and return to signal the caller that it shouldn't invalidate anything. ... gsi_next (gsi); return false; } and skip the invalidation of 'b'. Now, we're not invalidating the first *c store at the point we stroe to b either because maybe_invalidate has ao_ref r; tree size =3D NULL_TREE; if (si->nonzero_chars) { /* Include the terminating nul in the size of the string to consider when determining possible clobber. */ tree type =3D TREE_TYPE (si->nonzero_chars); size =3D fold_build2 (PLUS_EXPR, type, si->nonzero_chars, build_int_cst (type, 1)); } ao_ref_init_from_ptr_and_size (&r, si->ptr, size); if (stmt_may_clobber_ref_p_1 (stmt, &r)) and thus we ask the oracle whether b =3D 0; may clobber a store/load of *c of size 2 which it can't (an access of size two cannot refer to 'b'). Looking at compare_nonzero_chars the si->nonzero_chars check should likely be si->nonzero_chars && !integer_zerop (si->nonzero_chars) as well. Now, adding 1 is done for correctness AFAIU but then extending the access beyond the size of the object is invalid. So IMHO we have to adjust max_size to be 1 bigger than size instead. The following fixes the testcase: diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c index 423075b2bd1..6add8c99032 100644 --- a/gcc/tree-ssa-strlen.c +++ b/gcc/tree-ssa-strlen.c @@ -1284,16 +1284,19 @@ maybe_invalidate (gimple *stmt, bool zero_write =3D false) continue; ao_ref r; - tree size =3D NULL_TREE; - if (si->nonzero_chars) + tree size =3D si->nonzero_chars; + ao_ref_init_from_ptr_and_size (&r, si->ptr, size); + /* Include the terminating nul in the size of the string + to consider when determining possible clobber. But do not + add it to 'size' since we don't know whether it would + actually fit the allocated area. */ + if (known_size_p (r.size)) { - /* Include the terminating nul in the size of the string - to consider when determining possible clobber. */ - tree type =3D TREE_TYPE (si->nonzero_chars); - size =3D fold_build2 (PLUS_EXPR, type, si->nonzero_chars, - build_int_cst (type, 1)); + if (known_le (r.size, HOST_WIDE_INT_MAX - BITS_PER_UNIT)) + r.max_size +=3D BITS_PER_UNIT; + else + r.max_size =3D -1; } - ao_ref_init_from_ptr_and_size (&r, si->ptr, size); if (stmt_may_clobber_ref_p_1 (stmt, &r)) { if (dump_file && (dump_flags & TDF_DETAILS))=