在 2019/5/9 上午10:22, JunMa 写道:
在 2019/5/9 上午3:02, Bernd Edlinger 写道:
On 5/8/19 4:31 PM, Richard Biener wrote:
On Tue, May 7, 2019 at 4:34 AM JunMa <ju...@linux.alibaba.com> wrote:
在 2019/5/6 下午7:58, JunMa 写道:
在 2019/5/6 下午6:02, Richard Biener 写道:
On Thu, Mar 21, 2019 at 5:57 AM JunMa <ju...@linux.alibaba.com> wrote:
Hi
For now, gcc can not fold code like:

const char a[5] = "123"
__builtin_memchr (a, '7', sizeof a)

It tries to avoid folding out of string length although length of a
is 5.
This is a bit conservative, it's safe to folding memchr/bcmp/memcmp
builtins when constant string stores in array with some trailing nuls.

This patch folds these cases by exposing additional length of
trailing nuls in c_getstr().
Bootstrapped/regtested on x86_64-linux, ok for trunk?
It's probably better if gimple_fold_builtin_memchr uses string_constant
directly instead?
We can use string_constant in gimple_fold_builtin_memchr, however it is a
bit complex to use it in memchr/memcmp constant folding.
You are changing memcmp constant folding but only have a testcase
for memchr.
I'll add later.
If we decide to amend c_getstr I would rather make it return the
total length instead of the number of trailing zeros.
I think return the total length is safe in other place as well.
I used the argument in patch since I do not want any impact on
other part at all.

Although it is safe to use total length, I found that function
inline_expand_builtin_string_cmp() which used c_getstr() may emit
redundant rtls for trailing null chars when total length is returned.

Also it is trivial to handle constant string with trailing null chars.

So this updated patch follow richard's suggestion: using string_constant
directly.

Bootstrapped/regtested on x86_64-linux, ok for trunk?
Doesn't this fold to NULL for the case where seaching for '0' and it
doesn't occur in the string constant but only the zero-padding?
So you'd need to conditionalize on c being not zero (or handle
that case specially by actually finding the first zero in the padding)?
I think there was work to have all string constants zero terminated
but I'm not sure if we can rely on that here.  Bernd?

It turned out that there is a number of languages that don't have zero-terminated strings by default, which would have complicated the patch just too much for too
little benefit.

In the end, it was more important to guarantee that mem_size >= string_length holds.

The string_length returned form  c_getstr() is equal to mem_size when string_length > string_size, so I'll add assert

in the patch.

In C it is just a convention that string constants have usually a zero-termination, but even with C there are ways how strings constants can be not zero-terminated.

There can in theory be optimizations that introduce not zero-terminated strings, like tree-ssa-forwprop.c, where a not zero-terminated string constant is folded
in simplify_builtin_call.

In such a case, c_getstr might in theory return a string without zero-termination,
but I think it will be rather difficult to find a C test case for that.

Well, if I had a test case for that I would probably fix it in c_getstr to consider
the implicit padding as equivalent to an explicit zero-termination.


c_getstr() makes sure  the returned string has zero-termination when 2nd argument is NULL, but not when string_length is returned.  I think it is the caller's responsibility to take care of both of the returned string and length.



Update the patch with assert checking on condition
string_length <= string_size. FYI.

Also Bootstrapped/regtested on x86_64-linux.

Regards
JunMa


Thanks
JunMa



Bernd.


Richard.

Regards
JunMa

gcc/ChangeLog

2019-05-07  Jun Ma <ju...@linux.alibaba.com>

      PR Tree-optimization/89772
      * gimple-fold.c (gimple_fold_builtin_memchr): consider trailing nuls in
      out-of-bound accesses checking.

gcc/testsuite/ChangeLog

2019-05-07  Jun Ma <ju...@linux.alibaba.com>

      PR Tree-optimization/89772
      * gcc.dg/builtin-memchr-4.c: New test.
Thanks
JunMa
Richard.

Regards
JunMa


gcc/ChangeLog

2019-03-21  Jun Ma <ju...@linux.alibaba.com>

       PR Tree-optimization/89772
       * fold-const.c (c_getstr): Add new parameter to get length of
additional
       trailing nuls after constant string.
       * gimple-fold.c (gimple_fold_builtin_memchr): consider
trailing nuls in
       out-of-bound accesses checking.
       * fold-const-call.c (fold_const_call): Likewise.


gcc/testsuite/ChangeLog

2019-03-21  Jun Ma <ju...@linux.alibaba.com>

       PR Tree-optimization/89772
       * gcc.dg/builtin-memchr-4.c: New test.


---
 gcc/gimple-fold.c                       | 10 +++++++++-
 gcc/testsuite/gcc.dg/builtin-memchr-4.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/builtin-memchr-4.c

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 1b10bae..c4fd5b1 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -2557,7 +2557,15 @@ gimple_fold_builtin_memchr (gimple_stmt_iterator *gsi)
       const char *r = (const char *)memchr (p1, c, MIN (length, 
string_length));
       if (r == NULL)
        {
-         if (length <= string_length)
+         tree mem_size, offset_node;
+         string_constant (arg1, &offset_node, &mem_size, NULL);
+         unsigned HOST_WIDE_INT offset = (offset_node == NULL_TREE)
+                                         ? 0 : tree_to_uhwi (offset_node);
+         /* MEM_SIZE is the size of the array the string literal
+            is stored in.  */
+         unsigned HOST_WIDE_INT string_size = tree_to_uhwi (mem_size) - offset;
+         gcc_checking_assert (string_length <= string_size);
+         if (length <= string_size)
            {
              replace_call_with_value (gsi, build_int_cst (ptr_type_node, 0));
              return true;
diff --git a/gcc/testsuite/gcc.dg/builtin-memchr-4.c 
b/gcc/testsuite/gcc.dg/builtin-memchr-4.c
new file mode 100644
index 0000000..3ef424c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/builtin-memchr-4.c
@@ -0,0 +1,30 @@
+/* PR tree-optimization/89772
+   Verify that memchr calls with a pointer to a constant character
+   are folded as expected.
+   { dg-do compile }
+   { dg-options "-O1 -Wall -fdump-tree-lower" } */
+
+typedef __SIZE_TYPE__  size_t;
+typedef __WCHAR_TYPE__ wchar_t;
+
+extern void* memchr (const void*, int, size_t);
+extern int printf (const char*, ...);
+extern void abort (void);
+
+#define A(expr)                                                \
+  ((expr)                                              \
+   ? (void)0                                           \
+   : (printf ("assertion failed on line %i: %s\n",     \
+             __LINE__, #expr),                         \
+      abort ()))
+
+const char a[8] = {'a',0,'b'};
+
+void test_memchr_cst_char (void)
+{
+  A (!memchr (a, 'c', 2));
+  A (!memchr (a, 'c', 5));
+  A (!memchr (a, 'c', sizeof a));
+}
+
+/* { dg-final { scan-tree-dump-not "abort" "gimple" } } */
-- 
1.8.3.1

Reply via email to