From 6ac7e79109ec07a2c5dea00be2974734c7fadff0 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 19 Nov 2015 10:51:35 -0500
Subject: [PATCH 32/35] FIXME: experiments with a -Wmissing-break switch
 warning

---
 gcc/c-family/c-common.c                     |  96 ++++++++++++++++++++++++++
 gcc/c-family/c-common.h                     |   2 +
 gcc/c-family/c.opt                          |   4 ++
 gcc/c/c-parser.c                            |   9 ++-
 gcc/cp/semantics.c                          |   3 +
 gcc/testsuite/c-c++-common/Wmissing-break.c | 100 ++++++++++++++++++++++++++++
 6 files changed, 213 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wmissing-break.c

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 146e805..a87258e 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -12878,4 +12878,100 @@ diagnose_mismatched_attributes (tree olddecl, tree newdecl)
   return warned;
 }
 
+/* FIXME.  */
+
+static void
+validate_fallthrough (tree from_case, auto_vec<tree> &stmts_in_from_case,
+		      tree to_case)
+{
+  gcc_assert (TREE_CODE (from_case) == CASE_LABEL_EXPR);
+  gcc_assert (TREE_CODE (to_case) == CASE_LABEL_EXPR);
+
+  /* Don't complain about falling through empty cases. */
+  if (stmts_in_from_case.length () == 0)
+    return;
+
+  if (warning_at (EXPR_LOCATION (from_case),
+		  OPT_Wmissing_break,
+		  "control flow falls through from this %<case%> to..."))
+    inform (EXPR_LOCATION (to_case),
+	    "...this %<case%>; did you mean to add a %<break%>?");
+}
+
+/* FIXME.  */
+#if 0
+class case_
+{
+ public:
+  case_ (tree case_stmt) : m_case_stmt (case_stmt)
+  {
+    gcc_assert (TREE_CODE (case_stmt) == CASE_LABEL_EXPR);
+  }
+
+ private:
+  tree m_case_stmt;
+  auto_vec<tree> m_stmts;
+};
+#endif
+
+/* FIXME.  */
+
+void
+validate_switch (tree body)
+{
+  //debug_tree (body);
+
+  gcc_assert (TREE_CODE (body) == STATEMENT_LIST);
+
+#if 1
+  tree_stmt_iterator iter;
+  tree cur_case = NULL_TREE;
+  auto_vec<tree> stmts_in_cur_case;
+#if 0
+  case_ cur_case = NULL;
+  case_ last_case = NULL;
+#endif
+  for (iter = tsi_start (body); !tsi_end_p (iter); tsi_next (&iter))
+    {
+      tree stmt = tsi_stmt (iter);
+      //debug_tree (stmt);
+      switch (TREE_CODE (stmt))
+	{
+	case CASE_LABEL_EXPR:
+	  {
+	    if (cur_case)
+	      validate_fallthrough (cur_case, stmts_in_cur_case, stmt);
+	    cur_case = stmt;
+	    stmts_in_cur_case.truncate (0);
+	    //inform (EXPR_LOCATION (stmt), "case label");
+#if 0
+	    last_case = cur_case;
+	    cur_case = new case_ (stmt); // FIXME: memory management???
+#endif
+	  }
+	  break;
+
+	/* The C++ frontend generates explicit BREAK_STMT nodes;
+	   the C frontend generates GOTO_EXPR nodes.  */
+	case BREAK_STMT:
+	case GOTO_EXPR:
+	  {
+	    //inform (EXPR_LOCATION (stmt), "goto_expr");
+	    //last_case = cur_case;
+	    cur_case = NULL_TREE;
+	    stmts_in_cur_case.truncate (0);
+	  }
+	  break;
+
+	default:
+	  if (cur_case)
+	    stmts_in_cur_case.safe_push (stmt);
+	  //inform (EXPR_LOCATION (stmt), "stmt");
+	  break;
+	}
+    }
+#endif
+
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 0ee9f56..c19cae3 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1487,4 +1487,6 @@ extern bool cilk_recognize_spawn (tree, tree *);
    results.  Returns -1 if SOURCE_DATE_EPOCH is not defined.  */
 extern time_t get_source_date_epoch (void);
 
+extern void validate_switch (tree);
+
 #endif /* ! GCC_C_COMMON_H */
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 918df16..c7e3034 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -581,6 +581,10 @@ Wmissing-braces
 C ObjC C++ ObjC++ Var(warn_missing_braces) Warning LangEnabledBy(C ObjC,Wall)
 Warn about possibly missing braces around initializers.
 
+Wmissing-break
+C ObjC C++ ObjC++ Var(warn_misleading_break) Warning
+Warn about possibly missing break statements within switch statements.
+
 Wmissing-declarations
 C ObjC C++ ObjC++ Var(warn_missing_declarations) Warning
 Warn about global functions without previous declarations.
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 6523c08..7bebf72 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -58,6 +58,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/c-indentation.h"
 #include "gimple-expr.h"
 #include "context.h"
+#include "print-tree.h"
 
 /* We need to walk over decls with incomplete struct/union/enum types
    after parsing the whole translation unit.
@@ -5613,9 +5614,15 @@ c_parser_switch_statement (c_parser *parser, bool *if_p)
       tree t = build1 (LABEL_EXPR, void_type_node, c_break_label);
       SET_EXPR_LOCATION (t, here);
       add_stmt (t);
+      //inform (here, "here");
     }
   c_break_label = save_break;
-  add_stmt (c_end_compound_stmt (switch_loc, block, flag_isoc99));
+  tree switch_stmt = c_end_compound_stmt (switch_loc, block, flag_isoc99);
+  add_stmt (switch_stmt);
+  //inform (switch_loc, "switch_loc");
+  //inform (switch_cond_loc, "switch_cond_loc");
+  if (warn_misleading_break)
+    validate_switch (body);
   c_parser_maybe_reclassify_token (parser);
 }
 
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 06dee5a..b897a95 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -1186,6 +1186,9 @@ finish_switch_stmt (tree switch_stmt)
   scope = SWITCH_STMT_SCOPE (switch_stmt);
   SWITCH_STMT_SCOPE (switch_stmt) = NULL;
   add_stmt (do_poplevel (scope));
+
+  if (warn_misleading_break)
+    validate_switch (SWITCH_STMT_BODY (switch_stmt));
 }
 
 /* Begin a try-block.  Returns a newly-created TRY_BLOCK if
diff --git a/gcc/testsuite/c-c++-common/Wmissing-break.c b/gcc/testsuite/c-c++-common/Wmissing-break.c
new file mode 100644
index 0000000..a24199d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wmissing-break.c
@@ -0,0 +1,100 @@
+/* { dg-options "-Wmissing-break" } */
+/*
+  Compare with https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02198.html
+  aka "[PATCH, PR target/68405, i386, committed] Add missing break"
+    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68405
+  Looks like this one was seen by cppcheck.
+ */
+int
+test_1 (int i, int k)
+{
+  int j;
+
+  switch (i)
+    {
+    case 0: /* { dg-warning "control flow falls through" } */
+      j = 3; 
+    
+    case 1: /* { dg-message "did you mean to add a .break." } */
+      j = 5;
+      break;
+
+    default:
+      j = i * 2;
+      break;
+    }
+
+  return j + k;
+}
+
+int
+test_2 (int i, int k)
+{
+  int j;
+
+  switch (i)
+    {
+    case 0: /* We shouldn't warn here. */
+    case 1:
+      j = 5;
+      break;
+
+    default:
+      j = i * 2;
+      break;
+    }
+
+  return j + k;
+}
+
+int
+test_3 (int i, int k)
+{
+  int j;
+  switch (i)
+    {
+    case 0: /* { dg-warning "control flow falls through" } */
+      j = 3;
+
+    case 1: /* { dg-message "did you mean to add a .break." } */
+    case 2:
+      j = 5;
+      break;
+    }
+     
+  return j + k; 
+}
+
+#if 0
+int
+test_4 (short *to, short *from, int count)
+{
+    int n = (count + 7) / 8;
+    switch (count % 8) {
+    case 0: do { *to = *from++;
+    case 7:      *to = *from++;
+    case 6:      *to = *from++;
+    case 5:      *to = *from++;
+    case 4:      *to = *from++;
+    case 3:      *to = *from++;
+    case 2:      *to = *from++;
+    case 1:      *to = *from++;
+            } while (--n > 0);
+    }
+}
+
+int
+test_5 (int i, int k)
+{
+  switch (i)
+    {
+    case 0:
+      if (k > 3)
+	return 2;
+      /* We shouldn't warn about this fallthrough.  */
+    case 1:
+      return k * 4;
+    }
+  return k;
+}
+#endif
-- 
1.8.5.3

