From 93f7f1968916f2bc5ba39d39b57047e7a3c1b298 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 6 Mar 2019 12:34:11 -0500
Subject: [PATCH 064/161] FIXME: implement pending_diagnostic objects

---
 gcc/analyzer/engine.cc                         |   9 +-
 gcc/analyzer/hardcoded-state-machine-file.cc   |  50 ++++++++--
 gcc/analyzer/hardcoded-state-machine-malloc.cc | 132 +++++++++++++++++++------
 gcc/analyzer/hardcoded-state-machine-taint.cc  |  28 ++++--
 gcc/analyzer/hardcoded-state-machine.h         |  24 ++++-
 gcc/diagnostic-core.h                          |   2 +
 gcc/diagnostic.c                               |  18 ++++
 7 files changed, 215 insertions(+), 48 deletions(-)

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index ea1f0f2..5ff6330 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -678,10 +678,10 @@ public:
 
   void warn_for_state (const supernode *node, const gimple *stmt,
 		       tree var, state_machine::state_t state,
-		       const metadata *m,
-		       const char *fmt, ...) FINAL OVERRIDE
+		       pending_diagnostic *d) FINAL OVERRIDE
   {
     JIT_LOG_FUNC (m_engine->get_logger ());
+    gcc_assert (d);
 
     var_state_t vs_from (var, state);
     if (const_cast <sm_instance *> (m_old_sm_inst)->m_active_vars.m_set.contains (vs_from))
@@ -712,17 +712,22 @@ public:
 	/* Push/pop cfun to ensure we get "In function " announcements
 	   from the diagnostics subsystem.  */
 	push_cfun (node->m_fun);
+	d->emit (&rich_loc);
+#if 0
 	va_list ap;
 	va_start (ap, fmt);
 	emit_diagnostic_valist (DK_WARNING, &rich_loc, m, 0, fmt,
 				&ap);
 	va_end (ap);
+#endif
 	pop_cfun ();
       }
     else
       if (m_engine->get_logger ())
 	m_engine->log ("not in state %s; no error",
 		       m_engine->m_sm.get_state_name (state));
+
+    delete d;
   }
 
 private:
diff --git a/gcc/analyzer/hardcoded-state-machine-file.cc b/gcc/analyzer/hardcoded-state-machine-file.cc
index 5e0ae3b..8d74298 100644
--- a/gcc/analyzer/hardcoded-state-machine-file.cc
+++ b/gcc/analyzer/hardcoded-state-machine-file.cc
@@ -40,6 +40,42 @@ static const bool DEBUG = false;
 
 ////////////////////////////////////////////////////////////////////////////
 
+class double_fclose : public pending_diagnostic
+{
+public:
+  double_fclose (tree arg) : m_arg (arg) {}
+
+  void emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    warning_at (rich_loc, 0,
+		"double %<fclose%> of FILE %qE",
+		m_arg);
+  }
+
+private:
+  tree m_arg;
+};
+
+class file_leak : public pending_diagnostic
+{
+public:
+  file_leak (tree arg) : m_arg (arg) {}
+
+  void emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    metadata m;
+    m.add_cwe (775);
+    warning_at (rich_loc, 0,
+		"leak of FILE %qE",
+		m_arg);
+  }
+
+private:
+  tree m_arg;
+};
+
+////////////////////////////////////////////////////////////////////////////
+
 fileptr_state_machine::fileptr_state_machine (logger *logger)
 : state_machine ("file", logger)
 {
@@ -88,8 +124,8 @@ fileptr_state_machine::on_stmt (sm_context *sm_ctxt,
 
 	  sm_ctxt->on_transition (node, stmt , arg, m_nonnull, m_closed);
 
-	  sm_ctxt->warn_for_state (node, stmt, arg, m_closed, NULL,
-				   "double %<fclose%> of FILE %qE", arg);
+	  sm_ctxt->warn_for_state (node, stmt, arg, m_closed,
+				   new double_fclose (arg));
 	  sm_ctxt->on_transition (node, stmt, arg, m_closed, m_stop);
 	}
 
@@ -149,10 +185,8 @@ fileptr_state_machine::on_leak (sm_context *sm_ctxt,
 				tree var,
 				state_machine::state_t state) const
 {
-  metadata m;
-  m.add_cwe (775);
-  sm_ctxt->warn_for_state (node, stmt, var, m_unchecked, &m,
-			   "leak of FILE %qE", var);
-  sm_ctxt->warn_for_state (node, stmt, var, m_nonnull, &m,
-			   "leak of FILE %qE", var);
+  sm_ctxt->warn_for_state (node, stmt, var, m_unchecked,
+			   new file_leak (var));
+  sm_ctxt->warn_for_state (node, stmt, var, m_nonnull,
+			   new file_leak (var));
 }
diff --git a/gcc/analyzer/hardcoded-state-machine-malloc.cc b/gcc/analyzer/hardcoded-state-machine-malloc.cc
index d49d7d4..32bd22c 100644
--- a/gcc/analyzer/hardcoded-state-machine-malloc.cc
+++ b/gcc/analyzer/hardcoded-state-machine-malloc.cc
@@ -40,6 +40,91 @@ static const bool DEBUG = false;
 
 ////////////////////////////////////////////////////////////////////////////
 
+class double_free : public pending_diagnostic
+{
+public:
+  double_free (tree arg) : m_arg (arg) {}
+
+  void emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    metadata m;
+    m.add_cwe (415); /* CWE-415: Double Free.  */
+    warning_at (rich_loc, m, "double-free of %qE", m_arg);
+  }
+
+private:
+  tree m_arg;
+};
+
+class possible_null_deref : public pending_diagnostic
+{
+public:
+  possible_null_deref (tree arg) : m_arg (arg) {}
+
+  void emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    /* CWE-690: Unchecked Return Value to NULL Pointer Dereference.  */
+    metadata m;
+    m.add_cwe (690);
+    warning_at (rich_loc, m, "use of possibly-NULL %qE", m_arg);
+  }
+
+private:
+  tree m_arg;
+};
+
+class null_deref : public pending_diagnostic
+{
+public:
+  null_deref (tree arg) : m_arg (arg) {}
+
+  void emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    /* CWE-690: Unchecked Return Value to NULL Pointer Dereference.  */
+    metadata m;
+    m.add_cwe (690);
+    warning_at (rich_loc, m, "use of NULL %qE", m_arg);
+  }
+
+private:
+  tree m_arg;
+};
+
+class use_after_free : public pending_diagnostic
+{
+public:
+  use_after_free (tree arg) : m_arg (arg) {}
+
+  void emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    /* CWE-416: Use After Free.  */
+    metadata m;
+    m.add_cwe (416);
+    warning_at (rich_loc, m, "use after free of %qE", m_arg);
+  }
+
+private:
+  tree m_arg;
+};
+
+class malloc_leak : public pending_diagnostic
+{
+public:
+  malloc_leak (tree arg) : m_arg (arg) {}
+
+  void emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    metadata m;
+    m.add_cwe (401);
+    warning_at (rich_loc, m, "leak of %qE", m_arg);
+  }
+
+private:
+  tree m_arg;
+};
+
+////////////////////////////////////////////////////////////////////////////
+
 malloc_state_machine::malloc_state_machine (logger *logger)
 : state_machine ("malloc", logger)
 {
@@ -91,10 +176,8 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	     we don't want want to complain about double-free of NULL.  */
 
 	  /* free -> stop, with warning.  */
-	  metadata m;
-	  m.add_cwe (415); /* CWE-415: Double Free.  */
-	  sm_ctxt->warn_for_state (node, stmt, arg, m_free, &m,
-				   "double-free of %qE", arg);
+	  sm_ctxt->warn_for_state (node, stmt, arg, m_free,
+				   new double_free (arg));
 	  sm_ctxt->on_transition (node, stmt, arg, m_free, m_stop);
 	}
     }
@@ -128,26 +211,17 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	{
 	  tree arg = TREE_OPERAND (op, 0);
 
-	  {
-	    /* CWE-690: Unchecked Return Value to NULL Pointer Dereference.  */
-	    metadata m;
-	    m.add_cwe (690);
-	    sm_ctxt->warn_for_state (node, stmt, arg, m_unchecked, &m,
-				     "use of possibly-NULL %qE", arg);
-	    sm_ctxt->on_transition (node, stmt, arg, m_unchecked, m_nonnull);
-
-	    sm_ctxt->warn_for_state (node, stmt, arg, m_null, &m,
-				     "use of NULL %qE", arg);
-	    sm_ctxt->on_transition (node, stmt, arg, m_null, m_stop);
-	  }
-
-	  {
-	    metadata m;
-	    m.add_cwe (416); /* CWE-416: Use After Free.  */
-	    sm_ctxt->warn_for_state (node, stmt, arg, m_free, &m,
-				     "use after free of %qE", arg);
-	    sm_ctxt->on_transition (node, stmt, arg, m_free, m_stop);
-	  }
+	  sm_ctxt->warn_for_state (node, stmt, arg, m_unchecked,
+				   new possible_null_deref (arg));
+	  sm_ctxt->on_transition (node, stmt, arg, m_unchecked, m_nonnull);
+
+	  sm_ctxt->warn_for_state (node, stmt, arg, m_null,
+				   new null_deref (arg));
+	  sm_ctxt->on_transition (node, stmt, arg, m_null, m_stop);
+
+	  sm_ctxt->warn_for_state (node, stmt, arg, m_free,
+				   new use_after_free (arg));
+	  sm_ctxt->on_transition (node, stmt, arg, m_free, m_stop);
 	}
     }
 }
@@ -202,10 +276,8 @@ malloc_state_machine::on_leak (sm_context *sm_ctxt,
 			       tree var,
 			       state_machine::state_t state) const
 {
-  metadata m;
-  m.add_cwe (401);
-  sm_ctxt->warn_for_state (node, stmt, var, m_unchecked, &m,
-			   "leak of %qE", var);
-  sm_ctxt->warn_for_state (node, stmt, var, m_nonnull, &m,
-			   "leak of %qE", var);
+  sm_ctxt->warn_for_state (node, stmt, var, m_unchecked,
+			   new malloc_leak (var));
+  sm_ctxt->warn_for_state (node, stmt, var, m_nonnull,
+			   new malloc_leak (var));
 }
diff --git a/gcc/analyzer/hardcoded-state-machine-taint.cc b/gcc/analyzer/hardcoded-state-machine-taint.cc
index 6e0b1a1..682726c 100644
--- a/gcc/analyzer/hardcoded-state-machine-taint.cc
+++ b/gcc/analyzer/hardcoded-state-machine-taint.cc
@@ -40,6 +40,26 @@ static const bool DEBUG = false;
 
 ////////////////////////////////////////////////////////////////////////////
 
+class tainted_array_index : public pending_diagnostic
+{
+public:
+  tainted_array_index (tree arg) : m_arg (arg) {}
+
+  void emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    metadata m;
+    m.add_cwe (129);
+    warning_at (rich_loc, m,
+		"use of tainted value %qE in array lookup without bounds checking",
+		m_arg);
+  }
+
+private:
+  tree m_arg;
+};
+
+////////////////////////////////////////////////////////////////////////////
+
 taint_state_machine::taint_state_machine (logger *logger)
 : state_machine ("taint", logger)
 {
@@ -99,12 +119,8 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt,
       if (op == ARRAY_REF)
 	{
 	  tree arg = TREE_OPERAND (rhs1, 1);
-	  metadata m;
-	  m.add_cwe (129);
-	  sm_ctxt->warn_for_state
-	    (node, stmt, arg, m_tainted, &m,
-	     "use of tainted value %qE in array lookup without bounds checking",
-	     arg);
+	  sm_ctxt->warn_for_state  (node, stmt, arg, m_tainted,
+				    new tainted_array_index (arg));
 	  sm_ctxt->on_transition (node, stmt, arg, m_tainted, m_stop);
 	}
     }
diff --git a/gcc/analyzer/hardcoded-state-machine.h b/gcc/analyzer/hardcoded-state-machine.h
index 37e68e2..c109116 100644
--- a/gcc/analyzer/hardcoded-state-machine.h
+++ b/gcc/analyzer/hardcoded-state-machine.h
@@ -64,6 +64,27 @@ class metadata : public diagnostic_metadata
  private:
 };
 
+/* FIXME:
+   Diagnostics are emitted using va_list, which can't be portably stored
+   for later use, so we have to make it be a vfunc.  */
+
+class pending_diagnostic
+{
+ public:
+  virtual void emit (rich_location *) = 0;
+};
+
+#if 0
+/* A thing for state machines to pass to the engine to create
+   pending_diagnostic instances (so that we can filter without having 
+   to create pending_diagnostic instances first).  */
+
+class diagnostic_emitter
+{
+  virtual pending_diagnostic *make_diagnostic () = 0;
+};
+#endif
+
 /* Interface for the state machine to call the checker engine, at a
    particular stmt.  */
 
@@ -85,8 +106,7 @@ public:
   //virtual void transition (tree var, state_machine::state_t to) = 0;
   virtual void warn_for_state (const supernode *node, const gimple *stmt,
 			       tree var, state_machine::state_t state,
-			       const metadata *m,
-			       const char *fmt, ...) = 0;
+			       pending_diagnostic *d) = 0;
 
   // etc, what else?
 
diff --git a/gcc/diagnostic-core.h b/gcc/diagnostic-core.h
index 1fd8f40..0770ac7 100644
--- a/gcc/diagnostic-core.h
+++ b/gcc/diagnostic-core.h
@@ -81,6 +81,8 @@ extern bool warning_at (location_t, int, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
 extern bool warning_at (rich_location *, int, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
+extern bool warning_at (rich_location *, const diagnostic_metadata &, const char *, ...)
+    ATTRIBUTE_GCC_DIAG(3,4);
 extern void error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
 extern void error_n (location_t, unsigned HOST_WIDE_INT, const char *,
 		     const char *, ...)
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index a0f8fa6..3a7f831 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -1409,6 +1409,24 @@ warning_at (rich_location *richloc, int opt, const char *gmsgid, ...)
   return ret;
 }
 
+/* FIXME.  */
+
+bool
+warning_at (rich_location *richloc, const diagnostic_metadata &metadata,
+	    const char *gmsgid, ...)
+{
+  gcc_assert (richloc);
+
+  auto_diagnostic_group d;
+  va_list ap;
+  va_start (ap, gmsgid);
+  bool ret
+    = diagnostic_impl (richloc, &metadata, 0, gmsgid, &ap,
+		       DK_WARNING);
+  va_end (ap);
+  return ret;
+}
+
 /* Same as warning_n plural variant below, but using RICHLOC.  */
 
 bool
-- 
1.8.5.3

