c-typeck.c (check_modify_expr): New function.

	* c-typeck.c (check_modify_expr): New function.
	(build_modify_expr): Call it if warn_sequence_point.
	* c-decl.c (warn_sequence_point): New variable.
	(c_decode_option): Handle -Wsequence-point and
	-Wno-sequence-point.  Enable -Wsequence-point as part of -Wall.
	* c-tree.h (warn_sequence_point): Declare.
	* invoke.texi (-Wsequence-point): Document.
	* toplev.c (documented_lang_options): Add -Wsequence-point and
	-Wno-sequence-point.
	Original work by Michael Meeks, 16 Jun 1998.

testsuite:
	* gcc.dg/sequence-pt-1.c: New test.

From-SVN: r36840

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 52e180f1c4d9b6a260b3e8e114d8eb96dc6569b0..2d4e90b29165e5e1184d4bd5bc9cf6ac96ee7b7b 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,17 @@
+2000-10-11  Michael Meeks  <mmeeks@gnu.org>
+            Joseph S. Myers  <jsm28@cam.ac.uk>
+
+       * c-typeck.c (check_modify_expr): New function.
+       (build_modify_expr): Call it if warn_sequence_point.
+       * c-decl.c (warn_sequence_point): New variable.
+       (c_decode_option): Handle -Wsequence-point and
+       -Wno-sequence-point.  Enable -Wsequence-point as part of -Wall.
+       * c-tree.h (warn_sequence_point): Declare.
+       * invoke.texi (-Wsequence-point): Document.
+       * toplev.c (documented_lang_options): Add -Wsequence-point and
+       -Wno-sequence-point.
+       Original work by Michael Meeks, 16 Jun 1998.
+
 Wed Oct 11 06:15:41 2000  Richard Kenner  <kenner@vlsi1.ultra.nyu.edu>
 
        * tree.c (get_narrower): Don't look at precision of field if

diff --git a/gcc/c-decl.c b/gcc/c-decl.c
index 39e0b4a978c46971ecf443a1a9d6df26ab746b27..de45da782130ed7e8c995cd0bcfc5a19ba548d97 100644 (file)
--- a/gcc/c-decl.c
+++ b/gcc/c-decl.c
@@ -497,6 +497,10 @@ int warn_float_equal = 0;
 
 int warn_multichar = 1;
 
+/* Nonzero means warn about possible violations of sequence point rules.  */
+
+int warn_sequence_point;
+
 /* The variant of the C language being processed.  */
 
 c_language_kind c_language = clk_c;
@@ -765,6 +769,10 @@ c_decode_option (argc, argv)
     warn_return_type = 1;
   else if (!strcmp (p, "-Wno-return-type"))
     warn_return_type = 0;
+  else if (!strcmp (p, "-Wsequence-point"))
+    warn_sequence_point = 1;
+  else if (!strcmp (p, "-Wno-sequence-point"))
+    warn_sequence_point = 0;
   else if (!strcmp (p, "-Wcomment"))
     ; /* cpp handles this one.  */
   else if (!strcmp (p, "-Wno-comment"))
@@ -826,6 +834,7 @@ c_decode_option (argc, argv)
       warn_format = 1;
       warn_char_subscripts = 1;
       warn_parentheses = 1;
+      warn_sequence_point = 1;
       warn_missing_braces = 1;
       /* We set this to 2 here, but 1 in -Wmain, so -ffreestanding can turn
         it off only if it's not explicit.  */

diff --git a/gcc/c-tree.h b/gcc/c-tree.h
index b8adce15454f6aae02dc5b472f3bd7901ead2d67..a936b22438d8ecb1c6b4db865415bfef773f5ef5 100644 (file)
--- a/gcc/c-tree.h
+++ b/gcc/c-tree.h
@@ -366,6 +366,10 @@ extern int warn_missing_braces;
 
 extern int warn_sign_compare;
 
+/* Warn about possible violations of sequence point rules.  */
+
+extern int warn_sequence_point;
+
 /* Warn about testing equality of floating point numbers. */
 
 extern int warn_float_equal;

diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c
index 9f2ec8d1e78235ffdd682cb3243caad919d45201..d40aee19f66e1a7a5a25e268f29f73e9b538af21 100644 (file)
--- a/gcc/c-typeck.c
+++ b/gcc/c-typeck.c
@@ -61,6 +61,7 @@ static tree pointer_diff              PARAMS ((tree, tree));
 static tree unary_complex_lvalue       PARAMS ((enum tree_code, tree));
 static void pedantic_lvalue_warning    PARAMS ((enum tree_code));
 static tree internal_build_compound_expr PARAMS ((tree, int));
+static void check_modify_expr          PARAMS ((tree, tree));
 static tree convert_for_assignment     PARAMS ((tree, tree, const char *,
                                                 tree, tree, int));
 static void warn_for_assignment                PARAMS ((const char *, const char *,
@@ -3814,6 +3815,132 @@ build_c_cast (type, expr)
 
   return value;
 }
+\f
+/* Recursive check for expressions that break the sequence point rules
+   and so have undefined semantics (e.g. n = n++).  FIXME: if walk_tree
+   gets moved out of the C++ front end, this should probably be moved
+   to code shared between the front ends and use walk_tree.  */
+static void
+check_modify_expr (lhs, rhs)
+     tree lhs, rhs;
+{
+  tree identifier_name;   /* A VAR_DECL name on the LHS that could
+                            be the same as one on the RHS. */
+  identifier_name = NULL_TREE;
+
+  if ((lhs == NULL_TREE) || (rhs == NULL_TREE))
+    return;
+
+  switch (TREE_CODE (rhs))
+    {
+    case ERROR_MARK:
+      return;
+    case VAR_DECL:
+    case PARM_DECL:
+      identifier_name = DECL_NAME (rhs);
+      break;
+    case PREDECREMENT_EXPR:
+    case PREINCREMENT_EXPR:
+    case POSTDECREMENT_EXPR:
+    case POSTINCREMENT_EXPR:
+      {
+       tree var_decl = TREE_OPERAND (rhs, 0);
+       if (TREE_CODE (var_decl) == VAR_DECL
+           || TREE_CODE (var_decl) == PARM_DECL)
+         identifier_name = DECL_NAME (var_decl);
+      }
+      break;
+    case TREE_LIST:
+      {
+       tree parm = TREE_CHAIN (rhs);
+       /* Now scan all the list, e.g. indices of multi dimensional array.  */
+       while (parm)
+         {
+           check_modify_expr (lhs, TREE_VALUE (parm));
+           parm = TREE_CHAIN (parm);
+         }
+      }
+      return;
+    case NOP_EXPR:
+    case CONVERT_EXPR:
+    case NON_LVALUE_EXPR:
+      check_modify_expr (lhs, TREE_OPERAND (rhs, 0));
+      return;
+    case MODIFY_EXPR:
+      /* First check for form a = b = a++ by checking RHS.  */
+      check_modify_expr (lhs, TREE_OPERAND (rhs, 1));
+      /* Then check for a = (a = 1) + 2 and a = b[a++] = c.  */
+      if (TREE_CODE (TREE_OPERAND (rhs, 0)) == VAR_DECL
+         || TREE_CODE (TREE_OPERAND (rhs, 0)) == PARM_DECL)
+       {
+         identifier_name = DECL_NAME (TREE_OPERAND (rhs, 0));
+         break;
+       }
+      else
+       {
+         check_modify_expr (lhs, TREE_OPERAND (rhs, 0));
+         return;
+       }
+    default:
+      /* We don't know what to do... pray check_modify_expr removes
+        loops in the tree.  */
+      switch (TREE_CODE_CLASS (TREE_CODE (rhs)))
+       {
+       case 'r':
+       case '<':
+       case '2':
+       case 'b':
+       case '1':
+       case 'e':
+       case 's':
+       case 'x':
+         {
+           int lp;
+           int max = first_rtl_op (TREE_CODE (rhs));
+           for (lp = 0; lp < max; lp++)
+             check_modify_expr (lhs, TREE_OPERAND (rhs, lp));
+           return;
+         }
+       default:
+         return;
+       }
+      break;
+    }
+  if (identifier_name != NULL_TREE)
+    {
+      switch (TREE_CODE (lhs))
+       {
+       case ERROR_MARK:
+         return;
+         /* Perhaps this variable was incremented on the RHS.  */
+       case VAR_DECL:
+       case PARM_DECL:
+         if (TREE_CODE (rhs) != VAR_DECL && TREE_CODE (rhs) != PARM_DECL)
+           if (DECL_NAME (lhs) == identifier_name)
+             warning ("operation on `%s' may be undefined",
+                      IDENTIFIER_POINTER (DECL_NAME (lhs)));
+         break;
+       case PREDECREMENT_EXPR:
+       case PREINCREMENT_EXPR:
+       case POSTDECREMENT_EXPR:
+       case POSTINCREMENT_EXPR:
+         {
+           tree var_decl = TREE_OPERAND (lhs, 0);
+           if (TREE_CODE (var_decl) == VAR_DECL
+               || TREE_CODE (var_decl) == PARM_DECL)
+             if (identifier_name == DECL_NAME (var_decl))
+               warning ("operation on `%s' may be undefined",
+                        IDENTIFIER_POINTER (DECL_NAME (var_decl)));
+         }
+         break;
+       default:
+         /* To save duplicating tree traversal code swap args, and recurse.  */
+         check_modify_expr (rhs, lhs);
+         break;
+       }
+    }
+}
+
 \f
 /* Build an assignment expression of lvalue LHS from value RHS.
    MODIFYCODE is the code for a binary operator that we use
@@ -3969,6 +4096,11 @@ build_modify_expr (lhs, modifycode, rhs)
   if (TREE_CODE (newrhs) == ERROR_MARK)
     return error_mark_node;
 
+  if (warn_sequence_point)
+    check_modify_expr (lhs, rhs);
+
+  /* Scan operands */
+
   result = build (MODIFY_EXPR, lhstype, lhs, newrhs);
   TREE_SIDE_EFFECTS (result) = 1;
 

diff --git a/gcc/invoke.texi b/gcc/invoke.texi
index ce719542e0c3f7fa2eb2ae4d536fbac0b3e7670c..d7320d187f3ad0ba7ceb4bf3c37d8573e740b430 100644 (file)
--- a/gcc/invoke.texi
+++ b/gcc/invoke.texi
@@ -1559,6 +1559,52 @@ the enclosing @code{if}.  The resulting code would look like this:
 @}
 @end smallexample
 
+@item -Wsequence-point
+Warn about code that may have undefined semantics because of violations
+of sequence point rules in the C standard.
+
+The C standard defines the order in which expressions in a C program are
+evaluated in terms of @dfn{sequence points}, which represent a partial
+ordering between the execution of parts of the program: those executed
+before the sequence point, and those executed after it.  These occur
+after the evaluation of a full expression (one which is not part of a
+larger expression), after the evaluation of the first operand of a
+@code{&&}, @code{||}, @code{? :} or @code{,} (comma) operator, before a
+function is called (but after the evaluation of its arguments and the
+expression denoting the called function), and in certain other places.
+Other than as expressed by the sequence point rules, the order of
+evaluation of subexpressions of an expression is not specified.  All
+these rules describe only a partial order rather than a total order,
+since, for example, if two functions are called within one expression
+with no sequence point between them, the order in which the functions
+are called is not specified.  However, the standards committee have
+ruled that function calls do not overlap.
+
+It is not specified when between sequence points modifications to the
+values of objects take effect.  Programs whose behavior depends on this
+have undefined behavior; the C standard specifies that ``Between the
+previous and next sequence point an object shall have its stored value
+modified at most once by the evaluation of an expression.  Furthermore,
+the prior value shall be read only to determine the value to be
+stored.''.  If a program breaks these rules, the results on any
+particular implementation are entirely unpredictable.
+
+Examples of code with undefined behavior are @code{a = a++;}, @code{a[n]
+= b[n++]} and @code{a[i++] = i;}.  Some more complicated cases are not
+diagnosed by this option, and it may give an occasional false positive
+result, but in general it has been found fairly effective at detecting
+this sort of problem in programs.
+
+The present implementation of this option only works for C programs.  A
+future implementation may also work for C++ programs.
+
+There is some controversy over the precise meaning of the sequence point
+rules in subtle cases.  Alternative formal definitions may be found in
+Clive Feather's ``Annex S''
+@uref{http://wwwold.dkuug.dk/JTC1/SC22/WG14/www/docs/n908.htm} and in
+Michael Norrish's thesis
+@uref{http://www.cl.cam.ac.uk/users/mn200/PhD/thesis-report.ps.gz}.
+
 @item -Wreturn-type
 Warn whenever a function is defined with a return-type that defaults to
 @code{int}.  Also warn about any @code{return} statement with no

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 1f1578635e134fa2127d98340a50242590e1d719..c24008514d6b37303d003869bf0763fbd783a65a 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2000-10-11  Joseph S. Myers  <jsm28@cam.ac.uk>
+
+       * gcc.dg/sequence-pt-1.c: New test.
+
 2000-10-11  Geoff Keating  <geoffk@cygnus.com>
 
        * gcc.c-torture/execute/20001011-1.c: New testcase.

diff --git a/gcc/testsuite/gcc.dg/sequence-pt-1.c b/gcc/testsuite/gcc.dg/sequence-pt-1.c
new file mode 100644 (file)
index 0000000..f5738b9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/sequence-pt-1.c
@@ -0,0 +1,42 @@
+/* Test for sequence point warnings.  */
+/* Origin: Michael Meeks in
+   <URL:http://gcc.gnu.org/ml/gcc-patches/1998-06/msg00316.html>,
+   adapted to a testcase by Joseph Myers <jsm28@cam.ac.uk>.  */
+/* { dg-do compile } */
+/* { dg-options "-Wsequence-point" } */
+
+struct s
+{
+  int a;
+};
+
+extern int fn (int);
+extern int sprintf (char *, const char *, ...);
+
+void
+foo (int a, int b, int n, int p, int *ptr, struct s *sptr,
+     int *ap, int *bp, int **cp, char *ans)
+{
+  int len;
+  a = a++; /* { dg-warning "undefined" "sequence point warning" } */
+  a = --a; /* { dg-warning "undefined" "sequence point warning" } */
+  a = ++a + b; /* { dg-warning "undefined" "sequence point warning" } */
+  a = a-- + b; /* { dg-warning "undefined" "sequence point warning" } */
+  a = (a++ && 4); /* { dg-bogus "undefined" "bogus sequence point warning" { xfail *-*-* } } */
+  ap[n] = bp[n++]; /* { dg-warning "undefined" "sequence point warning" } */
+  ap[--n] = bp[n]; /* { dg-warning "undefined" "sequence point warning" } */
+  ap[++n] = bp[--n]; /* { dg-warning "undefined" "sequence point warning" } */
+  cp[n][n] = cp[n][n]++; /* { dg-warning "undefined" "sequence point warning" { xfail *-*-* } } */
+  cp[n][p] = cp[n][n++]; /* { dg-warning "undefined" "sequence point warning" } */
+  *ptr++ = (int)ptr++; /* { dg-warning "undefined" "sequence point warning" } */
+  sptr->a = sptr->a++; /* { dg-warning "undefined" "sequence point warning" { xfail *-*-* } } */
+  sptr->a = (int)(sptr++); /* { dg-warning "undefined" "sequence point warning" } */
+  len = sprintf (ans, "%d", len++); /* { dg-bogus "undefined" "bogus sequence point warning" { xfail *-*-* } } */
+  *ptr++ = fn (*ptr); /* { dg-warning "undefined" "sequence point warning" { xfail *-*-* } } */
+  a = b = a++; /* { dg-warning "undefined" "sequence point warning" } */
+  b = a = --b; /* { dg-warning "undefined" "sequence point warning" } */
+  a = 1 + (a = 1); /* { dg-warning "undefined" "sequence point warning" } */
+  a = (a = b); /* { dg-warning "undefined" "sequence point warning" } */
+  a = (a = b) + 1; /* { dg-warning "undefined" "sequence point warning" } */
+  a = (bp[a++] = b) + 1; /* { dg-warning "undefined" "sequence point warning" } */
+}

diff --git a/gcc/toplev.c b/gcc/toplev.c
index 4e7ec89b96078ed3bb806c84b5c4014597c9c741..fb7c71bcb3aeb8d97825cd96dbe4b796f2c07472 100644 (file)
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1247,6 +1247,9 @@ documented_lang_options[] =
   { "-Wno-nested-externs", "" },
   { "-Wparentheses", "Warn about possible missing parentheses" },
   { "-Wno-parentheses", "" },
+  { "-Wsequence-point",
+    "Warn about possible violations of sequence point rules" },
+  { "-Wno-sequence-point", "" },
   { "-Wpointer-arith", "Warn about function pointer arithmetic" },
   { "-Wno-pointer-arith", "" },
   { "-Wredundant-decls",