Here is the new patch. Bootstrapped and tested on x86_64-unknown-linux-gnu. OK for trunk?
Thanks, Teresa 2014-11-13 <tejohn...@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 <pins...@gmail.com> wrote: > On Wed, Nov 12, 2014 at 9:38 PM, Teresa Johnson <tejohn...@google.com> wrote: >> On Wed, Nov 12, 2014 at 9:30 PM, Andrew Pinski <pins...@gmail.com> wrote: >>> On Wed, Nov 12, 2014 at 9:25 PM, Teresa Johnson <tejohn...@google.com> >>> wrote: >>>> Added testcase. Here is the new patch: >>>> >>>> 2014-11-12 <tejohn...@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 <davi...@google.com> >>>> wrote: >>>>> missing test case? >>>>> >>>>> David >>>>> >>>>> On Wed, Nov 12, 2014 at 2:13 PM, Teresa Johnson <tejohn...@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 <tejohn...@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 | tejohn...@google.com | 408-460-2413 >>>> >>>> >>>> >>>> -- >>>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413