diff mbox

Don't ICE with huge alignment (PR middle-end/60226)

Message ID 20140703101823.GL20427@redhat.com
State New
Headers show

Commit Message

Marek Polacek July 3, 2014, 10:18 a.m. UTC 
On Mon, Jun 30, 2014 at 03:40:18PM -0700, Mike Stump wrote:
> 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).

Yeah, I did that.  One issue with that is that round_up now wraps
the value, so I had to add a check for huge size before rounding up,
otherwise we'd regress on e.g. PR42611.

How about the following?

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2014-07-03  Marek Polacek  <polacek@redhat.com>

	PR c/60226
	* fold-const.c (round_up_loc): Change the parameter type.
	Remove assert.
	* fold-const.h (round_up_loc): Adjust declaration.
	* stor-layout.c (finalize_record_size): Check for too large types.

	* c-c++-common/pr60226.c: New test.


	Marek

Comments

Jeff Law July 7, 2014, 5:43 p.m. UTC | #1 
On 07/03/14 04:18, Marek Polacek wrote:
> On Mon, Jun 30, 2014 at 03:40:18PM -0700, Mike Stump wrote:
>> 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).
>
> Yeah, I did that.  One issue with that is that round_up now wraps
> the value, so I had to add a check for huge size before rounding up,
> otherwise we'd regress on e.g. PR42611.
>
> How about the following?
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2014-07-03  Marek Polacek  <polacek@redhat.com>
>
> 	PR c/60226
> 	* fold-const.c (round_up_loc): Change the parameter type.
> 	Remove assert.
> 	* fold-const.h (round_up_loc): Adjust declaration.
> 	* stor-layout.c (finalize_record_size): Check for too large types.
>
> 	* c-c++-common/pr60226.c: New test.
OK.
Jeff
diff mbox

Patch

diff --git gcc/fold-const.c gcc/fold-const.c
index d22eac1..c57ac7b 100644
--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -16647,11 +16647,10 @@  fold_ignored_result (tree t)
 /* Return the value of VALUE, rounded up to a multiple of DIVISOR. */
 
 tree
-round_up_loc (location_t loc, tree value, int divisor)
+round_up_loc (location_t loc, tree value, unsigned int divisor)
 {
   tree div = NULL_TREE;
 
-  gcc_assert (divisor > 0);
   if (divisor == 1)
     return value;
 
diff --git gcc/fold-const.h gcc/fold-const.h
index dcb97a1..3b5fd84 100644
--- gcc/fold-const.h
+++ gcc/fold-const.h
@@ -144,7 +144,7 @@  extern tree combine_comparisons (location_t, enum tree_code, enum tree_code,
 extern void debug_fold_checksum (const_tree);
 extern bool may_negate_without_overflow_p (const_tree);
 #define round_up(T,N) round_up_loc (UNKNOWN_LOCATION, T, N)
-extern tree round_up_loc (location_t, tree, int);
+extern tree round_up_loc (location_t, tree, unsigned int);
 #define round_down(T,N) round_down_loc (UNKNOWN_LOCATION, T, N)
 extern tree round_down_loc (location_t, tree, int);
 extern tree size_int_kind (HOST_WIDE_INT, enum size_type_kind);
diff --git gcc/stor-layout.c gcc/stor-layout.c
index cfd436f..19e7adb 100644
--- gcc/stor-layout.c
+++ gcc/stor-layout.c
@@ -1587,6 +1587,11 @@  finalize_record_size (record_layout_info rli)
     unpadded_size_unit
       = size_binop (PLUS_EXPR, unpadded_size_unit, size_one_node);
 
+  if (TREE_CODE (unpadded_size_unit) == INTEGER_CST
+      && !TREE_OVERFLOW (unpadded_size_unit)
+      && !valid_constant_size_p (unpadded_size_unit))
+    error ("type %qT is too large", rli->t);
+
   /* Round the size up to be a multiple of the required alignment.  */
   TYPE_SIZE (rli->t) = round_up (unpadded_size, TYPE_ALIGN (rli->t));
   TYPE_SIZE_UNIT (rli->t)
diff --git gcc/testsuite/c-c++-common/pr60226.c gcc/testsuite/c-c++-common/pr60226.c
index e69de29..3a1c261 100644
--- gcc/testsuite/c-c++-common/pr60226.c
+++ gcc/testsuite/c-c++-common/pr60226.c
@@ -0,0 +1,14 @@ 
+/* 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" } */
+typedef int __attribute__ ((aligned (1 << 29))) int29; /* { dg-error "requested alignment is too large" } */
+
+void
+f (void)
+{
+  struct { __attribute__((aligned (1 << 28))) double a; } x1;
+  struct { __attribute__((aligned (1 << 29))) double a; } x2; /* { dg-error "requested alignment is too large" } */
+}