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

---
 gcc/analyzer/engine.cc                            | 41 ++++++++++++++++++++++-
 gcc/analyzer/sm-file.cc                           | 18 ++++++----
 gcc/analyzer/sm-malloc.cc                         | 19 +++++++----
 gcc/analyzer/sm.h                                 | 18 +++++++++-
 gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c |  3 +-
 5 files changed, 83 insertions(+), 16 deletions(-)

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index c38c9fa..2f66720 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -472,6 +472,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 DST_SID (in a new region_model)
    in the given STATE, where MAP can be used to map SID back to an "old"
@@ -535,6 +564,14 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
 
   svalue_id old_sid = map.get_src_for_dst (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);
 
@@ -552,6 +589,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_idx, sm, m_enode_for_diag,
@@ -574,7 +612,8 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
 	}
     }
 
-  pending_diagnostic *pd = sm.on_leak (leaked_tree);
+  impl_deferred_tree t (*m_old_state->m_region_model, old_sid, leaked_tree);
+  pending_diagnostic *pd = sm.on_leak (leaked_tree, &t);
   if (pd)
     m_eg->get_diagnostic_manager ().add_diagnostic
       (&sm, m_enode_for_diag, m_enode_for_diag->get_supernode (),
diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc
index e10b88b..65a4900 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -54,7 +54,8 @@ public:
 		     tree rhs) const FINAL OVERRIDE;
 
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
-  pending_diagnostic *on_leak (tree var) const FINAL OVERRIDE;
+  pending_diagnostic *on_leak (tree var,
+			       const deferred_tree *t) const FINAL OVERRIDE;
 
   /* Start state.  */
   state_t m_start;
@@ -156,9 +157,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"; }
 
@@ -170,7 +175,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)
@@ -195,6 +200,7 @@ public:
 
 private:
   diagnostic_event_id_t m_fopen_event;
+  deferred_tree *m_t;
 };
 
 ////////////////////////////////////////////////////////////////////////////
@@ -320,9 +326,9 @@ fileptr_state_machine::can_purge_p (state_t s) const
    state 'unchecked' and 'nonnull'.  */
 
 pending_diagnostic *
-fileptr_state_machine::on_leak (tree var) const
+fileptr_state_machine::on_leak (tree var, const deferred_tree *t) const
 {
-  return new file_leak (*this, var);
+  return new file_leak (*this, var, t->clone ());
 }
 
 } // anonymous namespace
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index 58e39e7..2575272 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -57,7 +57,8 @@ public:
 		     tree rhs) const FINAL OVERRIDE;
 
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
-  pending_diagnostic *on_leak (tree var) const FINAL OVERRIDE;
+  pending_diagnostic *on_leak (tree var,
+			       const deferred_tree *t) const FINAL OVERRIDE;
 
   /* Start state.  */
   state_t m_start;
@@ -444,8 +445,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"; }
 
@@ -454,7 +460,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)
@@ -479,6 +485,7 @@ public:
 
 private:
   diagnostic_event_id_t m_malloc_event;
+  deferred_tree *m_t;
 };
 
 class free_of_non_heap : public malloc_diagnostic
@@ -776,9 +783,9 @@ malloc_state_machine::can_purge_p (state_t s) const
    'nonnull').  */
 
 pending_diagnostic *
-malloc_state_machine::on_leak (tree var) const
+malloc_state_machine::on_leak (tree var, const deferred_tree *t) const
 {
-  return new malloc_leak (*this, var);
+  return new malloc_leak (*this, var, t->clone ());
 }
 
 } // anonymous namespace
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index b376868..94e9048 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.  */
@@ -79,7 +93,9 @@ public:
   virtual bool can_purge_p (state_t s) const = 0;
 
   /* Called when VAR leaks (and !can_purge_p).  */
-  virtual pending_diagnostic *on_leak (tree var ATTRIBUTE_UNUSED) const
+  virtual pending_diagnostic *on_leak (tree var ATTRIBUTE_UNUSED,
+				       const deferred_tree *t
+				         ATTRIBUTE_UNUSED) const
   {
     return NULL;
   }
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

