From 1d10b10ce07035cbe042e8b49ba0d30b29108388 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 22 Jul 2020 20:59:54 -0400
Subject: [PATCH 319/377] FIXME: fix enode splitting at sm state changes

---
 gcc/analyzer/engine.cc                        | 58 +++++++++++++++++--
 gcc/analyzer/exploded-graph.h                 | 14 ++++-
 gcc/analyzer/program-point.h                  |  4 ++
 .../gcc.dg/analyzer/CVE-2005-1689-minimal.c   | 10 ++++
 4 files changed, 78 insertions(+), 8 deletions(-)

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 4c0855bce48..8f5500e8d19 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -2757,13 +2757,17 @@ exploded_graph::add_function_entry (function *fun)
    diagnostics when purging state *before* we have a new enode;
    this is a pre-existing enode.  Not sure if we will need this,
    but it seemed necessary when porting leak-detection from the
-   old region_model implementation.  */
+   old region_model implementation.
+
+   If IS_SPLIT is true, then don't add the new node to the worklist, and
+   set its status to STATUS_SPLIT.  */
 
 exploded_node *
 exploded_graph::get_or_create_node (const program_point &point,
 				    const program_state &state,
 				    const exploded_node *enode_for_diag,
-				    state_change *change)
+				    state_change *change,
+				    bool is_split)
 {
   logger * const logger = get_logger ();
   LOG_FUNC (logger);
@@ -2948,8 +2952,17 @@ exploded_graph::get_or_create_node (const program_point &point,
       delete js;
     }
 
-  /* Add the new node to the worlist.  */
-  m_worklist.add_node (node);
+  if (is_split)
+    {
+      /* Don't add "split" enodes to the worklist; we've already
+	 effectively processed them.  */
+      node->set_status (exploded_node::STATUS_SPLIT);
+    }
+  else
+    {
+      /* Add the new node to the worlist.  */
+      m_worklist.add_node (node);
+    }
   return node;
 }
 
@@ -3440,7 +3453,11 @@ exploded_graph::process_node (exploded_node *node)
 	   their enode (for which stmt_requires_new_enode_p returns true)
 
 	   Update next_state in-place, to get the result of the one
-	   or more stmts that are processed.  */
+	   or more stmts that are processed.
+
+	   Split the node in-place if an sm-state-change occurs, so that
+	   the sm-state-change occurs on an edge where the src enode has
+	   exactly one stmt, the one that caused the change. */
 	program_state next_state (state);
 	state_change change;
 	const supernode *snode = point.get_supernode ();
@@ -3491,7 +3508,33 @@ exploded_graph::process_node (exploded_node *node)
 	      = next_state.prune_for_point (*this, next_point, node, &change);
 
 	    if (flags.m_sm_changes || flag_analyzer_fine_grained)
-	      break;
+	      {
+		program_point split_point
+		  = program_point::before_stmt (point.get_supernode (),
+						stmt_idx,
+						point.get_call_string ());
+		if (split_point != node->get_point ())
+		  {
+		    /* If we're not at the start of NODE, split the enode at
+		       this stmt, so we have:
+			 node -> split_enode
+		       so that the next edge we add will be:
+			 split_enode -> next
+		       and any state change will effectively occur on that
+		       latter edge, and split_enode will contain just stmt.  */
+		    exploded_node *split_enode
+		      = get_or_create_node (split_point, old_state, node, NULL,
+					    true);
+		    if (!split_enode)
+		      return;
+		    add_edge (node, split_enode, NULL, change);
+		    node = split_enode;
+		    break;
+		  }
+		else
+		  /* If we're at the start of NODE, stop iterating. */
+		  break;
+	      }
 	  }
 	unsigned next_idx = stmt_idx + 1;
 	program_point next_point
@@ -4967,6 +5010,9 @@ private:
       case exploded_node::STATUS_MERGER:
 	pp_string (pp, "(M)");
 	break;
+      case exploded_node::STATUS_SPLIT:
+	pp_string (pp, "(S)");
+	break;
       }
     gv->end_tdtr ();
     /* Dump any saved_diagnostics at this enode.  */
diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h
index bea6c42e4b5..a462651da0f 100644
--- a/gcc/analyzer/exploded-graph.h
+++ b/gcc/analyzer/exploded-graph.h
@@ -234,7 +234,16 @@ class exploded_node : public dnode<eg_traits>
 
     /* Node was left unprocessed due to merger; it won't have had
        exploded_graph::process_node called on it.  */
-    STATUS_MERGER
+    STATUS_MERGER,
+
+    /* Node is a "split" point, introduced during node processing when
+       a sm-state-change occurred so that this node contains exactly
+       one stmt: the stmt that causes the state change, so that the
+       state change occurs along the successor edge from this node,
+       and the stmt in question is at the start of its enode.
+       Such split enodes are never added to the worklist, as they've
+       effectively already been processed.  */
+    STATUS_SPLIT
   };
 
   exploded_node (const point_and_state &ps, int index);
@@ -825,7 +834,8 @@ public:
   exploded_node *get_or_create_node (const program_point &point,
 				     const program_state &state,
 				     const exploded_node *enode_for_diag,
-				     state_change *change);
+				     state_change *change,
+				     bool is_split = false);
   exploded_edge *add_edge (exploded_node *src, exploded_node *dest,
 			   const superedge *sedge,
 			   const state_change &change,
diff --git a/gcc/analyzer/program-point.h b/gcc/analyzer/program-point.h
index 23e8a0b91b0..734bb1fb920 100644
--- a/gcc/analyzer/program-point.h
+++ b/gcc/analyzer/program-point.h
@@ -189,6 +189,10 @@ public:
     return (m_function_point == other.m_function_point
 	    && m_call_string == other.m_call_string);
   }
+  bool operator!= (const program_point &other) const
+  {
+    return !(*this == other);
+  }
 
   /* Accessors.  */
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/CVE-2005-1689-minimal.c b/gcc/testsuite/gcc.dg/analyzer/CVE-2005-1689-minimal.c
index aa9deb3be19..f84d530c392 100644
--- a/gcc/testsuite/gcc.dg/analyzer/CVE-2005-1689-minimal.c
+++ b/gcc/testsuite/gcc.dg/analyzer/CVE-2005-1689-minimal.c
@@ -28,3 +28,13 @@ test_3 (krb5_data inbuf, int flag)
   }
   free((char *)inbuf.data); /* { dg-warning "double-'free' of 'inbuf.data'" } */
 }
+
+extern void unknown_fn (void *);
+
+void
+test_4 (krb5_data inbuf)
+{
+  unknown_fn (NULL);
+  free(inbuf.data); /* { dg-message "first 'free' here" } */
+  free(inbuf.data); /* { dg-warning "double-'free' of 'inbuf.data'" } */
+}
-- 
2.26.2

