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

Richard Biener richard.guenther@gmail.com
Thu Nov 13 14:32:00 GMT 2014


On Thu, Nov 13, 2014 at 3:20 PM, Teresa Johnson <tejohnson@google.com> wrote:
> Here is the new patch. Bootstrapped and tested on
> x86_64-unknown-linux-gnu. OK for trunk?

Ok for trunk and branches.

Thanks,
Richard.

> 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



More information about the Gcc-patches mailing list