| Message ID | 20110718174949.GY2687@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jul 18, 2011 at 7:49 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > As the testcase below shows, fold_nonarray_ctor_reference doesn't have a > correct check whether a field might overlap with the given offset, size > access (in this case a BIT_FIELD_REF). The bitfield ref wants to read > 8 bits starting at offset 0, the bitfield defined there goes from bit 1 > 1 bit and as fold_nonarray_ctor_reference doesn't detect an overlap, > it says the result is 0 as if there was an initializer, but without any > overlapping fields. Fixed thusly, bootstrapped/regtested on x86_64-linux > and i686-linux, ok for trunk/4.6? Ok. Thanks, Richard. > For trunk I think we could eventually do better and handle even multiple > fields overlapping the access and/or non-zero offsets within that, > provided the recursive call to fold_ctor_reference returned an INTEGER_CST > we could mask/shift/or them together. > > 2011-07-18 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/49768 > * gimple-fold.c (fold_nonarray_ctor_reference): Return NULL > if offset is smaller than bitoffset, but offset+size is bigger > than bitoffset. > > * gcc.c-torture/execute/pr49768.c: New test. > > --- gcc/gimple-fold.c.jj 2011-07-15 20:46:49.000000000 +0200 > +++ gcc/gimple-fold.c 2011-07-18 14:28:35.000000000 +0200 > @@ -3231,7 +3231,7 @@ fold_nonarray_ctor_reference (tree type, > double_int bitoffset; > double_int byte_offset_cst = tree_to_double_int (byte_offset); > double_int bits_per_unit_cst = uhwi_to_double_int (BITS_PER_UNIT); > - double_int bitoffset_end; > + double_int bitoffset_end, access_end; > > /* Variable sized objects in static constructors makes no sense, > but field_size can be NULL for flexible array members. */ > @@ -3252,14 +3252,16 @@ fold_nonarray_ctor_reference (tree type, > else > bitoffset_end = double_int_zero; > > - /* Is OFFSET in the range (BITOFFSET, BITOFFSET_END)? */ > - if (double_int_cmp (uhwi_to_double_int (offset), bitoffset, 0) >= 0 > + access_end = double_int_add (uhwi_to_double_int (offset), > + uhwi_to_double_int (size)); > + > + /* Is there any overlap between [OFFSET, OFFSET+SIZE) and > + [BITOFFSET, BITOFFSET_END)? */ > + if (double_int_cmp (access_end, bitoffset, 0) > 0 > && (field_size == NULL_TREE > || double_int_cmp (uhwi_to_double_int (offset), > bitoffset_end, 0) < 0)) > { > - double_int access_end = double_int_add (uhwi_to_double_int (offset), > - uhwi_to_double_int (size)); > double_int inner_offset = double_int_sub (uhwi_to_double_int (offset), > bitoffset); > /* We do have overlap. Now see if field is large enough to > @@ -3267,6 +3269,8 @@ fold_nonarray_ctor_reference (tree type, > fields. */ > if (double_int_cmp (access_end, bitoffset_end, 0) > 0) > return NULL_TREE; > + if (double_int_cmp (uhwi_to_double_int (offset), bitoffset, 0) < 0) > + return NULL_TREE; > return fold_ctor_reference (type, cval, > double_int_to_uhwi (inner_offset), size); > } > --- gcc/testsuite/gcc.c-torture/execute/pr49768.c.jj 2011-07-18 14:37:18.000000000 +0200 > +++ gcc/testsuite/gcc.c-torture/execute/pr49768.c 2011-07-18 14:37:03.000000000 +0200 > @@ -0,0 +1,12 @@ > +/* PR tree-optimization/49768 */ > + > +extern void abort (void); > + > +int > +main () > +{ > + static struct { unsigned int : 1; unsigned int s : 1; } s = { .s = 1 }; > + if (s.s != 1) > + abort (); > + return 0; > +} > > Jakub >
--- gcc/gimple-fold.c.jj 2011-07-15 20:46:49.000000000 +0200 +++ gcc/gimple-fold.c 2011-07-18 14:28:35.000000000 +0200 @@ -3231,7 +3231,7 @@ fold_nonarray_ctor_reference (tree type, double_int bitoffset; double_int byte_offset_cst = tree_to_double_int (byte_offset); double_int bits_per_unit_cst = uhwi_to_double_int (BITS_PER_UNIT); - double_int bitoffset_end; + double_int bitoffset_end, access_end; /* Variable sized objects in static constructors makes no sense, but field_size can be NULL for flexible array members. */ @@ -3252,14 +3252,16 @@ fold_nonarray_ctor_reference (tree type, else bitoffset_end = double_int_zero; - /* Is OFFSET in the range (BITOFFSET, BITOFFSET_END)? */ - if (double_int_cmp (uhwi_to_double_int (offset), bitoffset, 0) >= 0 + access_end = double_int_add (uhwi_to_double_int (offset), + uhwi_to_double_int (size)); + + /* Is there any overlap between [OFFSET, OFFSET+SIZE) and + [BITOFFSET, BITOFFSET_END)? */ + if (double_int_cmp (access_end, bitoffset, 0) > 0 && (field_size == NULL_TREE || double_int_cmp (uhwi_to_double_int (offset), bitoffset_end, 0) < 0)) { - double_int access_end = double_int_add (uhwi_to_double_int (offset), - uhwi_to_double_int (size)); double_int inner_offset = double_int_sub (uhwi_to_double_int (offset), bitoffset); /* We do have overlap. Now see if field is large enough to @@ -3267,6 +3269,8 @@ fold_nonarray_ctor_reference (tree type, fields. */ if (double_int_cmp (access_end, bitoffset_end, 0) > 0) return NULL_TREE; + if (double_int_cmp (uhwi_to_double_int (offset), bitoffset, 0) < 0) + return NULL_TREE; return fold_ctor_reference (type, cval, double_int_to_uhwi (inner_offset), size); } --- gcc/testsuite/gcc.c-torture/execute/pr49768.c.jj 2011-07-18 14:37:18.000000000 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr49768.c 2011-07-18 14:37:03.000000000 +0200 @@ -0,0 +1,12 @@ +/* PR tree-optimization/49768 */ + +extern void abort (void); + +int +main () +{ + static struct { unsigned int : 1; unsigned int s : 1; } s = { .s = 1 }; + if (s.s != 1) + abort (); + return 0; +}