Re: [PATCH] Fix PR/63841: empty constructor doesn't zero-initialize

[Teresa Johnson <tejohnson at google dot com>]

Here is the new patch. Bootstrapped and tested on
x86_64-unknown-linux-gnu. OK for trunk?

Thanks,
Teresa

2014-11-13    <tejohnson@google.com>

gcc:
        PR tree-optimization/63841
        * tree.c (initializer_zerop): A constructor with no elements
        does not zero initialize.

gcc/testsuite:
        PR tree-optimization/63841
        * g++.dg/tree-ssa/pr63841.C: New test.

Index: tree.c
===================================================================
--- tree.c      (revision 217190)
+++ tree.c      (working copy)
@@ -10330,6 +10330,8 @@ initializer_zerop (const_tree init)
       {
        unsigned HOST_WIDE_INT idx;

+        if (TREE_CLOBBER_P (init))
+          return false;
        FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), idx, elt)
          if (!initializer_zerop (elt))
            return false;
Index: testsuite/g++.dg/tree-ssa/pr63841.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr63841.C (revision 0)
+++ testsuite/g++.dg/tree-ssa/pr63841.C (working copy)
@@ -0,0 +1,38 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include <cstdio>
+#include <string>
+
+std::string __attribute__ ((noinline)) comp_test_write() {
+  std::string data;
+
+  for (int i = 0; i < 2; ++i) {
+    char b = 1 >> (i * 8);
+    data.append(&b, 1);
+  }
+
+  return data;
+}
+
+std::string __attribute__ ((noinline)) comp_test_write_good() {
+  std::string data;
+
+  char b;
+  for (int i = 0; i < 2; ++i) {
+    b = 1 >> (i * 8);
+    data.append(&b, 1);
+  }
+
+  return data;
+}
+
+int main() {
+  std::string good = comp_test_write_good();
+  printf("expected: %hx\n", *(short*)good.c_str());
+
+  std::string bad = comp_test_write();
+  printf("got: %hx\n", *(short*)bad.c_str());
+
+  return good != bad;
+}

On Wed, Nov 12, 2014 at 9:40 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, Nov 12, 2014 at 9:38 PM, Teresa Johnson <tejohnson@google.com> wrote:
>> On Wed, Nov 12, 2014 at 9:30 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Wed, Nov 12, 2014 at 9:25 PM, Teresa Johnson <tejohnson@google.com> wrote:
>>>> Added testcase. Here is the new patch:
>>>>
>>>> 2014-11-12    <tejohnson@google.com>
>>>>
>>>> gcc:
>>>>         PR tree-optimization/63841
>>>>         * tree.c (initializer_zerop): A constructor with no elements
>>>>         does not zero initialize.
>>>
>>> Actually an empty constructor does zero initialize.  A clobber does not.
>>
>> Ok, thanks, I wasn't sure.
>>
>>>
>>> The check you want is TREE_CLOBBER_P instead.
>>
>> So in initializer_zerop, a constructor that is a TREE_CLOBBER_P would
>> return false, right? I will make that change and retest.
>
> Yes that is correct.  Note TREE_CLOBBER_P already checks if it is an
> constructor.
>
> Thanks,
> Andrew
>
>>
>> Thanks,
>> Teresa
>>
>>>
>>> Thanks,
>>> Andrew
>>>
>>>
>>>>
>>>> gcc/testsuite:
>>>>         * g++.dg/tree-ssa/pr63841.C: New test.
>>>>
>>>> Index: tree.c
>>>> ===================================================================
>>>> --- tree.c      (revision 217190)
>>>> +++ tree.c      (working copy)
>>>> @@ -10330,6 +10330,8 @@ initializer_zerop (const_tree init)
>>>>        {
>>>>         unsigned HOST_WIDE_INT idx;
>>>>
>>>> +        if (!CONSTRUCTOR_NELTS (init))
>>>> +          return false;
>>>>         FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), idx, elt)
>>>>           if (!initializer_zerop (elt))
>>>>             return false;
>>>> Index: testsuite/g++.dg/tree-ssa/pr63841.C
>>>> ===================================================================
>>>> --- testsuite/g++.dg/tree-ssa/pr63841.C (revision 0)
>>>> +++ testsuite/g++.dg/tree-ssa/pr63841.C (working copy)
>>>> @@ -0,0 +1,38 @@
>>>> +/* { dg-do run } */
>>>> +/* { dg-options "-O2" } */
>>>> +
>>>> +#include <cstdio>
>>>> +#include <string>
>>>> +
>>>> +std::string __attribute__ ((noinline)) comp_test_write() {
>>>> +  std::string data;
>>>> +
>>>> +  for (int i = 0; i < 2; ++i) {
>>>> +    char b = 1 >> (i * 8);
>>>> +    data.append(&b, 1);
>>>> +  }
>>>> +
>>>> +  return data;
>>>> +}
>>>> +
>>>> +std::string __attribute__ ((noinline)) comp_test_write_good() {
>>>> +  std::string data;
>>>> +
>>>> +  char b;
>>>> +  for (int i = 0; i < 2; ++i) {
>>>> +    b = 1 >> (i * 8);
>>>> +    data.append(&b, 1);
>>>> +  }
>>>> +
>>>> +  return data;
>>>> +}
>>>> +
>>>> +int main() {
>>>> +  std::string good = comp_test_write_good();
>>>> +  printf("expected: %hx\n", *(short*)good.c_str());
>>>> +
>>>> +  std::string bad = comp_test_write();
>>>> +  printf("got: %hx\n", *(short*)bad.c_str());
>>>> +
>>>> +  return good != bad;
>>>> +}
>>>>
>>>> On Wed, Nov 12, 2014 at 2:17 PM, Xinliang David Li <davidxl@google.com> wrote:
>>>>> missing test case?
>>>>>
>>>>> David
>>>>>
>>>>> On Wed, Nov 12, 2014 at 2:13 PM, Teresa Johnson <tejohnson@google.com> wrote:
>>>>>> This patch fixes an issue where tree-strlen was incorrectly removing a
>>>>>> store of 0 into a string because it thought a prior CLOBBER (which is
>>>>>> an empty constructor with no elements) was zero-initializing the
>>>>>> string.
>>>>>>
>>>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu. OK for trunk?
>>>>>>
>>>>>> Thanks,
>>>>>> Teresa
>>>>>>
>>>>>> 2014-11-12    <tejohnson@google.com>
>>>>>>
>>>>>>         PR tree-optimization/63841
>>>>>>         * tree.c (initializer_zerop): A constructor with no elements
>>>>>>         does not zero initialize.
>>>>>>
>>>>>> Index: tree.c
>>>>>> ===================================================================
>>>>>> --- tree.c      (revision 217190)
>>>>>> +++ tree.c      (working copy)
>>>>>> @@ -10330,6 +10330,8 @@ initializer_zerop (const_tree init)
>>>>>>        {
>>>>>>         unsigned HOST_WIDE_INT idx;
>>>>>>
>>>>>> +        if (!CONSTRUCTOR_NELTS (init))
>>>>>> +          return false;
>>>>>>         FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), idx, elt)
>>>>>>           if (!initializer_zerop (elt))
>>>>>>             return false;
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
>>>>
>>>>
>>>>
>>>> --
>>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413