Message ID | 20140304164029.GN16545@redhat.com |
---|---|
State | New |
Headers | show |
Ping. On Tue, Mar 04, 2014 at 05:40:29PM +0100, Marek Polacek wrote: > This should fix ICE on insane alignment. Normally, check_user_alignment > detects e.g. alignment 1 << 32, but not 1 << 28. However, record_align > is in bits, so it's actually 8 * (1 << 28) and that's greater than > INT_MAX. This patch rejects such code. > > In the middle hunk, we should give up when an error occurs, we don't > want to call finalize_type_size in that case -- we'd ICE in there. > > Regtested/bootstrapped on x86_64-linux, ok for trunk? > > 2014-03-04 Marek Polacek <polacek@redhat.com> > > PR middle-end/60226 > * stor-layout.c (layout_type): Return if alignment of array elements > is greater than element size. Error out if requested alignment is too > large. > cp/ > * class.c (layout_class_type): Error out if requested alignment is too > large. > testsuite/ > * c-c++-common/pr60226.c: New test. > > diff --git gcc/cp/class.c gcc/cp/class.c > index b46391b..e6325b3 100644 > --- gcc/cp/class.c > +++ gcc/cp/class.c > @@ -6378,6 +6378,14 @@ layout_class_type (tree t, tree *virtuals_p) > if (TYPE_PACKED (t) && !layout_pod_type_p (t)) > rli->packed_maybe_necessary = true; > > + if (rli->record_align >= (1U << (HOST_BITS_PER_INT - 1))) > + { > + TYPE_SIZE (rli->t) = integer_zero_node; > + TYPE_SIZE_UNIT (rli->t) = integer_zero_node; > + error ("requested alignment is too large"); > + return; > + } > + > /* Let the back end lay out the type. */ > finish_record_layout (rli, /*free_p=*/true); > > diff --git gcc/stor-layout.c gcc/stor-layout.c > index 084d195..445f0d5 100644 > --- gcc/stor-layout.c > +++ gcc/stor-layout.c > @@ -2266,8 +2266,11 @@ layout_type (tree type) > && !TREE_OVERFLOW (TYPE_SIZE_UNIT (element)) > && !integer_zerop (TYPE_SIZE_UNIT (element)) > && compare_tree_int (TYPE_SIZE_UNIT (element), > - TYPE_ALIGN_UNIT (element)) < 0) > - error ("alignment of array elements is greater than element size"); > + TYPE_ALIGN_UNIT (element)) < 0) > + { > + error ("alignment of array elements is greater than element size"); > + return; > + } > break; > } > > @@ -2294,6 +2297,14 @@ layout_type (tree type) > if (TREE_CODE (type) == QUAL_UNION_TYPE) > TYPE_FIELDS (type) = nreverse (TYPE_FIELDS (type)); > > + if (rli->record_align >= (1U << (HOST_BITS_PER_INT - 1))) > + { > + TYPE_SIZE (rli->t) = integer_zero_node; > + TYPE_SIZE_UNIT (rli->t) = integer_zero_node; > + error ("requested alignment is too large"); > + return; > + } > + > /* Finish laying out the record. */ > finish_record_layout (rli, /*free_p=*/true); > } > diff --git gcc/testsuite/c-c++-common/pr60226.c gcc/testsuite/c-c++-common/pr60226.c > index e69de29..0d7d74d 100644 > --- gcc/testsuite/c-c++-common/pr60226.c > +++ gcc/testsuite/c-c++-common/pr60226.c > @@ -0,0 +1,12 @@ > +/* PR c/60226 */ > +/* { dg-do compile } */ > +/* { dg-options "-Wno-c++-compat" { target c } } */ > + > +typedef int __attribute__ ((aligned (1 << 28))) int28; > +int28 foo[4] = {}; /* { dg-error "alignment of array elements is greater than element size" } */ > + > +void > +f (void) > +{ > + struct { __attribute__((aligned (1 << 28))) double a; } x; /* { dg-error "requested alignment is too large" } */ > +} > > Marek Marek
On 03/04/14 09:40, Marek Polacek wrote: > This should fix ICE on insane alignment. Normally, check_user_alignment > detects e.g. alignment 1 << 32, but not 1 << 28. However, record_align > is in bits, so it's actually 8 * (1 << 28) and that's greater than > INT_MAX. This patch rejects such code. > > In the middle hunk, we should give up when an error occurs, we don't > want to call finalize_type_size in that case -- we'd ICE in there. > > Regtested/bootstrapped on x86_64-linux, ok for trunk? > > 2014-03-04 Marek Polacek <polacek@redhat.com> > > PR middle-end/60226 > * stor-layout.c (layout_type): Return if alignment of array elements > is greater than element size. Error out if requested alignment is too > large. > cp/ > * class.c (layout_class_type): Error out if requested alignment is too > large. > testsuite/ > * c-c++-common/pr60226.c: New test. Is this still applicable after the wide-int changes? I haven't looked closely. jeff
On Jun 30, 2014, at 12:50 PM, Jeff Law <law@redhat.com> wrote: > On 03/04/14 09:40, Marek Polacek wrote: >> This should fix ICE on insane alignment. Normally, check_user_alignment >> detects e.g. alignment 1 << 32, but not 1 << 28. However, record_align >> is in bits, so it's actually 8 * (1 << 28) and that's greater than >> INT_MAX. This patch rejects such code. >> >> In the middle hunk, we should give up when an error occurs, we don't >> want to call finalize_type_size in that case -- we'd ICE in there. >> >> Regtested/bootstrapped on x86_64-linux, ok for trunk? >> >> 2014-03-04 Marek Polacek <polacek@redhat.com> >> >> PR middle-end/60226 >> * stor-layout.c (layout_type): Return if alignment of array elements >> is greater than element size. Error out if requested alignment is too >> large. >> cp/ >> * class.c (layout_class_type): Error out if requested alignment is too >> large. >> testsuite/ >> * c-c++-common/pr60226.c: New test. > Is this still applicable after the wide-int changes? I haven't looked closely. I glanced at it: (gdb) p/x TYPE_ALIGN (type) $1 = 2147483648 (gdb) p/x TYPE_ALIGN (type) $2 = 0x80000000 The callee is int, the caller uses unsigned int. The assert I see is because the routines are not type correct: => TYPE_SIZE (type) = round_up (TYPE_SIZE (type), TYPE_ALIGN (type)); (gdb) ptype TYPE_ALIGN (type) type = unsigned int tree round_up_loc (location_t loc, tree value, int divisor) { tree div = NULL_TREE; =>gcc_assert (divisor > 0); Would be nice if the routine was type correct (wrt unsigned).
On Jun 30, 2014, at 3:40 PM, Mike Stump <mikestump@comcast.net> wrote:
>> Is this still applicable after the wide-int changes? I haven't looked closely.
Oops, forgot to state what I wanted to state… Yes, it still aborts post wide-int…
On Mon, Jun 30, 2014 at 01:50:12PM -0600, Jeff Law wrote: > On 03/04/14 09:40, Marek Polacek wrote: > >This should fix ICE on insane alignment. Normally, check_user_alignment > >detects e.g. alignment 1 << 32, but not 1 << 28. However, record_align > >is in bits, so it's actually 8 * (1 << 28) and that's greater than > >INT_MAX. This patch rejects such code. > > > >In the middle hunk, we should give up when an error occurs, we don't > >want to call finalize_type_size in that case -- we'd ICE in there. > > > >Regtested/bootstrapped on x86_64-linux, ok for trunk? > > > >2014-03-04 Marek Polacek <polacek@redhat.com> > > > > PR middle-end/60226 > > * stor-layout.c (layout_type): Return if alignment of array elements > > is greater than element size. Error out if requested alignment is too > > large. > >cp/ > > * class.c (layout_class_type): Error out if requested alignment is too > > large. > >testsuite/ > > * c-c++-common/pr60226.c: New test. > Is this still applicable after the wide-int changes? I haven't looked > closely. Yeah, it applies cleanly. But I tried the int -> unsigned change which Mike suggested and that cures the ICE. I'll send a patch momentarily. Marek
diff --git gcc/cp/class.c gcc/cp/class.c index b46391b..e6325b3 100644 --- gcc/cp/class.c +++ gcc/cp/class.c @@ -6378,6 +6378,14 @@ layout_class_type (tree t, tree *virtuals_p) if (TYPE_PACKED (t) && !layout_pod_type_p (t)) rli->packed_maybe_necessary = true; + if (rli->record_align >= (1U << (HOST_BITS_PER_INT - 1))) + { + TYPE_SIZE (rli->t) = integer_zero_node; + TYPE_SIZE_UNIT (rli->t) = integer_zero_node; + error ("requested alignment is too large"); + return; + } + /* Let the back end lay out the type. */ finish_record_layout (rli, /*free_p=*/true); diff --git gcc/stor-layout.c gcc/stor-layout.c index 084d195..445f0d5 100644 --- gcc/stor-layout.c +++ gcc/stor-layout.c @@ -2266,8 +2266,11 @@ layout_type (tree type) && !TREE_OVERFLOW (TYPE_SIZE_UNIT (element)) && !integer_zerop (TYPE_SIZE_UNIT (element)) && compare_tree_int (TYPE_SIZE_UNIT (element), - TYPE_ALIGN_UNIT (element)) < 0) - error ("alignment of array elements is greater than element size"); + TYPE_ALIGN_UNIT (element)) < 0) + { + error ("alignment of array elements is greater than element size"); + return; + } break; } @@ -2294,6 +2297,14 @@ layout_type (tree type) if (TREE_CODE (type) == QUAL_UNION_TYPE) TYPE_FIELDS (type) = nreverse (TYPE_FIELDS (type)); + if (rli->record_align >= (1U << (HOST_BITS_PER_INT - 1))) + { + TYPE_SIZE (rli->t) = integer_zero_node; + TYPE_SIZE_UNIT (rli->t) = integer_zero_node; + error ("requested alignment is too large"); + return; + } + /* Finish laying out the record. */ finish_record_layout (rli, /*free_p=*/true); } diff --git gcc/testsuite/c-c++-common/pr60226.c gcc/testsuite/c-c++-common/pr60226.c index e69de29..0d7d74d 100644 --- gcc/testsuite/c-c++-common/pr60226.c +++ gcc/testsuite/c-c++-common/pr60226.c @@ -0,0 +1,12 @@ +/* PR c/60226 */ +/* { dg-do compile } */ +/* { dg-options "-Wno-c++-compat" { target c } } */ + +typedef int __attribute__ ((aligned (1 << 28))) int28; +int28 foo[4] = {}; /* { dg-error "alignment of array elements is greater than element size" } */ + +void +f (void) +{ + struct { __attribute__((aligned (1 << 28))) double a; } x; /* { dg-error "requested alignment is too large" } */ +}