This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Fix constant expressions ICEs


This patch fixes some ICEs caused by my constant expressions changes,
resulting from C_MAYBE_CONST_EXPR appearing where it should not have
appeared.

In PR 39614, it was possible for the C_MAYBE_CONST_EXPRs created by
note_integer_operands (which deals with expressions that are not
integer constant expressions themselves but may appear in unevaluated
subexpressions thereof - for example, division by zero or shifts by an
out-of-range constant value) to be nested, which broke the principle
that the expression inside a C_MAYBE_CONST_EXPR had already been fully
folded and lowered (to avoid repeated folding of the same
expressions).  This is fixed by arranging for the functions calling
note_integer_operands to remove an incoming C_MAYBE_CONST_EXPR from
their arguments if those arguments may be used in integer constant
expressions.

In PR 39673, the problem was convert calling fold to fold the output
of various conversion functions - which might just be the same as the
input expression, so leading to an attempt to fold a
C_MAYBE_CONST_EXPR with the generic fold instead of c_fully_fold.
Since the conversion functions already use fold_build* in many cases
and c_fully_fold will also do recursive folding, there is now less
need for these fold () calls (some of which were already present in
r247, the first revision of c-convert.c in the repository); however,
there are still cases where these conversion functions do not fold the
resulting trees, or where changes to this folding have fallout
elsewhere, so this patch removes the fold () calls only for the
C_MAYBE_CONST_EXPR case.  It may be possible to reduce the folding
here further in future.

Similarly, there is no need for a fold call when parsing
__builtin_choose_expr (integer constant expressions have already been
folded at this point and nothing else is valid for the condition), and
that fold call can cause similar problems, so is also removed.

Bootstrapped with no regressions on i686-pc-linux-gnu.  Applied to
mainline.

2009-04-08  Joseph Myers  <joseph@codesourcery.com>

	PR c/39614
	PR c/39673
	* c-common.h (C_MAYBE_CONST_EXPR_PRE, C_MAYBE_CONST_EXPR_EXPR,
	C_MAYBE_CONST_EXPR_INT_OPERANDS, C_MAYBE_CONST_EXPR_NON_CONST,
	EXPR_INT_CONST_OPERANDS): Remove duplicate definitions.
	* c-convert.c (convert): Do not call fold on results of conversion
	functions when the result is a C_MAYBE_CONST_EXPR.
	* c-parser.c (c_parser_postfix_expression): Do not fold condition
	of __builtin_choose_expr.
	* c-typeck.c (remove_c_maybe_const_expr): New.
	(build_unary_op, build_conditional_expr, build_compound_expr,
	build_binary_op, c_objc_common_truthvalue_conversion): Call
	remove_c_maybe_const_expr on any input C_MAYBE_CONST_EXPR with
	integer operands.

testsuite:
2009-04-08  Joseph Myers  <joseph@codesourcery.com>

	PR c/39614
	PR c/39673
	* gcc.c-torture/compile/pr39614-1.c,
	gcc.c-torture/compile/pr39614-2.c,
	gcc.c-torture/compile/pr39614-3.c,
	gcc.c-torture/compile/pr39614-4.c,
	gcc.c-torture/compile/pr39614-5.c,
	gcc.c-torture/compile/pr39673-1.c,
	gcc.c-torture/compile/pr39673-2.c: New tests.
	* gcc.dg/gnu89-const-expr-2.c, gcc.dg/gnu99-const-expr-2.c: Test
	more cases.
	* gcc.dg/overflow-warn-1.c, gcc.dg/overflow-warn-2.c,
	gcc.dg/overflow-warn-3.c, gcc.dg/overflow-warn-4.c: Update
	expected errors.

Index: c-common.h
===================================================================
--- c-common.h	(revision 145673)
+++ c-common.h	(working copy)
@@ -874,21 +874,6 @@ extern void finish_file	(void);
        || (TREE_CODE (EXPR) == C_MAYBE_CONST_EXPR	\
 	   && C_MAYBE_CONST_EXPR_INT_OPERANDS (EXPR))))
 
-/* C_MAYBE_CONST_EXPR accessors.  */
-#define C_MAYBE_CONST_EXPR_PRE(NODE)			\
-  TREE_OPERAND (C_MAYBE_CONST_EXPR_CHECK (NODE), 0)
-#define C_MAYBE_CONST_EXPR_EXPR(NODE)			\
-  TREE_OPERAND (C_MAYBE_CONST_EXPR_CHECK (NODE), 1)
-#define C_MAYBE_CONST_EXPR_INT_OPERANDS(NODE)		\
-  TREE_LANG_FLAG_0 (C_MAYBE_CONST_EXPR_CHECK (NODE))
-#define C_MAYBE_CONST_EXPR_NON_CONST(NODE)		\
-  TREE_LANG_FLAG_1 (C_MAYBE_CONST_EXPR_CHECK (NODE))
-#define EXPR_INT_CONST_OPERANDS(EXPR)			\
-  (INTEGRAL_TYPE_P (TREE_TYPE (EXPR))			\
-   && (TREE_CODE (EXPR) == INTEGER_CST			\
-       || (TREE_CODE (EXPR) == C_MAYBE_CONST_EXPR	\
-	   && C_MAYBE_CONST_EXPR_INT_OPERANDS (EXPR))))
-
 /* In a FIELD_DECL, nonzero if the decl was originally a bitfield.  */
 #define DECL_C_BIT_FIELD(NODE) \
   (DECL_LANG_FLAG_4 (FIELD_DECL_CHECK (NODE)) == 1)
Index: c-convert.c
===================================================================
--- c-convert.c	(revision 145673)
+++ c-convert.c	(working copy)
@@ -70,6 +70,7 @@ convert (tree type, tree expr)
   tree e = expr;
   enum tree_code code = TREE_CODE (type);
   const char *invalid_conv_diag;
+  tree ret;
 
   if (type == error_mark_node
       || expr == error_mark_node
@@ -97,26 +98,56 @@ convert (tree type, tree expr)
       error ("void value not ignored as it ought to be");
       return error_mark_node;
     }
-  if (code == VOID_TYPE)
-    return fold_convert (type, e);
-  if (code == INTEGER_TYPE || code == ENUMERAL_TYPE)
-    return fold (convert_to_integer (type, e));
-  if (code == BOOLEAN_TYPE)
-    return fold_convert 
-      (type, c_objc_common_truthvalue_conversion (input_location, expr));
-  if (code == POINTER_TYPE || code == REFERENCE_TYPE)
-    return fold (convert_to_pointer (type, e));
-  if (code == REAL_TYPE)
-    return fold (convert_to_real (type, e));
-  if (code == FIXED_POINT_TYPE)
-    return fold (convert_to_fixed (type, e));
-  if (code == COMPLEX_TYPE)
-    return fold (convert_to_complex (type, e));
-  if (code == VECTOR_TYPE)
-    return fold (convert_to_vector (type, e));
-  if ((code == RECORD_TYPE || code == UNION_TYPE)
-      && lang_hooks.types_compatible_p (type, TREE_TYPE (expr)))
-      return e;
+
+  switch (code)
+    {
+    case VOID_TYPE:
+      return fold_convert (type, e);
+
+    case INTEGER_TYPE:
+    case ENUMERAL_TYPE:
+      ret = convert_to_integer (type, e);
+      goto maybe_fold;
+
+    case BOOLEAN_TYPE:
+      return fold_convert 
+	(type, c_objc_common_truthvalue_conversion (input_location, expr));
+
+    case POINTER_TYPE:
+    case REFERENCE_TYPE:
+      ret = convert_to_pointer (type, e);
+      goto maybe_fold;
+
+    case REAL_TYPE:
+      ret = convert_to_real (type, e);
+      goto maybe_fold;
+
+    case FIXED_POINT_TYPE:
+      ret = convert_to_fixed (type, e);
+      goto maybe_fold;
+
+    case COMPLEX_TYPE:
+      ret = convert_to_complex (type, e);
+      goto maybe_fold;
+
+    case VECTOR_TYPE:
+      ret = convert_to_vector (type, e);
+      goto maybe_fold;
+
+    case RECORD_TYPE:
+    case UNION_TYPE:
+      if (lang_hooks.types_compatible_p (type, TREE_TYPE (expr)))
+	return e;
+      break;
+
+    default:
+      break;
+
+    maybe_fold:
+      if (TREE_CODE (ret) != C_MAYBE_CONST_EXPR)
+	ret = fold (ret);
+      return ret;
+    }
 
   error ("conversion to non-scalar type requested");
   return error_mark_node;
Index: c-parser.c
===================================================================
--- c-parser.c	(revision 145673)
+++ c-parser.c	(working copy)
@@ -5403,7 +5403,7 @@ c_parser_postfix_expression (c_parser *p
 	  {
 	    tree c;
 
-	    c = fold (e1.value);
+	    c = e1.value;
 	    if (TREE_CODE (c) != INTEGER_CST
 		|| !INTEGRAL_TYPE_P (TREE_TYPE (c)))
 	      error_at (loc,
Index: c-typeck.c
===================================================================
--- c-typeck.c	(revision 145673)
+++ c-typeck.c	(working copy)
@@ -151,6 +151,20 @@ note_integer_operands (tree expr)
   return ret;
 }
 
+/* Having checked whether EXPR may appear in an unevaluated part of an
+   integer constant expression and found that it may, remove any
+   C_MAYBE_CONST_EXPR noting this fact and return the resulting
+   expression.  */
+
+static inline tree
+remove_c_maybe_const_expr (tree expr)
+{
+  if (TREE_CODE (expr) == C_MAYBE_CONST_EXPR)
+    return C_MAYBE_CONST_EXPR_EXPR (expr);
+  else
+    return expr;
+}
+
 /* This is a cache to hold if two types are compatible or not.  */
 
 struct tagged_tu_seen_cache {
@@ -3013,6 +3027,8 @@ build_unary_op (location_t location,
   bool int_operands;
 
   int_operands = EXPR_INT_CONST_OPERANDS (xarg);
+  if (int_operands)
+    arg = remove_c_maybe_const_expr (arg);
 
   if (code != ADDR_EXPR)
     arg = require_complete_type (arg);
@@ -3576,9 +3592,20 @@ build_conditional_expr (tree ifexp, bool
   tree ep_result_type = NULL;
   tree orig_op1 = op1, orig_op2 = op2;
   bool int_const, op1_int_operands, op2_int_operands, int_operands;
+  bool ifexp_int_operands;
   tree ret;
   bool objc_ok;
 
+  op1_int_operands = EXPR_INT_CONST_OPERANDS (orig_op1);
+  if (op1_int_operands)
+    op1 = remove_c_maybe_const_expr (op1);
+  op2_int_operands = EXPR_INT_CONST_OPERANDS (orig_op2);
+  if (op2_int_operands)
+    op2 = remove_c_maybe_const_expr (op2);
+  ifexp_int_operands = EXPR_INT_CONST_OPERANDS (ifexp);
+  if (ifexp_int_operands)
+    ifexp = remove_c_maybe_const_expr (ifexp);
+
   /* Promote both alternatives.  */
 
   if (TREE_CODE (TREE_TYPE (op1)) != VOID_TYPE)
@@ -3793,8 +3820,6 @@ build_conditional_expr (tree ifexp, bool
   if (result_type != TREE_TYPE (op2))
     op2 = convert_and_check (result_type, op2);
 
-  op1_int_operands = EXPR_INT_CONST_OPERANDS (orig_op1);
-  op2_int_operands = EXPR_INT_CONST_OPERANDS (orig_op2);
   if (ifexp_bcp && ifexp == truthvalue_true_node)
     {
       op2_int_operands = true;
@@ -3805,7 +3830,7 @@ build_conditional_expr (tree ifexp, bool
       op1_int_operands = true;
       op2 = c_fully_fold (op2, require_constant_value, NULL);
     }
-  int_const = int_operands = (EXPR_INT_CONST_OPERANDS (ifexp)
+  int_const = int_operands = (ifexp_int_operands
 			      && op1_int_operands
 			      && op2_int_operands);
   if (int_operands)
@@ -3837,9 +3862,17 @@ build_conditional_expr (tree ifexp, bool
 tree
 build_compound_expr (tree expr1, tree expr2)
 {
+  bool expr1_int_operands, expr2_int_operands;
   tree eptype = NULL_TREE;
   tree ret;
 
+  expr1_int_operands = EXPR_INT_CONST_OPERANDS (expr1);
+  if (expr1_int_operands)
+    expr1 = remove_c_maybe_const_expr (expr1);
+  expr2_int_operands = EXPR_INT_CONST_OPERANDS (expr2);
+  if (expr2_int_operands)
+    expr2 = remove_c_maybe_const_expr (expr2);
+
   if (TREE_CODE (expr1) == EXCESS_PRECISION_EXPR)
     expr1 = TREE_OPERAND (expr1, 0);
   if (TREE_CODE (expr2) == EXCESS_PRECISION_EXPR)
@@ -3881,8 +3914,8 @@ build_compound_expr (tree expr1, tree ex
   ret = build2 (COMPOUND_EXPR, TREE_TYPE (expr2), expr1, expr2);
 
   if (flag_isoc99
-      && EXPR_INT_CONST_OPERANDS (expr1)
-      && EXPR_INT_CONST_OPERANDS (expr2))
+      && expr1_int_operands
+      && expr2_int_operands)
     ret = note_integer_operands (ret);
 
   if (eptype)
@@ -8440,6 +8473,7 @@ build_binary_op (location_t location, en
   tree op0, op1;
   tree ret = error_mark_node;
   const char *invalid_op_diag;
+  bool op0_int_operands, op1_int_operands;
   bool int_const, int_const_or_overflow, int_operands;
 
   /* Expression code to give to the expression when it is built.
@@ -8498,8 +8532,16 @@ build_binary_op (location_t location, en
   if (location == UNKNOWN_LOCATION)
     location = input_location;
 
-  int_operands = (EXPR_INT_CONST_OPERANDS (orig_op0)
-		  && EXPR_INT_CONST_OPERANDS (orig_op1));
+  op0 = orig_op0;
+  op1 = orig_op1;
+
+  op0_int_operands = EXPR_INT_CONST_OPERANDS (orig_op0);
+  if (op0_int_operands)
+    op0 = remove_c_maybe_const_expr (op0);
+  op1_int_operands = EXPR_INT_CONST_OPERANDS (orig_op1);
+  if (op1_int_operands)
+    op1 = remove_c_maybe_const_expr (op1);
+  int_operands = (op0_int_operands && op1_int_operands);
   if (int_operands)
     {
       int_const_or_overflow = (TREE_CODE (orig_op0) == INTEGER_CST
@@ -8513,13 +8555,8 @@ build_binary_op (location_t location, en
 
   if (convert_p)
     {
-      op0 = default_conversion (orig_op0);
-      op1 = default_conversion (orig_op1);
-    }
-  else
-    {
-      op0 = orig_op0;
-      op1 = orig_op1;
+      op0 = default_conversion (op0);
+      op1 = default_conversion (op1);
     }
 
   orig_type0 = type0 = TREE_TYPE (op0);
@@ -9196,6 +9233,8 @@ c_objc_common_truthvalue_conversion (loc
 
   int_const = (TREE_CODE (expr) == INTEGER_CST && !TREE_OVERFLOW (expr));
   int_operands = EXPR_INT_CONST_OPERANDS (expr);
+  if (int_operands)
+    expr = remove_c_maybe_const_expr (expr);
 
   /* ??? Should we also give an error for void and vectors rather than
      leaving those to give errors later?  */
Index: testsuite/gcc.c-torture/compile/pr39614-1.c
===================================================================
--- testsuite/gcc.c-torture/compile/pr39614-1.c	(revision 0)
+++ testsuite/gcc.c-torture/compile/pr39614-1.c	(revision 0)
@@ -0,0 +1,7 @@
+typedef struct page {
+ unsigned long flags;
+} mem_map_t;
+static inline void set_page_zone(struct page *page, unsigned long zone_num)
+{
+ page->flags &= ~(~0UL << (64 - 8));
+}
Index: testsuite/gcc.c-torture/compile/pr39614-2.c
===================================================================
--- testsuite/gcc.c-torture/compile/pr39614-2.c	(revision 0)
+++ testsuite/gcc.c-torture/compile/pr39614-2.c	(revision 0)
@@ -0,0 +1,6 @@
+int i;
+void
+f (void)
+{
+  i = (1 / 0) / 0;
+}
Index: testsuite/gcc.c-torture/compile/pr39614-3.c
===================================================================
--- testsuite/gcc.c-torture/compile/pr39614-3.c	(revision 0)
+++ testsuite/gcc.c-torture/compile/pr39614-3.c	(revision 0)
@@ -0,0 +1,6 @@
+int i;
+void
+f (void)
+{
+  i = (1 ? 1 / 0 : 1 / 0);
+}
Index: testsuite/gcc.c-torture/compile/pr39614-4.c
===================================================================
--- testsuite/gcc.c-torture/compile/pr39614-4.c	(revision 0)
+++ testsuite/gcc.c-torture/compile/pr39614-4.c	(revision 0)
@@ -0,0 +1,6 @@
+int i;
+void
+f (void)
+{
+  i = (1 / 0 ? 1 : 0);
+}
Index: testsuite/gcc.c-torture/compile/pr39614-5.c
===================================================================
--- testsuite/gcc.c-torture/compile/pr39614-5.c	(revision 0)
+++ testsuite/gcc.c-torture/compile/pr39614-5.c	(revision 0)
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-w -std=c99" } */
+int i;
+void
+f (void)
+{
+  i = (1 / 0, 1 / 0);
+}
Index: testsuite/gcc.c-torture/compile/pr39673-1.c
===================================================================
--- testsuite/gcc.c-torture/compile/pr39673-1.c	(revision 0)
+++ testsuite/gcc.c-torture/compile/pr39673-1.c	(revision 0)
@@ -0,0 +1,6 @@
+unsigned long f1();
+int f2();
+
+int store_aff_word(int x) {
+  return (int) (x ? f1() : f2());
+}
Index: testsuite/gcc.c-torture/compile/pr39673-2.c
===================================================================
--- testsuite/gcc.c-torture/compile/pr39673-2.c	(revision 0)
+++ testsuite/gcc.c-torture/compile/pr39673-2.c	(revision 0)
@@ -0,0 +1,6 @@
+unsigned long long f1();
+int f2();
+
+int store_aff_word(int x) {
+  return (int) (x ? f1() : f2());
+}
Index: testsuite/gcc.dg/gnu89-const-expr-2.c
===================================================================
--- testsuite/gcc.dg/gnu89-const-expr-2.c	(revision 145673)
+++ testsuite/gcc.dg/gnu89-const-expr-2.c	(working copy)
@@ -20,4 +20,7 @@ f (void)
   a = __builtin_choose_expr ((void *)0, b, c); /* { dg-error "constant" } */
   a = __builtin_choose_expr (0 * (INT_MAX + 1), b, c); /* { dg-warning "integer overflow in expression" } */
   /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 21 } */
+  a = __builtin_choose_expr (1 / 0, 0, 0); /* { dg-warning "division by zero" } */
+  /* { dg-error "not a constant" "error" { target *-*-* } 23 } */
+  a = __builtin_choose_expr ((1 ? 1 : a), b, c); /* { dg-error "constant" } */
 }
Index: testsuite/gcc.dg/gnu99-const-expr-2.c
===================================================================
--- testsuite/gcc.dg/gnu99-const-expr-2.c	(revision 145673)
+++ testsuite/gcc.dg/gnu99-const-expr-2.c	(working copy)
@@ -20,4 +20,7 @@ f (void)
   a = __builtin_choose_expr ((void *)0, b, c); /* { dg-error "constant" } */
   a = __builtin_choose_expr (0 * (INT_MAX + 1), b, c); /* { dg-warning "integer overflow in expression" } */
   /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 21 } */
+  a = __builtin_choose_expr (1 / 0, 0, 0); /* { dg-warning "division by zero" } */
+  /* { dg-error "not a constant" "error" { target *-*-* } 23 } */
+  a = __builtin_choose_expr ((1 ? 1 : a), b, c); /* { dg-error "constant" } */
 }
Index: testsuite/gcc.dg/overflow-warn-1.c
===================================================================
--- testsuite/gcc.dg/overflow-warn-1.c	(revision 145673)
+++ testsuite/gcc.dg/overflow-warn-1.c	(working copy)
@@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-war
 void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */
 /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } 49 } */
 void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
-/* { dg-error "initializer element is not constant" "constant" { target *-*-* } 51 } */
+/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } 51 } */
 /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } 51 } */
 void *r = (1 ? 0 : INT_MAX+1);
 
Index: testsuite/gcc.dg/overflow-warn-2.c
===================================================================
--- testsuite/gcc.dg/overflow-warn-2.c	(revision 145673)
+++ testsuite/gcc.dg/overflow-warn-2.c	(working copy)
@@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-war
 void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */
 /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } 49 } */
 void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
-/* { dg-error "initializer element is not constant" "constant" { target *-*-* } 51 } */
+/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } 51 } */
 /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } 51 } */
 void *r = (1 ? 0 : INT_MAX+1);
 
Index: testsuite/gcc.dg/overflow-warn-3.c
===================================================================
--- testsuite/gcc.dg/overflow-warn-3.c	(revision 145673)
+++ testsuite/gcc.dg/overflow-warn-3.c	(working copy)
@@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-war
 /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 54 } */
 /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } 54 } */
 void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
-/* { dg-error "initializer element is not constant" "constant" { target *-*-* } 57 } */
+/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } 57 } */
 /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } 57 } */
 void *r = (1 ? 0 : INT_MAX+1);
 
Index: testsuite/gcc.dg/overflow-warn-4.c
===================================================================
--- testsuite/gcc.dg/overflow-warn-4.c	(revision 145673)
+++ testsuite/gcc.dg/overflow-warn-4.c	(working copy)
@@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-war
 /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 54 } */
 /* { dg-error "initialization makes pointer from integer without a cast" "null" { target *-*-* } 54 } */
 void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
-/* { dg-error "initializer element is not constant" "constant" { target *-*-* } 57 } */
+/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } 57 } */
 /* { dg-error "initialization makes pointer from integer without a cast" "null" { target *-*-* } 57 } */
 void *r = (1 ? 0 : INT_MAX+1);
 

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]