Fix infinite loop/crash if array initializer index equals max value

Submitted by Senthil Kumar Selvaraj on Sept. 4, 2013, 1:34 p.m.

Details

Message ID 20130904133418.GA726@atmel.com
State New
Headers show

Commit Message

Senthil Kumar Selvaraj Sept. 4, 2013, 1:34 p.m.
On Fri, Aug 23, 2013 at 09:49:55PM +0000, Joseph S. Myers wrote:
> On Thu, 22 Aug 2013, Selvaraj, Senthil_Kumar wrote:
> 
> > 2013-08-23  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
> > 	* c-typeck.c (output_pending_init_elements): Handle overflow of
> > 	constructor_unfilled_index.
> 
> This patch needs to add include a testcase to the testsuite that fails 
> before and passes after the patch.  (I realise such a test may only be 
> able to run for a subset of targets.)

Reattaching the patch with a testcase for the AVR target. I'm not sure
how to generalize the testcase for other targets - the constant is the
max value (unsigned) of the mode used to represent initialized array
indices.

The attached test fails with a timeout before applying the patch, and
passes after applying it.

Regards
Senthil

gcc/c/ChangeLog

2013-09-04  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
	* c-typeck.c (output_pending_init_elements): Handle overflow of
	constructor_unfilled_index.

gcc/testsuite/ChangeLog

2013-09-04  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
	* gcc.dg/large-size-array-7.c: New test to verify overflow handling
	of constructor_unfilled_index.

Comments

Joseph S. Myers Sept. 4, 2013, 3:15 p.m.
On Wed, 4 Sep 2013, Senthil Kumar Selvaraj wrote:

> Reattaching the patch with a testcase for the AVR target. I'm not sure
> how to generalize the testcase for other targets - the constant is the
> max value (unsigned) of the mode used to represent initialized array
> indices.

Logically that should be __SIZE_MAX__, unless there's an unusual choice of 
size_t.

Are there any issues with large compiler memory consumption if you use 
__SIZE_MAX__ there for ordinary 32-bit or 64-bit targets?  If so, that 
would be a reason to keep the test restricted to AVR; if not, it would 
seem better to have __SIZE_MAX__ in the test (or a macro defined in the 
test that defaults to __SIZE_MAX__ but is overridden for particular 
targets, if some targets need it overridden).

Patch hide | download patch | download mbox

diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 30871db..ed2e37a 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -7953,8 +7953,9 @@  output_pending_init_elements (int all, struct obstack * braced_init_obstack)
 				 TREE_TYPE (constructor_type),
 				 constructor_unfilled_index, 0, false,
 				 braced_init_obstack);
-	  else if (tree_int_cst_lt (constructor_unfilled_index,
-				    elt->purpose))
+      else if (!TREE_OVERFLOW_P (constructor_unfilled_index)
+            && tree_int_cst_lt (constructor_unfilled_index,
+                   elt->purpose))
 	    {
 	      /* Advance to the next smaller node.  */
 	      if (elt->left)
@@ -7979,7 +7980,8 @@  output_pending_init_elements (int all, struct obstack * braced_init_obstack)
 		  while (elt->parent && elt->parent->right == elt)
 		    elt = elt->parent;
 		  elt = elt->parent;
-		  if (elt && tree_int_cst_lt (constructor_unfilled_index,
+          if (elt && !TREE_OVERFLOW_P (constructor_unfilled_index)
+              && tree_int_cst_lt (constructor_unfilled_index,
 					      elt->purpose))
 		    {
 		      next = elt->purpose;
diff --git gcc/testsuite/gcc.dg/large-size-array-7.c gcc/testsuite/gcc.dg/large-size-array-7.c
new file mode 100644
index 0000000..196767d
--- /dev/null
+++ gcc/testsuite/gcc.dg/large-size-array-7.c
@@ -0,0 +1,5 @@ 
+/* { dg-do compile { target "avr-*-*" } } */
+/* { dg-options "-O2" } */
+static char * name[] = {
+    [0xFFFFFF]  = "bar"
+  }; /* { dg-error "too large" } */