re PR c/16302 (gcc fails to warn about some common logic errors)

2009-05-15  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR 16302
	* fold-const.c (make_range,build_range_check,merge_ranges): Move
	declaration to...
	(merge_ranges): Returns bool. 
	* tree.h (make_range): .. to here.
	(build_range_check): Likewise.
	(merge_ranges): Likewise. Renamed from merge_ranges.
	* c-typeck.c (parser_build_binary_op): Update calls to
	warn_logical_operator.
	* c-common.c (warn_logical_operator): Add new warning.
	* c-common.h (warn_logical_operator): Update declaration.
cp/
	* call.c (build_new_op): Update calls to warn_logical_operator.
	
testsuite/
	* gcc.dg/pr16302.c: New.
	* g++.dg/warn/pr16302.C: New.

From-SVN: r147596

11 files changed, 253 insertions, 16 deletions

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b9a1ce20..f107086 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,19 @@
 2009-05-15  Manuel López-Ibáñez  <manu@gcc.gnu.org>
 
+	PR 16302
+	* fold-const.c (make_range,build_range_check,merge_ranges): Move
+	declaration to...
+	(merge_ranges): Returns bool. 
+	* tree.h (make_range): .. to here.
+	(build_range_check): Likewise.
+	(merge_ranges): Likewise. Renamed from merge_ranges.
+	* c-typeck.c (parser_build_binary_op): Update calls to
+	warn_logical_operator.
+	* c-common.c (warn_logical_operator): Add new warning.
+	* c-common.h (warn_logical_operator): Update declaration.
+	
+2009-05-15  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
 	* ira-conflicts.c (add_insn_allocno_copies): Fix wrong
 	conditional.
 	
diff --git a/gcc/c-common.c b/gcc/c-common.c
index df6673c..e5c3d0d 100644
--- a/gcc/c-common.c
+++ b/gcc/c-common.c
@@ -1716,14 +1716,18 @@ overflow_warning (tree value)
 /* Warn about uses of logical || / && operator in a context where it
    is likely that the bitwise equivalent was intended by the
    programmer.  We have seen an expression in which CODE is a binary
-   operator used to combine expressions OP_LEFT and OP_RIGHT, which
-   before folding had CODE_LEFT and CODE_RIGHT.  */
-
+   operator used to combine expressions OP_LEFT and OP_RIGHT, which before folding
+   had CODE_LEFT and CODE_RIGHT, into an expression of type TYPE.  */
 void
-warn_logical_operator (location_t location, enum tree_code code,
+warn_logical_operator (location_t location, enum tree_code code, tree type,
 		       enum tree_code code_left, tree op_left, 
 		       enum tree_code ARG_UNUSED (code_right), tree op_right)
 {
+  int or_op = (code == TRUTH_ORIF_EXPR || code == TRUTH_OR_EXPR);
+  int in0_p, in1_p, in_p;
+  tree low0, low1, low, high0, high1, high, lhs, rhs, tem;
+  bool strict_overflow_p = false;
+
   if (code != TRUTH_ANDIF_EXPR
       && code != TRUTH_AND_EXPR
       && code != TRUTH_ORIF_EXPR
@@ -1743,13 +1747,65 @@ warn_logical_operator (location_t location, enum tree_code code,
       && !integer_zerop (op_right)
       && !integer_onep (op_right))
     {
-      if (code == TRUTH_ORIF_EXPR || code == TRUTH_OR_EXPR)
+      if (or_op)
 	warning_at (location, OPT_Wlogical_op, "logical %<or%>"
 		    " applied to non-boolean constant");
       else
 	warning_at (location, OPT_Wlogical_op, "logical %<and%>"
 		    " applied to non-boolean constant");
       TREE_NO_WARNING (op_left) = true;
+      return;
+    }
+
+  /* We do not warn for constants because they are typical of macro
+     expansions that test for features.  */
+  if (CONSTANT_CLASS_P (op_left) || CONSTANT_CLASS_P (op_right))
+    return;
+
+  /* This warning only makes sense with logical operands.  */
+  if (!(truth_value_p (TREE_CODE (op_left))
+	|| INTEGRAL_TYPE_P (TREE_TYPE (op_left)))
+      || !(truth_value_p (TREE_CODE (op_right))
+	   || INTEGRAL_TYPE_P (TREE_TYPE (op_right))))
+    return;
+
+  lhs = make_range (op_left, &in0_p, &low0, &high0, &strict_overflow_p);
+  rhs = make_range (op_right, &in1_p, &low1, &high1, &strict_overflow_p);
+
+  if (lhs && TREE_CODE (lhs) == C_MAYBE_CONST_EXPR)
+    lhs = C_MAYBE_CONST_EXPR_EXPR (lhs);
+
+  if (rhs && TREE_CODE (rhs) == C_MAYBE_CONST_EXPR)
+    rhs = C_MAYBE_CONST_EXPR_EXPR (rhs);
+  
+  /* If this is an OR operation, invert both sides; we will invert
+     again at the end.  */
+  if (or_op)
+    in0_p = !in0_p, in1_p = !in1_p;
+  
+  /* If both expressions are the same, if we can merge the ranges, and we
+     can build the range test, return it or it inverted.  If one of the
+     ranges is always true or always false, consider it to be the same
+     expression as the other.  */
+  if ((lhs == 0 || rhs == 0 || operand_equal_p (lhs, rhs, 0))
+      && merge_ranges (&in_p, &low, &high, in0_p, low0, high0,
+		       in1_p, low1, high1)
+      && 0 != (tem = build_range_check (type,
+					lhs != 0 ? lhs
+					: rhs != 0 ? rhs : integer_zero_node,
+					in_p, low, high)))
+    {
+      if (TREE_CODE (tem) != INTEGER_CST)
+	return;
+
+      if (or_op)
+        warning_at (location, OPT_Wlogical_op,
+                    "logical %<or%> "
+                    "of collectively exhaustive tests is always true");
+      else
+        warning_at (location, OPT_Wlogical_op,
+                    "logical %<and%> "
+                    "of mutually exclusive tests is always false");
     }
 }
 
diff --git a/gcc/c-common.h b/gcc/c-common.h
index 250a7ff..c4fba4d 100644
--- a/gcc/c-common.h
+++ b/gcc/c-common.h
@@ -804,7 +804,7 @@ extern bool strict_aliasing_warning (tree, tree, tree);
 extern void warnings_for_convert_and_check (tree, tree, tree);
 extern tree convert_and_check (tree, tree);
 extern void overflow_warning (tree);
-extern void warn_logical_operator (location_t, enum tree_code,
+extern void warn_logical_operator (location_t, enum tree_code, tree,
 				   enum tree_code, tree, enum tree_code, tree);
 extern void check_main_parameter_types (tree decl);
 extern bool c_determine_visibility (tree);
diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c
index 13cd3e3..62b5ee9 100644
--- a/gcc/c-typeck.c
+++ b/gcc/c-typeck.c
@@ -2961,7 +2961,7 @@ parser_build_binary_op (location_t location, enum tree_code code,
     warn_about_parentheses (code, code1, arg1.value, code2, arg2.value);
 
   if (warn_logical_op)
-    warn_logical_operator (input_location, code,
+    warn_logical_operator (input_location, code, TREE_TYPE (result.value),
 			   code1, arg1.value, code2, arg2.value);
 
   /* Warn about comparisons against string literals, with the exception
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 3d5aeb0..b4d7826 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,8 @@
+2009-05-15  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
+	PR 16302
+	* call.c (build_new_op): Update calls to warn_logical_operator.
+
 2009-05-14  Ian Lance Taylor  <iant@google.com>
 
 	* class.c (layout_class_type): Change itk to unsigned int.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index ee13ba2..607e3ed 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -4161,7 +4161,7 @@ build_new_op (enum tree_code code, int flags, tree arg1, tree arg2, tree arg3,
 	      /* We need to call warn_logical_operator before
 		 converting arg2 to a boolean_type.  */
 	      if (complain & tf_warning)
-		warn_logical_operator (input_location, code,
+		warn_logical_operator (input_location, code, boolean_type_node,
 				       code_orig_arg1, arg1,
 				       code_orig_arg2, arg2);
 
@@ -4202,7 +4202,7 @@ build_new_op (enum tree_code code, int flags, tree arg1, tree arg2, tree arg3,
     case TRUTH_ORIF_EXPR:
     case TRUTH_AND_EXPR:
     case TRUTH_OR_EXPR:
-      warn_logical_operator (input_location, code,
+      warn_logical_operator (input_location, code, boolean_type_node,
 			     code_orig_arg1, arg1, code_orig_arg2, arg2);
       /* Fall through.  */
     case PLUS_EXPR:
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 1a1a80f..4b8fe38 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -119,10 +119,10 @@ static int simple_operand_p (const_tree);
 static tree range_binop (enum tree_code, tree, tree, int, tree, int);
 static tree range_predecessor (tree);
 static tree range_successor (tree);
-static tree make_range (tree, int *, tree *, tree *, bool *);
-static tree build_range_check (tree, tree, int, tree, tree);
-static int merge_ranges (int *, tree *, tree *, int, tree, tree, int, tree,
-			 tree);
+extern tree make_range (tree, int *, tree *, tree *, bool *);
+extern tree build_range_check (tree, tree, int, tree, tree);
+extern bool merge_ranges (int *, tree *, tree *, int, tree, tree, int,
+			  tree, tree);
 static tree fold_range_test (enum tree_code, tree, tree, tree);
 static tree fold_cond_expr_with_comparison (tree, tree, tree, tree);
 static tree unextend (tree, int, int, tree);
@@ -4414,7 +4414,7 @@ range_binop (enum tree_code code, tree type, tree arg0, int upper0_p,
    because signed overflow is undefined; otherwise, do not change
    *STRICT_OVERFLOW_P.  */
 
-static tree
+tree
 make_range (tree exp, int *pin_p, tree *plow, tree *phigh,
 	    bool *strict_overflow_p)
 {
@@ -4706,7 +4706,7 @@ make_range (tree exp, int *pin_p, tree *plow, tree *phigh,
    type, TYPE, return an expression to test if EXP is in (or out of, depending
    on IN_P) the range.  Return 0 if the test couldn't be created.  */
 
-static tree
+tree
 build_range_check (tree type, tree exp, int in_p, tree low, tree high)
 {
   tree etype = TREE_TYPE (exp), value;
@@ -4877,7 +4877,7 @@ range_successor (tree val)
 /* Given two ranges, see if we can merge them into one.  Return 1 if we
    can, 0 if we can't.  Set the output range into the specified parameters.  */
 
-static int
+bool
 merge_ranges (int *pin_p, tree *plow, tree *phigh, int in0_p, tree low0,
 	      tree high0, int in1_p, tree low1, tree high1)
 {
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 2f3cb4e..3a8f4ab 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2009-05-15  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
+	PR 16302
+	* gcc.dg/pr16302.c: New.
+	* g++.dg/warn/pr16302.C: New.
+
 2009-05-15  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>
 
 	* gcc.dg/torture/builtin-math-5.c: New.
diff --git a/gcc/testsuite/g++.dg/warn/pr16302.C b/gcc/testsuite/g++.dg/warn/pr16302.C
new file mode 100644
index 0000000..a6f1a45
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr16302.C
@@ -0,0 +1,76 @@
+// PR 16302
+/* { dg-do compile } */
+/* { dg-options "-Wlogical-op" } */
+void bar (int);
+int
+foo (int argc, char *argv[])
+{
+  if (argc != 1 || argc != 2) return 1; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  if (argc < 0 && argc > 10) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  if (argc || !argc) return 1; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  if (argc && !argc) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc != 1 || argc != 2); /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  bar (argc < 0 && argc > 10); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc || !argc); /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  bar (argc && !argc); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc != 1 || argc != 2) ? 1 : 0 ; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  return (argc < 0 && argc > 10) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc || !argc) ? 1 : 0; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  return (argc && !argc) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+
+  if (argc == 2 && argc == 1) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  if (argc < 0 && argc > 10) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  if (argc || !argc) return 1; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  if (argc && !argc) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc == 2 && argc == 1); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc < 0 && argc > 10); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc || !argc); /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  bar (argc && !argc); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc == 2 && argc == 1) ? 1 : 0 ; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc < 0 && argc > 10) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc || !argc) ? 1 : 0; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  return (argc && !argc) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+
+  if (argc == 2 && argc == 1) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  if (argc < 0 && argc > 10) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  if (!argc || argc) return 1; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  if (!argc && argc) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc == 2 && argc == 1); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc < 0 && argc > 10); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (!argc || argc); /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  bar (!argc && argc); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc == 2 && argc == 1) ? 1 : 0 ; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc < 0 && argc > 10) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (!argc || argc) ? 1 : 0; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  return (!argc && argc) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+
+  return 0;
+}
+
+int
+foo2 (int argc)
+{
+  if (5 != 1 || 5 != 2) return 1;
+  if (5 < 0 && 5 > 10) return 1;
+  if (1 || 0) return 1;
+  if (0 && 1) return 1;
+  if (2 || !2) return 1;
+  if (2 && !2) return 1;
+  if (!(!2) || !(2)) return 1;
+  if (!(!2) && !(2)) return 1;
+  bar (5 != 1 || 5 != 2);
+  bar (5 < 0 && 5 > 10);
+  bar (1 || 0);
+  bar (0 && 1);
+  bar (2 || !2);
+  bar (2 && !2);
+  return (5 != 1 || 5 != 2) ? 1 : 0 ;
+  return (5 < 0 && 5 > 10) ? 1 : 0;
+  return (1 || 0) ? 1 : 0 ;
+  return (0 && 1) ? 1 : 0;
+  return (2 || !2) ? 1 : 0;
+  return (2 && !2) ? 1 : 0;
+
+  return 0;
+}
+
diff --git a/gcc/testsuite/gcc.dg/pr16302.c b/gcc/testsuite/gcc.dg/pr16302.c
new file mode 100644
index 0000000..0daa513
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr16302.c
@@ -0,0 +1,76 @@
+/* PR 16302 */
+/* { dg-do compile } */
+/* { dg-options "-Wlogical-op" } */
+void bar (int);
+int
+foo (int argc, char *argv[])
+{
+  if (argc != 1 || argc != 2) return 1; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  if (argc < 0 && argc > 10) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  if (argc || !argc) return 1; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  if (argc && !argc) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc != 1 || argc != 2); /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  bar (argc < 0 && argc > 10); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc || !argc); /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  bar (argc && !argc); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc != 1 || argc != 2) ? 1 : 0 ; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  return (argc < 0 && argc > 10) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc || !argc) ? 1 : 0; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  return (argc && !argc) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+
+  if (argc == 2 && argc == 1) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  if (argc < 0 && argc > 10) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  if (argc || !argc) return 1; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  if (argc && !argc) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc == 2 && argc == 1); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc < 0 && argc > 10); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc || !argc); /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  bar (argc && !argc); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc == 2 && argc == 1) ? 1 : 0 ; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc < 0 && argc > 10) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc || !argc) ? 1 : 0; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  return (argc && !argc) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+
+  if (argc == 2 && argc == 1) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  if (argc < 0 && argc > 10) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  if (!argc || argc) return 1; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  if (!argc && argc) return 1; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc == 2 && argc == 1); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (argc < 0 && argc > 10); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  bar (!argc || argc); /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  bar (!argc && argc); /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc == 2 && argc == 1) ? 1 : 0 ; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (argc < 0 && argc > 10) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+  return (!argc || argc) ? 1 : 0; /* { dg-warning "'or' of collectively exhaustive tests is always true" } */
+  return (!argc && argc) ? 1 : 0; /* { dg-warning "'and' of mutually exclusive tests is always false" } */
+
+  return 0;
+}
+
+int
+foo2 (int argc)
+{
+  if (5 != 1 || 5 != 2) return 1;
+  if (5 < 0 && 5 > 10) return 1;
+  if (1 || 0) return 1;
+  if (0 && 1) return 1;
+  if (2 || !2) return 1;
+  if (2 && !2) return 1;
+  if (!(!2) || !(2)) return 1;
+  if (!(!2) && !(2)) return 1;
+  bar (5 != 1 || 5 != 2);
+  bar (5 < 0 && 5 > 10);
+  bar (1 || 0);
+  bar (0 && 1);
+  bar (2 || !2);
+  bar (2 && !2);
+  return (5 != 1 || 5 != 2) ? 1 : 0 ;
+  return (5 < 0 && 5 > 10) ? 1 : 0;
+  return (1 || 0) ? 1 : 0 ;
+  return (0 && 1) ? 1 : 0;
+  return (2 || !2) ? 1 : 0;
+  return (2 && !2) ? 1 : 0;
+
+  return 0;
+}
+
diff --git a/gcc/tree.h b/gcc/tree.h
index 52c4fdf..66d4e25 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4820,6 +4820,10 @@ extern bool is_builtin_name (const char*);
 extern int get_object_alignment (tree, unsigned int, unsigned int);
 extern tree fold_call_stmt (gimple, bool);
 extern tree gimple_fold_builtin_snprintf_chk (gimple, tree, enum built_in_function);
+extern tree make_range (tree, int *, tree *, tree *, bool *);
+extern tree build_range_check (tree, tree, int, tree, tree);
+extern bool merge_ranges (int *, tree *, tree *, int, tree, tree, int, 
+			  tree, tree);
 
 /* In convert.c */
 extern tree strip_float_extensions (tree);