From 5c6644563eb837367f84f1a34eddd038da420cca Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 20 May 2020 10:23:42 -0400
Subject: [PATCH 161/315] FIXME: fix "double-free of '<unknown>'" in
 CVE-2005-1689-minimal.c

---
 gcc/analyzer/engine.cc        | 12 ++++--------
 gcc/analyzer/region-model2.cc | 20 ++++++++++----------
 gcc/analyzer/sm-file.cc       |  7 ++++---
 gcc/analyzer/sm-malloc.cc     | 30 +++++++++++++++++-------------
 gcc/analyzer/sm-sensitive.cc  |  3 ++-
 gcc/analyzer/sm-taint.cc      | 11 ++++++-----
 gcc/analyzer/sm.h             |  7 ++++++-
 gcc/analyzer/store2.cc        |  8 ++++++++
 8 files changed, 57 insertions(+), 41 deletions(-)

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index b43e1643719..26f9f45d489 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -416,7 +416,7 @@ public:
      we can print:
        "double-free of 'inbuf.data'".  */
 
-  tree get_readable_tree (tree expr) FINAL OVERRIDE
+  tree get_diagnostic_tree (tree expr) FINAL OVERRIDE
   {
     /* Only for SSA_NAMEs of temporaries; otherwise, return EXPR, as it's
        likely to be the least surprising tree to report.  */
@@ -580,7 +580,7 @@ public:
      we can print:
        "double-free of 'inbuf.data'".  */
 
-  tree get_readable_tree (tree expr) FINAL OVERRIDE
+  tree get_diagnostic_tree (tree expr) FINAL OVERRIDE
   {
     /* Only for SSA_NAMEs of temporaries; otherwise, return EXPR, as it's
        likely to be the least surprising tree to report.  */
@@ -589,22 +589,17 @@ public:
     if (SSA_NAME_VAR (expr) != NULL)
       return expr;
 
-#if 1
-    // TODO:
-    return expr;
-#else
     gcc_assert (m_new_state);
     const svalue2 *sval = m_new_state->m_region_model2->get_rvalue (expr, NULL);
     /* Find trees for all regions storing the value.  */
     auto_vec<path_var> pvs;
-    m_new_state->m_region_model2->get_path_vars_for_svalue (sval, &pvs);
+    m_new_state->m_region_model2->get_path_vars_for_svalue2 (sval, &pvs);
     if (pvs.length () < 1)
       return expr;
     /* Pick the "best" such tree.  */
     // TODO: should we also consider (and consolidate) equiv classes?
     pvs.qsort (readability_comparator);
     return pvs[0].m_tree;
-#endif
   }
 
   state_machine::state_t get_global_state () const FINAL OVERRIDE
@@ -749,6 +744,7 @@ readability (const_tree expr)
       }
       break;
 
+    case PARM_DECL:
     case VAR_DECL:
       /* Arbitrarily-chosen "high readability" value.  */
       return 256;
diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc
index 1da9ea27784..274c5ed0362 100644
--- a/gcc/analyzer/region-model2.cc
+++ b/gcc/analyzer/region-model2.cc
@@ -2468,10 +2468,17 @@ field_region2::dump_to_pp (pretty_printer *pp, bool simple) const
 }
 
 bool
-field_region2::get_representative_path_var (path_var *) const
+field_region2::get_representative_path_var (path_var *out) const
 {
-  // TODO
-  return false;
+  path_var parent_pv (NULL_TREE, 0);
+  if (!get_parent_region ()->get_representative_path_var (&parent_pv))
+    return false;
+
+  *out = path_var (build3 (COMPONENT_REF,
+			   get_type (),
+			   parent_pv.m_tree, m_field, NULL_TREE),
+		   parent_pv.m_stack_depth);
+  return true;
 }
 
 /* class element_region2 : public region2.  */
@@ -6083,13 +6090,6 @@ region_model2::get_representative_tree (const svalue2 *sval) const
   auto_vec<path_var> pvs;
   m_store.get_representative_path_vars (sval, &pvs);
 
-  if (const initial_svalue2 *init_sval = sval->dyn_cast_initial_svalue2 ())
-    {
-      const region2 *reg = init_sval->get_region ();
-      path_var pv (NULL_TREE, 0);
-      if (reg->get_representative_path_var (&pv))
-	pvs.safe_push (pv);
-    }
   if (tree cst = sval->maybe_get_constant ())
     pvs.safe_push (path_var (cst, 0));
 
diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc
index 7c546997a0c..024bf965e9f 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -311,7 +311,8 @@ fileptr_state_machine::on_stmt (sm_context *sm_ctxt,
 	    tree lhs = gimple_call_lhs (call);
 	    if (lhs)
 	      {
-		lhs = sm_ctxt->get_readable_tree (lhs);
+		// FIXME:
+		//lhs = sm_ctxt->get_readable_tree (lhs);
 		sm_ctxt->on_transition (node, stmt, lhs, m_start, m_unchecked);
 	      }
 	    else
@@ -324,7 +325,7 @@ fileptr_state_machine::on_stmt (sm_context *sm_ctxt,
 	if (is_named_call_p (callee_fndecl, "fclose", call, 1))
 	  {
 	    tree arg = gimple_call_arg (call, 0);
-	    arg = sm_ctxt->get_readable_tree (arg);
+	    tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
 
 	    sm_ctxt->on_transition (node, stmt, arg, m_start, m_closed);
 
@@ -335,7 +336,7 @@ 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,
-				     new double_fclose (*this, arg));
+				     new double_fclose (*this, diag_arg));
 	    sm_ctxt->on_transition (node, stmt, arg, m_closed, m_stop);
 	    return true;
 	  }
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index 38a2f1eea0c..264810d82a6 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -619,7 +619,8 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	    tree lhs = gimple_call_lhs (call);
 	    if (lhs)
 	      {
-		lhs = sm_ctxt->get_readable_tree (lhs);
+		// FIXME:
+		//lhs = sm_ctxt->get_readable_tree (lhs);
 		sm_ctxt->on_transition (node, stmt, lhs, m_start, m_unchecked);
 	      }
 	    else
@@ -635,7 +636,8 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	    tree lhs = gimple_call_lhs (call);
 	    if (lhs)
 	      {
-		lhs = sm_ctxt->get_readable_tree (lhs);
+		// FIXME:
+		//lhs = sm_ctxt->get_readable_tree (lhs);
 		sm_ctxt->on_transition (node, stmt, lhs, m_start, m_non_heap);
 	      }
 	    return true;
@@ -646,8 +648,7 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	    || is_named_call_p (callee_fndecl, "__builtin_free", call, 1))
 	  {
 	    tree arg = gimple_call_arg (call, 0);
-
-	    arg = sm_ctxt->get_readable_tree (arg);
+	    tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
 
 	    /* start/unchecked/nonnull -> freed.  */
 	    sm_ctxt->on_transition (node, stmt, arg, m_start, m_freed);
@@ -659,12 +660,12 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 
 	    /* freed -> stop, with warning.  */
 	    sm_ctxt->warn_for_state (node, stmt, arg, m_freed,
-				     new double_free (*this, arg));
+				     new double_free (*this, diag_arg));
 	    sm_ctxt->on_transition (node, stmt, arg, m_freed, m_stop);
 
 	    /* non-heap -> stop, with warning.  */
 	    sm_ctxt->warn_for_state (node, stmt, arg, m_non_heap,
-				     new free_of_non_heap (*this, arg));
+				     new free_of_non_heap (*this, diag_arg));
 	    sm_ctxt->on_transition (node, stmt, arg, m_non_heap, m_stop);
 	    return true;
 	  }
@@ -685,15 +686,17 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 		  if (bitmap_empty_p (nonnull_args)
 		      || bitmap_bit_p (nonnull_args, i))
 		    {
+		      tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
 		      sm_ctxt->warn_for_state
 			(node, stmt, arg, m_unchecked,
-			 new possible_null_arg (*this, arg, callee_fndecl, i));
+			 new possible_null_arg (*this, diag_arg, callee_fndecl,
+						i));
 		      sm_ctxt->on_transition (node, stmt, arg, m_unchecked,
 					      m_nonnull);
 
 		      sm_ctxt->warn_for_state
 			(node, stmt, arg, m_null,
-			 new null_arg (*this, arg, callee_fndecl, i));
+			 new null_arg (*this, diag_arg, callee_fndecl, i));
 		      sm_ctxt->on_transition (node, stmt, arg, m_null, m_stop);
 		    }
 		}
@@ -714,7 +717,8 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	  tree lhs = gimple_assign_lhs (assign_stmt);
 	  if (lhs)
 	    {
-	      lhs = sm_ctxt->get_readable_tree (lhs);
+	      // FIXME:
+	      //lhs = sm_ctxt->get_readable_tree (lhs);
 	      sm_ctxt->on_transition (node, stmt, lhs, m_start, m_non_heap);
 	    }
 	}
@@ -732,18 +736,18 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
       if (TREE_CODE (op) == MEM_REF)
 	{
 	  tree arg = TREE_OPERAND (op, 0);
-	  arg = sm_ctxt->get_readable_tree (arg);
+	  tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
 
 	  sm_ctxt->warn_for_state (node, stmt, arg, m_unchecked,
-				   new possible_null_deref (*this, arg));
+				   new possible_null_deref (*this, diag_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 (*this, arg));
+				   new null_deref (*this, diag_arg));
 	  sm_ctxt->on_transition (node, stmt, arg, m_null, m_stop);
 
 	  sm_ctxt->warn_for_state (node, stmt, arg, m_freed,
-				   new use_after_free (*this, arg));
+				   new use_after_free (*this, diag_arg));
 	  sm_ctxt->on_transition (node, stmt, arg, m_freed, m_stop);
 	}
     }
diff --git a/gcc/analyzer/sm-sensitive.cc b/gcc/analyzer/sm-sensitive.cc
index ff0c3284843..b14bf147d0c 100644
--- a/gcc/analyzer/sm-sensitive.cc
+++ b/gcc/analyzer/sm-sensitive.cc
@@ -177,8 +177,9 @@ sensitive_state_machine::warn_for_any_exposure (sm_context *sm_ctxt,
 						const gimple *stmt,
 						tree arg) const
 {
+  tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
   sm_ctxt->warn_for_state (node, stmt, arg, m_sensitive,
-			   new exposure_through_output_file (*this, arg));
+			   new exposure_through_output_file (*this, diag_arg));
 }
 
 /* Implementation of state_machine::on_stmt vfunc for
diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc
index dff43a4ea80..63f6421f959 100644
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -208,7 +208,8 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt,
 	if (is_named_call_p (callee_fndecl, "fread", call, 4))
 	  {
 	    tree arg = gimple_call_arg (call, 0);
-	    arg = sm_ctxt->get_readable_tree (arg);
+	    // FIXME:
+	    // arg = sm_ctxt->get_readable_tree (arg);
 
 	    sm_ctxt->on_transition (node, stmt, arg, m_start, m_tainted);
 
@@ -231,7 +232,7 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt,
       if (op == ARRAY_REF)
 	{
 	  tree arg = TREE_OPERAND (rhs1, 1);
-	  arg = sm_ctxt->get_readable_tree (arg);
+	  tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
 
 	  /* Unsigned types have an implicit lower bound.  */
 	  bool is_unsigned = false;
@@ -241,14 +242,14 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt,
 	  /* Complain about missing bounds.  */
 	  sm_ctxt->warn_for_state
 	    (node, stmt, arg, m_tainted,
-	     new tainted_array_index (*this, arg,
+	     new tainted_array_index (*this, diag_arg,
 				      is_unsigned
 				      ? BOUNDS_LOWER : BOUNDS_NONE));
 	  sm_ctxt->on_transition (node, stmt, arg, m_tainted, m_stop);
 
 	  /* Complain about missing upper bound.  */
 	  sm_ctxt->warn_for_state  (node, stmt, arg, m_has_lb,
-				    new tainted_array_index (*this, arg,
+				    new tainted_array_index (*this, diag_arg,
 							     BOUNDS_LOWER));
 	  sm_ctxt->on_transition (node, stmt, arg, m_has_lb, m_stop);
 
@@ -256,7 +257,7 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt,
 	  if (!is_unsigned)
 	    {
 	      sm_ctxt->warn_for_state  (node, stmt, arg, m_has_ub,
-					new tainted_array_index (*this, arg,
+					new tainted_array_index (*this, diag_arg,
 								 BOUNDS_UPPER));
 	      sm_ctxt->on_transition (node, stmt, arg, m_has_ub, m_stop);
 	    }
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index ebd067a4541..5afb0c17a07 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -154,7 +154,12 @@ public:
 			       tree var, state_machine::state_t state,
 			       pending_diagnostic *d) = 0;
 
-  virtual tree get_readable_tree (tree expr)
+  /* For use when generating trees when creating pending_diagnostics, so that
+     rather than e.g.
+       "double-free of '<unknown>'"
+     we can print:
+       "double-free of 'inbuf.data'".  */
+  virtual tree get_diagnostic_tree (tree expr)
   {
     return expr;
   }
diff --git a/gcc/analyzer/store2.cc b/gcc/analyzer/store2.cc
index ae1a23cf19d..2c910bc0677 100644
--- a/gcc/analyzer/store2.cc
+++ b/gcc/analyzer/store2.cc
@@ -1236,6 +1236,14 @@ store2::get_representative_path_vars (const svalue2 *sval,
       binding_cluster2 *cluster = (*iter).second;
       cluster->get_representative_path_vars (base_reg, sval, out_pvs);
     }
+
+  if (const initial_svalue2 *init_sval = sval->dyn_cast_initial_svalue2 ())
+    {
+      const region2 *reg = init_sval->get_region ();
+      path_var pv (NULL_TREE, 0);
+      if (reg->get_representative_path_var (&pv))
+	out_pvs->safe_push (pv);
+    }
 }
 
 /* FIXME.  */
-- 
2.26.2

