From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 4DB01385840F; Sat, 2 Mar 2024 00:38:35 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4DB01385840F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1709339915; bh=ki0h6N+7X86eNOZrWgf8fkhFeW16FqfEubV3hZYKN7w=; h=From:To:Subject:Date:In-Reply-To:References:From; b=dbFflXeIXTIYGYmOTmXicZE/7fULSJzrwnv8wt/6UqDb8w4OGsg+BuvADnO5lphLD ILaLwdGQyV3jrrrd9QlWRWZTSkXALyMEo03q2RuzQqtqIVCBVr+7AEBM/cpNkqgpjj 3QNqGJmwu4wgb8z7gUbZUw+bMC6M9JANAZ12yJ0s= From: "cvs-commit at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug tree-optimization/113603] [12/13 Regression] ICE Segfault during GIMPLE pass: strlen at -O3 since r12-145 Date: Sat, 02 Mar 2024 00:38:32 +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: 14.0 X-Bugzilla-Keywords: ice-on-valid-code X-Bugzilla-Severity: normal X-Bugzilla-Who: cvs-commit at gcc dot gnu.org X-Bugzilla-Status: ASSIGNED X-Bugzilla-Resolution: X-Bugzilla-Priority: P2 X-Bugzilla-Assigned-To: jakub at gcc dot gnu.org X-Bugzilla-Target-Milestone: 12.4 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 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D113603 --- Comment #5 from GCC Commits --- The releases/gcc-13 branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:7bc85af4c48b555da4447405f1319b35ca769d7b commit r13-8386-g7bc85af4c48b555da4447405f1319b35ca769d7b Author: Jakub Jelinek Date: Tue Jan 30 09:58:05 2024 +0100 tree-ssa-strlen: Fix up handle_store [PR113603] Since r10-2101-gb631bdb3c16e85f35d3 handle_store uses count_nonzero_bytes{,_addr} which (more recently limited to statements with the same vuse) can walk earlier statements feeding the rhs of the store and call get_stridx on it. Unlike most of the other functions where get_stridx is called first on rhs and only later on lhs, handle_store calls get_stridx on the lhs bef= ore the count_nonzero_bytes* call and does some si->nonzero_bytes comparison on it. Now, strinfo structures are refcounted and it is important not to screw it up. What happens on the following testcase is that we call get_strinfo on t= he destination idx's base (g), which returns a strinfo at that moment with refcount of 2, one copy referenced in bb 2 final strinfos, one in = bb 3 (the vector of strinfos was unshared from the dominator there because s= ome other strinfo was added) and finally we process a store in bb 6. Now, count_nonzero_bytes is called and that sees &g[1] in a PHI and calls get_stridx on it, which in turn calls get_stridx_plus_constant because &g + 1 address doesn't have stridx yet. This creates a new strinfo for it: si =3D new_strinfo (ptr, idx, build_int_cst (size_type_node, nonzero_chars), basesi->full_string_p); set_strinfo (idx, si); and the latter call, because it is the first one in bb 6 that needs it, unshares the stridx_to_strinfo vector (so refcount of the g strinfo bec= omes 3). Now, get_stridx_plus_constant needs to chain the new strinfo of &g[1] in between the related strinfos, so after the g record. Because the strin= fo is now shared between the current bb and 2 other bbs, it needs to unshare_strinfo it (creating a new strinfo which can be modified as a c= opy of the old one, decrementing refcount of the old shared one and setting refcount of the new one to 1): if (strinfo *nextsi =3D get_strinfo (chainsi->next)) { nextsi =3D unshare_strinfo (nextsi); si->next =3D nextsi->idx; nextsi->prev =3D idx; } chainsi =3D unshare_strinfo (chainsi); if (chainsi->first =3D=3D 0) chainsi->first =3D chainsi->idx; chainsi->next =3D idx; Now, the bug is that the caller of this a couple of frames above, handle_store, holds on a pointer to this g strinfo (but doesn't know about the unsharing, so the pointer is to the old strinfo with refcount of 2), and later needs to update it, so it si =3D unshare_strinfo (si); and modifies some fields in it. This creates a new strinfo (with refcount of 1 which is stored into the vector of the current bb) based on the old strinfo for g and decrements refcount of the old one to 1. So, now we are in inconsistent state, because the old strinfo for g is referenced in bb 2 and bb 3 vectors, but has just refcount of 1, and then have one strinfo (the one created by unshare_strinfo (chainsi) in get_stridx_plus_constant) which has refcount of 1 but isn't referenced from anywhere anymore. Later on when we free one of the bb 2 or bb 3 vectors (forgot which) that decrements refcount from 1 to 0 and poisons the strinfo/returns it= to the pool, but then maybe_invalidate when looking at the other bb's poin= ter to it ICEs. The following patch fixes it by calling get_strinfo again, it is guaran= teed to return non-NULL, but could be an unshared copy instead of the origin= ally fetched shared one. I believe we only need to do this refetching for the case where get_str= info is called on the lhs before get_stridx is called on other operands, bec= ause we should be always modifying (apart from the chaining changes) the str= info for the destination of the statements, not other strinfos just consumed= in there. 2024-01-30 Jakub Jelinek PR tree-optimization/113603 * tree-ssa-strlen.cc (strlen_pass::handle_store): After count_nonzero_bytes call refetch si using get_strinfo in case it has been unshared in the meantime. * gcc.c-torture/compile/pr113603.c: New test. (cherry picked from commit d7250c1e02478586a0cd6d5cb67bf4d17249a7e7)=