[gcc r11-11493] tree-ssa-strlen: Fix up handle_store [PR113603]

[Jakub Jelinek]

https://gcc.gnu.org/g:1b91d111dad699a0738d158b0eca7fd12ded6211

commit r11-11493-g1b91d111dad699a0738d158b0eca7fd12ded6211
Author: Jakub Jelinek <jakub@redhat.com>
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 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 &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 = 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 &g[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.
    
    2024-01-30  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/113603
            * tree-ssa-strlen.c (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)

Diff:
---
 gcc/testsuite/gcc.c-torture/compile/pr113603.c | 40 ++++++++++++++++++++++++++
 gcc/tree-ssa-strlen.c                          |  3 ++
 2 files changed, 43 insertions(+)

diff --git a/gcc/testsuite/gcc.c-torture/compile/pr113603.c b/gcc/testsuite/gcc.c-torture/compile/pr113603.c
new file mode 100644
index 000000000000..0d4e817fbefa
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr113603.c
@@ -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 = &g[1];
+  do
+    {
+      for (;;)
+	{
+	  if (c)
+	    break;
+	  return &a;
+	}
+      for (f = 0;; f++)
+	{
+	  for (b = 0; b < 2; b++)
+	    *c = j[b][f];
+	  if (e)
+	    d = k;
+	  *k = *d;
+	  if (*c)
+	    break;
+	  if (f)
+	    break;
+	}
+    }
+  while (f);
+  return 0;
+}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index cfffc220073a..d2fd248f63b6 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -4802,6 +4802,9 @@ handle_store (gimple_stmt_iterator *gsi, bool *zero_write,
 
   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