Re: [PATCH] avoid assuming known string length is constant (PR 83896)

[Martin Sebor]

On 01/22/2018 09:33 AM, Jakub Jelinek wrote:

On Tue, Jan 16, 2018 at 03:20:24PM -0700, Martin Sebor wrote:

gcc/ChangeLog:

        PR tree-optimization/83896
        * tree-ssa-strlen.c (get_string_len): Rename...
        (get_string_cst_length): ...to this.  Return HOST_WIDE_INT.
        Avoid assuming length is constant.
        (handle_char_store): Use HOST_WIDE_INT for string length.

gcc/testsuite/ChangeLog:

        PR tree-optimization/83896
        * gcc.dg/strlenopt-43.c: New test.

...

   if (TREE_CODE (rhs) == MEM_REF
       && integer_zerop (TREE_OPERAND (rhs, 1)))

For the future, there is no reason it couldn't handle
also non-zero offsets if the to be returned length is bigger or
equal than that offset, where the offset can be then subtracted.
But let's defer that for GCC9.

I opened bug 84069 for this oversight.

@@ -2789,7 +2793,8 @@ handle_pointer_plus (gimple_stmt_iterator *gsi)
              if (idx > 0)
                {
                  strinfo *si = get_strinfo (idx);
-                 if (si && si->full_string_p)
+                 if (si && si->full_string_p
+                     && tree_fits_shwi_p (si->nonzero_chars))

If a && or || using condition doesn't fit onto a single line, it should be
split into one condition per line, so please change the above into:
                  if (si
                      && si->full_string_p
                      && tree_fits_shwi_p (si->nonzero_chars))

@@ -2822,7 +2824,7 @@ handle_char_store (gimple_stmt_iterator *gsi)
   unsigned HOST_WIDE_INT offset = 0;

   /* Set to the length of the string being assigned if known.  */
-  int rhslen;
+  HOST_WIDE_INT rhslen;

Please remove the empty line above the above comment, the function
starts with definitions of multiple variables, rhslen isn't even
initialized and there is nothing special on it compared to others,
so they should be in one block.

Ok for trunk with those changes.

I committed the patch with the tweaks suggested above and below
in 257100.

Martin

--- gcc/testsuite/gcc.dg/strlenopt-43.c (nonexistent)
+++ gcc/testsuite/gcc.dg/strlenopt-43.c (working copy)
@@ -0,0 +1,13 @@
+/* PR tree-optimization/83896 - ice in get_string_len on a call to strlen
+   with non-constant length
+   { dg-do compile }
+   { dg-options "-O2 -Wall -fdump-tree-optimized" } */
+
+extern char a[5];
+extern char b[];
+
+void f (void)
+{
+  if (__builtin_strlen (b) != 4)
+    __builtin_memcpy (a, b, sizeof a);
+}

Most of the strlenopt*.c tests use the strlenopt.h header and then
use the non-__builtin_ function names, if you want to change that too,
please do so, but not a requirement.

        Jakub