From 6753b76c4613b5679e22ec956ef945addc15d18a Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Mon, 21 Oct 2019 09:46:57 -0400
Subject: [PATCH 79/79] FIXME: WIP on using expression_graph for leaks

---
 gcc/analyzer/engine.cc                            | 41 ++++++++++++++++++++++-
 gcc/analyzer/sm-file.cc                           | 22 ++++++++----
 gcc/analyzer/sm-malloc.cc                         | 23 +++++++++----
 gcc/analyzer/sm-pattern-test.cc                   |  7 ++--
 gcc/analyzer/sm-sensitive.cc                      |  6 ++--
 gcc/analyzer/sm-taint.cc                          |  6 ++--
 gcc/analyzer/sm.h                                 | 17 +++++++++-
 gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c |  3 +-
 8 files changed, 101 insertions(+), 24 deletions(-)

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 5d897fc..3905355 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -507,6 +507,35 @@ readability_comparator (const void *p1, const void *p2)
   return r2 - r1;
 }
 
+/* FIXME: enough information to (hopefully) be able to generate a
+   user-readable tree for a SID if it's needed when emitting a diagnostic
+   (not doing the thing if it's expensive).
+   Is this a "promise"-style API?  (those tend to be async).  */
+
+class impl_deferred_tree : public deferred_tree
+{
+public:
+  impl_deferred_tree (const region_model &model, svalue_id sid, tree fallback)
+  : m_model (model), m_sid (sid), m_fallback (fallback) {}
+
+  deferred_tree *clone () const { return new impl_deferred_tree (*this); }
+
+  tree get_readable_expr () const
+  {
+    // FIXME: use old state, as hopefully it has a user-readable tree for old_sid
+    expression_graph expr_graph (m_model);
+    tree leaked_tree = expr_graph.get_tree_for_sid (m_sid);
+    if (leaked_tree)
+      return leaked_tree;
+    else
+      return m_fallback;
+  }
+private:
+  const region_model &m_model;
+  svalue_id m_sid;
+ tree m_fallback;
+};
+
 /* Create an sm_context and use it to call SM's on_leak vfunc, so that
    it can potentially complain about a leak of SID (in a new region_model)
    in the given STATE, where MAP can be used to map SID back to an "old"
@@ -534,6 +563,14 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
 
   svalue_id old_sid = map.get_src_for_dst (sid);
 
+#if 0
+  // FIXME: use old state, as hopefully it has a user-readable tree for old_sid
+  expression_graph expr_graph (*m_old_state->m_region_model);
+  tree leaked_tree = expr_graph.get_tree_for_sid (old_sid);
+  if (leaked_tree == NULL_TREE)
+    return;
+#else
+
   auto_vec<path_var> leaked_pvs;
   m_old_state->m_region_model->get_path_vars_for_svalue (old_sid, &leaked_pvs);
 
@@ -553,6 +590,7 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
   tree leaked_tree = leaked_pvs[0].m_tree;
   if (get_logger ())
     log ("best leaked_tree: %qE", leaked_tree);
+#endif
 
   leak_stmt_finder stmt_finder (*m_eg, leaked_tree);
   impl_sm_context sm_ctxt (*m_eg, sm, m_enode_for_diag,
@@ -574,8 +612,9 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
 	}
     }
 
+  impl_deferred_tree t (*m_old_state->m_region_model, old_sid, leaked_tree);
   sm.on_leak (&sm_ctxt, m_enode_for_diag->get_supernode (), m_stmt,
-	      leaked_tree, state);
+	      leaked_tree, state, &t);
 }
 
 /* Implementation of region_model_context::on_inherited_svalue vfunc
diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc
index 4fc308c..4ab953f 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -56,7 +56,9 @@ public:
 		const supernode *node,
 		const gimple *stmt,
 		tree var,
-		state_machine::state_t state) const FINAL OVERRIDE;
+		state_machine::state_t state,
+		const deferred_tree *t) const FINAL OVERRIDE;
+
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
 
   /* Start state.  */
@@ -159,9 +161,13 @@ private:
 class file_leak : public file_diagnostic
 {
 public:
-  file_leak (const fileptr_state_machine &sm, tree arg)
-    : file_diagnostic (sm, arg)
+  file_leak (const fileptr_state_machine &sm, tree arg, deferred_tree *t)
+  : file_diagnostic (sm, arg), m_t (t)
   {}
+  ~file_leak ()
+  {
+    delete m_t;
+  }
 
   const char *get_kind () const FINAL OVERRIDE { return "file_leak"; }
 
@@ -173,7 +179,7 @@ public:
     m.add_cwe (775);
     return warning_at (rich_loc, m, OPT_Wanalyzer_file_leak,
 		       "leak of FILE %qE",
-		       m_arg);
+		       m_t->get_readable_expr ());
   }
 
   label_text describe_state_change (const evdesc::state_change &change)
@@ -198,6 +204,7 @@ public:
 
 private:
   diagnostic_event_id_t m_fopen_event;
+  deferred_tree *m_t;
 };
 
 ////////////////////////////////////////////////////////////////////////////
@@ -308,13 +315,14 @@ fileptr_state_machine::on_leak (sm_context *sm_ctxt,
 				const supernode *node,
 				const gimple *stmt,
 				tree var,
-				state_machine::state_t state ATTRIBUTE_UNUSED)
+				state_machine::state_t state ATTRIBUTE_UNUSED,
+				const deferred_tree *t)
   const
 {
   sm_ctxt->warn_for_state (node, stmt, var, m_unchecked,
-			   new file_leak (*this, var));
+			   new file_leak (*this, var, t->clone ()));
   sm_ctxt->warn_for_state (node, stmt, var, m_nonnull,
-			   new file_leak (*this, var));
+			   new file_leak (*this, var, t->clone ()));
 }
 
 /* Implementation of state_machine::can_purge_p vfunc for fileptr_state_machine.
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index 5dd5e52..c9faee7 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -58,7 +58,9 @@ public:
 		const supernode *node,
 		const gimple *stmt,
 		tree var,
-		state_machine::state_t state) const FINAL OVERRIDE;
+		state_machine::state_t state,
+		const deferred_tree *t) const FINAL OVERRIDE;
+
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
 
   /* Start state.  */
@@ -446,8 +448,13 @@ private:
 class malloc_leak : public malloc_diagnostic
 {
 public:
-  malloc_leak (const malloc_state_machine &sm, tree arg)
-  : malloc_diagnostic (sm, arg) {}
+  malloc_leak (const malloc_state_machine &sm, tree arg, deferred_tree *t)
+  : malloc_diagnostic (sm, arg), m_t (t) {}
+
+  ~malloc_leak ()
+  {
+    delete m_t;
+  }
 
   const char *get_kind () const FINAL OVERRIDE { return "malloc_leak"; }
 
@@ -456,7 +463,7 @@ public:
     diagnostic_metadata m;
     m.add_cwe (401);
     return warning_at (rich_loc, m, OPT_Wanalyzer_malloc_leak,
-		       "leak of %qE", m_arg);
+		       "leak of %qE", m_t->get_readable_expr ());
   }
 
   label_text describe_state_change (const evdesc::state_change &change)
@@ -481,6 +488,7 @@ public:
 
 private:
   diagnostic_event_id_t m_malloc_event;
+  deferred_tree *m_t;
 };
 
 class free_of_non_heap : public malloc_diagnostic
@@ -771,13 +779,14 @@ malloc_state_machine::on_leak (sm_context *sm_ctxt,
 			       const supernode *node,
 			       const gimple *stmt,
 			       tree var,
-			       state_machine::state_t state ATTRIBUTE_UNUSED)
+			       state_machine::state_t state ATTRIBUTE_UNUSED,
+			       const deferred_tree *t)
   const
 {
   sm_ctxt->warn_for_state (node, stmt, var, m_unchecked,
-			   new malloc_leak (*this, var));
+			   new malloc_leak (*this, var, t->clone ()));
   sm_ctxt->warn_for_state (node, stmt, var, m_nonnull,
-			   new malloc_leak (*this, var));
+			   new malloc_leak (*this, var, t->clone ()));
 }
 
 /* Implementation of state_machine::can_purge_p vfunc for malloc_state_machine.
diff --git a/gcc/analyzer/sm-pattern-test.cc b/gcc/analyzer/sm-pattern-test.cc
index cf42cfd..3a3c0a9 100644
--- a/gcc/analyzer/sm-pattern-test.cc
+++ b/gcc/analyzer/sm-pattern-test.cc
@@ -60,7 +60,9 @@ public:
 		const supernode *node,
 		const gimple *stmt,
 		tree var,
-		state_machine::state_t state) const FINAL OVERRIDE;
+		state_machine::state_t state,
+		const deferred_tree *t) const FINAL OVERRIDE;
+
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
 
 private:
@@ -142,7 +144,8 @@ pattern_test_state_machine::on_leak (sm_context *sm_ctxt ATTRIBUTE_UNUSED,
 				     const gimple *stmt ATTRIBUTE_UNUSED,
 				     tree var ATTRIBUTE_UNUSED,
 				     state_machine::state_t state
-				       ATTRIBUTE_UNUSED)
+				       ATTRIBUTE_UNUSED,
+				     const deferred_tree *t ATTRIBUTE_UNUSED)
   const
 {
   /* Empty.  */
diff --git a/gcc/analyzer/sm-sensitive.cc b/gcc/analyzer/sm-sensitive.cc
index f634b8f..c31bb8b 100644
--- a/gcc/analyzer/sm-sensitive.cc
+++ b/gcc/analyzer/sm-sensitive.cc
@@ -58,7 +58,8 @@ public:
 		const supernode *node,
 		const gimple *stmt,
 		tree var,
-		state_machine::state_t state) const FINAL OVERRIDE;
+		state_machine::state_t state,
+		const deferred_tree *t) const FINAL OVERRIDE;
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
 
 private:
@@ -186,7 +187,8 @@ sensitive_state_machine::on_leak (sm_context *sm_ctxt ATTRIBUTE_UNUSED,
 				  const supernode *node ATTRIBUTE_UNUSED,
 				  const gimple *stmt ATTRIBUTE_UNUSED,
 				  tree var ATTRIBUTE_UNUSED,
-				  state_machine::state_t state ATTRIBUTE_UNUSED)
+				  state_machine::state_t state ATTRIBUTE_UNUSED,
+				  const deferred_tree *t ATTRIBUTE_UNUSED)
   const
 {
   /* Empty.  */
diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc
index c664a54..c43f043 100644
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -59,7 +59,8 @@ public:
 		const supernode *node,
 		const gimple *stmt,
 		tree var,
-		state_machine::state_t state) const FINAL OVERRIDE;
+		state_machine::state_t state,
+		const deferred_tree *t) const FINAL OVERRIDE;
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
 
   /* Start state.  */
@@ -315,7 +316,8 @@ taint_state_machine::on_leak (sm_context *sm_ctxt ATTRIBUTE_UNUSED,
 			      const supernode *node ATTRIBUTE_UNUSED,
 			      const gimple *stmt ATTRIBUTE_UNUSED,
 			      tree var ATTRIBUTE_UNUSED,
-			      state_machine::state_t state ATTRIBUTE_UNUSED)
+			      state_machine::state_t state ATTRIBUTE_UNUSED,
+			      const deferred_tree *t ATTRIBUTE_UNUSED)
   const
 {
   /* Empty.  */
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index 52f4734..d6cdba7 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -38,6 +38,20 @@ class state_machine;
 class sm_context;
 class pending_diagnostic;
 
+/* FIXME: enough information to (hopefully) be able to generate a
+   user-readable tree if it's needed when emitting a diagnostic, but
+   not generating it otherwise (as it could be expensive).
+   An on-stack one is passed to on_leak, so it needs to be cloned if it's to
+   be saved.  */
+
+class deferred_tree
+{
+ public:
+  virtual ~deferred_tree () {}
+  virtual deferred_tree *clone () const = 0;
+  virtual tree get_readable_expr () const = 0;
+};
+
 /* An abstract base class for a state machine describing an API.
    A mapping from state IDs to names, and various virtual functions
    for pattern-matching on statements.  */
@@ -77,7 +91,8 @@ public:
 			const supernode *node,
 			const gimple *stmt,
 			tree var,
-			state_machine::state_t state) const = 0;
+			state_machine::state_t state,
+			const deferred_tree *t) const = 0;
 
   /* Return true if it safe to discard the given state (to help
      when simplifying state objects).
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c
index 2a0c8a4..a9669d8 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c
@@ -38,8 +38,7 @@ int test_1 (int n)
 
   __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
 
-  return result; /* { dg-message "leak of 'p'" } */
-  /* FIXME: should this be 'ptr'?  */
+  return result; /* { dg-message "leak of 'ptr'" } */
 }
 
 /* A simpler version of the above.  */
-- 
1.8.5.3

