Hi!
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 before
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 the
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 some
other strinfo was added) and finally we process a store in bb 6.
Now, count_nonzero_bytes is called and that sees [1] in a PHI and
calls get_stridx on it, which in turn calls get_stridx_plus_constant
because + 1 address doesn't have stridx yet. This creates a new
strinfo for it:
si = 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 becomes
3).
Now, get_stridx_plus_constant needs to chain the new strinfo of [1] in
between the related strinfos, so after the g record. Because the strinfo
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 copy
of the old one, decrementing refcount of the old shared one and setting
refcount of the new one to 1):
if (strinfo *nextsi = get_strinfo (chainsi->next))
{
nextsi = unshare_strinfo (nextsi);
si->next = nextsi->idx;
nextsi->prev = idx;
}
chainsi = unshare_strinfo (chainsi);
if (chainsi->first == 0)
chainsi->first = chainsi->idx;
chainsi->next = 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 = 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 pointer
to it ICEs.
The following patch fixes it by calling get_strinfo again, it is guaranteed
to return non-NULL, but could be an unshared copy instead of the originally
fetched shared one.
I believe we only need to do this refetching for the case where get_strinfo
is called on the lhs before get_stridx is called on other operands, because
we should be always modifying (apart from the chaining changes) the strinfo
for the destination of the statements, not other strinfos just consumed in
there.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
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.
--- gcc/tree-ssa-strlen.cc.jj 2024-01-29 10:20:25.0 +0100
+++ gcc/tree-ssa-strlen.cc 2024-01-29 15:50:17.056461933 +0100
@@ -5044,6 +5044,9 @@ strlen_pass::handle_store (bool *zero_wr
if (si != NULL)
{
+ /* The count_nonzero_bytes call above might have unshared si.
+Fetch it again from the vector. */
+ si = get_strinfo (idx);
/* The corresponding element is set to 1 if the first and last
element, respectively, of the sequence of characters being
written over the string described by SI ends before
--- gcc/testsuite/gcc.c-torture/compile/pr113603.c.jj 2024-01-29
16:09:54.335227319 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr113603.c 2024-01-29
16:09:36.010481829 +0100
@@ -0,0 +1,40 @@
+/* PR tree-optimization/113603 */
+
+int a, e;
+signed char b;
+int *c;
+signed char *d;
+short f;
+signed char g[3];
+
+int *
+foo (void)
+{
+ for (int i = 0; i < 3; i++)
+g[i] = 2;
+ int j[100][100] = { {}, {4} };
+ signed char *k = [1];
+
