From 62d1e5f8504e94fecb4d29097fcb9fd55b44f89d Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 3 Apr 2019 11:05:39 -0400
Subject: [PATCH 177/217] FIXME: consolidate leak-printing by EC

---
 gcc/analyzer/engine.cc                   | 77 ++++++++++++++++++++++++++++++--
 gcc/testsuite/gcc.dg/analyzer/malloc-1.c | 18 +++++---
 2 files changed, 85 insertions(+), 10 deletions(-)

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index bb1b2ec..2ccb48d 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -2547,6 +2547,54 @@ get_stmt_for_leak (tree var, const backtrace &b)
   return NULL;
 }
 
+/* A measurement of how good EXPR is for presenting to the user, so
+   that e.g. we can say prefer printing
+     "leak of 'tmp.m_ptr'"
+   over:
+     leak of '<unknown>'.  */
+
+static int
+readability (tree expr)
+{
+  switch (TREE_CODE (expr))
+    {
+    case COMPONENT_REF:
+    case MEM_REF:
+      return readability (TREE_OPERAND (expr, 0)) - 1;
+
+    case SSA_NAME:
+      {
+	if (tree var = SSA_NAME_VAR (expr))
+	  return readability (var);
+	return -1;
+      }
+      break;
+
+    case VAR_DECL:
+      return 256;
+
+    default:
+      return 0;
+    }
+
+  return 0;
+}
+
+/* A qsort comparator for var_state_t for use by engine::detect_leaks,
+   to sort them into most user-readable to least user-readable.  */
+
+static int
+leak_comparator (const void *p1, const void *p2)
+{
+  const var_state_t *vs1 = (const var_state_t *)p1;
+  const var_state_t *vs2 = (const var_state_t *)p2;
+
+  int r1 = readability (vs1->m_var);
+  int r2 = readability (vs2->m_var);
+
+  return r2 - r1;
+}
+
 /* FIXME.  */
 
 void
@@ -2577,6 +2625,9 @@ engine::detect_leaks (const supernode *node, const sm_instance &sm_inst,
       }
   }
 
+  /* Gather all leaks into a vec so we can sort them.   */
+  auto_vec<var_state_t> leaks;
+
   for (hash_set<var_state_t>::iterator iter
 	 = sm_inst.m_active_vars.m_set.begin ();
        iter != sm_inst.m_active_vars.m_set.end ();
@@ -2587,11 +2638,25 @@ engine::detect_leaks (const supernode *node, const sm_instance &sm_inst,
       if (get_logger ())
 	log ("considering %s", (*iter).to_str (m_sm).c_str ());
 
-      /* Did this var's equivalence class escape?  If so, don't complain
+     leaks.safe_push (*iter);
+    }
+
+  /* Sort the leaks into most human-readable first, through
+     to least user-readable.  Given that we only emit one
+     leak per EC, this ought to ensure that we pick the most
+     user-readable description of each leaking EC.  */
+  leaks.qsort (leak_comparator);
+
+  /* Iterate through the sorted leaks, potentially emitting them.  */
+  int i;
+  var_state_t *vs;
+  FOR_EACH_VEC_ELT (leaks, i, vs)
+    {
+       /* Did this var's equivalence class escape?  If so, don't complain
 	 about it leaking.  */
       // FIXME: stack depth:
       equiv_class_id ec_id
-	= new_ps.get_or_add_equiv_class (path_var ((*iter).m_var, 0));
+	= new_ps.get_or_add_equiv_class (path_var (vs->m_var, 0));
       equiv_class &ec = ec_id.get_obj (new_ps);
       if (get_logger ())
 	log ("within %s", ec.to_str ().c_str ());
@@ -2609,7 +2674,7 @@ engine::detect_leaks (const supernode *node, const sm_instance &sm_inst,
 	}
 
       // TODO: determine most appropriate stmt
-      const gimple *stmt = get_stmt_for_leak ((*iter).m_var, b);
+      const gimple *stmt = get_stmt_for_leak (vs->m_var, b);
 
       // FIXME: various redundant copies here:
       checker_path p (b.m_path);
@@ -2618,7 +2683,11 @@ engine::detect_leaks (const supernode *node, const sm_instance &sm_inst,
 			      &new_sm_inst,
 			      &sm_inst,
 			      &p, &new_ps);
-      m_sm.on_leak (&ctxt, node, stmt, (*iter).m_var, (*iter).m_state);
+      m_sm.on_leak (&ctxt, node, stmt, vs->m_var, vs->m_state);
+
+      /* Mark this EC as having escaped, to suppress further leak warnings
+	 from it.  */
+      ec.m_escaped = true;
     }
 }
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
index c1a3930..128dfe9 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
@@ -333,17 +333,23 @@ struct link *test_30 (void)
 }
 #endif
 
-/* TODO: this one is showing two leaks:
+/* Without consolidation by EC, this one used to show two leaks:
      warning: leak of '<unknown>'
      warning: leak of 'tmp.m_ptr' 
-   We should only show the latter.  */
-#if 0
+   We should only show the latter (favoring the most user-readable
+   expression in the equivalence class).  */
 void test_30 (void)
 {
   struct link tmp;
-  tmp.m_ptr = (int *)malloc (sizeof (struct link));
-}
-#endif
+  tmp.m_ptr = (struct link *)malloc (sizeof (struct link));
+} /* { dg-warning "leak of 'tmp.m_ptr'" } */ 
+
+void test_31 (void)
+{
+  struct link tmp;
+  void *ptr = malloc (sizeof (struct link));
+  tmp.m_ptr = (struct link *)ptr;
+} /* { dg-warning "leak of 'ptr'" } */ 
 
 #if 0
 void test_31 (void *p)
-- 
1.8.5.3

