From 4a821c58388f33875d6c06ddc4c3e1c61298bf8d Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 23 Feb 2021 16:31:35 -0500
Subject: [PATCH 23/30] FIXME: implement concat-epath heuristic

---
 gcc/analyzer/diagnostic-manager.cc            | 167 +++++++++++++++++-
 gcc/analyzer/engine.cc                        |  21 +++
 gcc/analyzer/exploded-graph.h                 |   2 +
 gcc/testsuite/gcc.dg/analyzer/feasibility-1.c |   3 +-
 .../pr93355-localealias-feasibility-2.c       |   4 +-
 .../pr93355-localealias-feasibility-3.c       |   8 +-
 gcc/testsuite/gcc.dg/analyzer/unknown-fns-4.c |   2 +-
 7 files changed, 194 insertions(+), 13 deletions(-)

diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index f0f447f14dc..cb43ab2338a 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -90,6 +90,22 @@ private:
 
 /* class epath_finder.  */
 
+/* FIXME.  */
+
+static bool
+interesting_p (const exploded_node *enode)
+{
+  if (enode->m_preds.length () > 1)
+    return true;
+  if (enode->m_preds.length () == 1)
+    {
+      const exploded_node *pred = enode->m_preds[0]->m_src;
+      if (pred->m_succs.length () > 1)
+	return true;
+    }
+  return false;
+}
+
 /* Get the "best" exploded_path for reaching ENODE from the origin,
    returning ownership of it to the caller.
 
@@ -146,7 +162,156 @@ epath_finder::get_best_epath (const exploded_node *enode,
 			 desc, enode->m_index,
 			 snode_idx);
 	  delete epath;
-	  return NULL;
+
+	  /* TODO: Try harder; might need to clobber/delete *out_problem.
+	     Might want a cleaner interface for this.  */
+
+	  /* Create trimmed graph for just ENODE.
+	     Iterate through modifications.
+	       Create mutated graph expressing modifications.
+	       Find shortest path through modification.
+	       Is it feasible?
+	         Consider it.
+	     Return the shortest feasible path that was found
+	     via modifications.
+	     But we'll also want to debug feasibility issues, I guess.
+	     Modifications: perhaps require visiting an enode first,
+	     for "interesting" enodes, where enodes are
+	     (i) ones that weren't in the epath we found earlier
+	     (ii) ones whose predecessor has more than one out-edge
+	     (i.e. there was a choice beforehand)
+	     Does that get us further?
+	     Would be interesting to list those enodes, when logging.
+	     (and maybe highlight them in a custom dump?)
+	     Do we actually have to build a trimmed graph, or merely build
+	     the subset of interest?
+	     (and trimming could be confusing when dealing with indexes).  */
+	  // FIXME: find "interesting" enodes:
+	  unsigned i;
+	  exploded_node *iter_enode;
+	  exploded_path *best_concat_epath = NULL;
+	  exploded_node *best_iter_enode = NULL;
+	  FOR_EACH_VEC_ELT (m_eg.m_nodes, i, iter_enode)
+	    {
+	      if (iter_enode == enode)
+		continue;
+	      if (interesting_p (iter_enode))
+		{
+		  if (logger)
+		    logger->log ("EN: %i is interesting", iter_enode->m_index);
+		  // TODO
+		  // FIXME: also, this set is constant per EG; cache all of this
+		  /* Find the ones for which there's a feasible shortest path,
+		     and use these a prefixes.  */
+		  // FIXME: should we recurse here?
+		  exploded_path *prefix_epath
+		    = new exploded_path (m_sep.get_shortest_path (iter_enode));
+		  if (prefix_epath->feasible_p (logger, NULL,
+						m_eg.get_engine (), &m_eg))
+		    {
+		      if (logger)
+			logger->log ("prefix path to EN: %i is feasible",
+				     iter_enode->m_index);
+		      /* Find shortest path from iter_enode to ENODE.  */
+		      shortest_exploded_paths sep_from_prefix (m_eg, iter_enode);
+		      exploded_path suffix_epath
+			(sep_from_prefix.get_shortest_path (enode));
+
+		      /* Bail if no path exists.  */
+		      if (suffix_epath.length () == 0)
+			{
+			  if (logger)
+			    logger->log ("no suffix path from EN: %i to EN: %i",
+					 iter_enode->m_index, enode->m_index);
+			  continue;
+			}
+
+		      /* Concate prefix and suffix.  */
+		      exploded_path *concat_epath
+			= new exploded_path (*prefix_epath, suffix_epath);
+		      if (concat_epath->feasible_p (logger, NULL,
+						    m_eg.get_engine (), &m_eg))
+			{
+			  if (logger)
+			    logger->log ("concat path via EN: %i is feasible",
+					 iter_enode->m_index);
+			  if (best_concat_epath)
+			    {
+			      if (concat_epath->length ()
+				  < best_concat_epath->length ())
+				{
+				  if (logger)
+				    logger->log
+				      ("length (%i) beats existing best (%i)",
+				       concat_epath->length (),
+				       best_concat_epath->length ());
+				  delete best_concat_epath;
+				  best_concat_epath = concat_epath;
+				  best_iter_enode = iter_enode;
+				}
+			      else
+				{
+				  if (logger)
+				    logger->log
+				      ("length (%i) does not beat"
+				       " existing best (%i)",
+				       concat_epath->length (),
+				       best_concat_epath->length ());
+				  delete concat_epath;
+				}
+			    }
+			  else
+			    {
+			      if (logger)
+				logger->log
+				  ("first feasible concat path (length: %i)",
+				   concat_epath->length ());
+			      best_concat_epath = concat_epath;
+			      best_iter_enode = iter_enode;
+			    }
+			}
+		      else
+			{
+			  if (logger)
+			    logger->log
+			      ("concat path via EN: %i is not feasible",
+			       iter_enode->m_index);
+			  delete concat_epath;
+			}
+
+		      /* Concat them; is it feasible?
+			 TODO: reuse the state for the endpoint of the
+			 prefix path. */
+		      // TODO
+		    }
+		  else
+		    {
+		      if (logger)
+			logger->log ("prefix path to EN: %i is not feasible",
+				     iter_enode->m_index);
+		    }
+		  delete prefix_epath;
+		}
+	    }
+
+	  if (best_concat_epath)
+	    {
+	      if (logger)
+		logger->log ("found a feasible concat epath to EN: %i"
+			     " via EN: %i",
+			     enode->m_index,
+			     best_iter_enode->m_index);
+	      delete *out_problem;
+	      *out_problem = NULL;
+	      return best_concat_epath;
+	    }
+	  else
+	    {
+	      if (logger)
+		logger->log ("did not find a feasible concat epath to EN: %i",
+			     enode->m_index);
+	      return NULL;
+	    }
 	}
       else
 	{
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 76e2069c935..0d7af59f337 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -3411,6 +3411,10 @@ exploded_graph::to_json () const
   return egraph_obj;
 }
 
+/* class exploded_path.  */
+
+/* Copy ctor.  */
+
 exploded_path::exploded_path (const exploded_path &other)
 : m_edges (other.m_edges.length ())
 {
@@ -3420,6 +3424,23 @@ exploded_path::exploded_path (const exploded_path &other)
     m_edges.quick_push (eedge);
 }
 
+/* Construct by concatenating two existing paths,
+   which must be both non-empty, and join up.  */
+
+exploded_path::exploded_path (const exploded_path &prefix,
+			      const exploded_path &suffix)
+{
+  gcc_assert (prefix.m_edges.length () > 0);
+  gcc_assert (suffix.m_edges.length () > 0);
+  /* They must join up i.e. the end node of the final edge in PREFIX
+     must be the start node of the first edge in SUFFIX.  */
+  gcc_assert (prefix.m_edges[prefix.m_edges.length () - 1]->m_dest
+	      == suffix.m_edges[0]->m_src);
+  m_edges.reserve (prefix.m_edges.length () + suffix.m_edges.length ());
+  m_edges.splice (prefix.m_edges);
+  m_edges.splice (suffix.m_edges);
+}
+
 /* Look for the last use of SEARCH_STMT within this path.
    If found write the edge's index to *OUT_IDX and return true, otherwise
    return false.  */
diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h
index bcf58557de2..0a5e0f58a81 100644
--- a/gcc/analyzer/exploded-graph.h
+++ b/gcc/analyzer/exploded-graph.h
@@ -867,6 +867,8 @@ class exploded_path
 public:
   exploded_path () : m_edges () {}
   exploded_path (const exploded_path &other);
+  exploded_path (const exploded_path &prefix,
+		 const exploded_path &suffix);
 
   unsigned length () const { return m_edges.length (); }
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/feasibility-1.c b/gcc/testsuite/gcc.dg/analyzer/feasibility-1.c
index c96844467a5..0822bb499df 100644
--- a/gcc/testsuite/gcc.dg/analyzer/feasibility-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/feasibility-1.c
@@ -55,8 +55,7 @@ int test_6 (int a, int b)
     {
       if (!problem)
 	problem = 2;
-      __analyzer_dump_path (); /* { dg-message "path" "" { xfail *-*-* } } */
-      /* XFAIL is PR analyzer/96374.  */
+      __analyzer_dump_path (); /* { dg-message "path" } */
     }
   return problem;
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr93355-localealias-feasibility-2.c b/gcc/testsuite/gcc.dg/analyzer/pr93355-localealias-feasibility-2.c
index 1afc6df5da1..148429768cd 100644
--- a/gcc/testsuite/gcc.dg/analyzer/pr93355-localealias-feasibility-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr93355-localealias-feasibility-2.c
@@ -25,7 +25,5 @@ _nl_expand_alias (void)
     ++locale_alias_path;
 
   if (start < locale_alias_path)
-    __analyzer_dump_path (); /* { dg-message "path" "" { xfail *-*-* } } */
-  /* XFAIL: PR analyzer/96374
-     Use -fno-analyzer-feasibility to see the path.  */
+    __analyzer_dump_path (); /* { dg-message "path" } */
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr93355-localealias-feasibility-3.c b/gcc/testsuite/gcc.dg/analyzer/pr93355-localealias-feasibility-3.c
index a86483113ff..50d338855bc 100644
--- a/gcc/testsuite/gcc.dg/analyzer/pr93355-localealias-feasibility-3.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr93355-localealias-feasibility-3.c
@@ -41,9 +41,7 @@ read_alias_file (const char *fname, char *cp)
 {
   FILE *fp;
 
-  fp = fopen (fname, "r"); /* { dg-message "opened here" "" { xfail *-*-* } } */
-  /* XFAIL: PR analyzer/96374
-     Use -fno-analyzer-feasibility to see the path.  */
+  fp = fopen (fname, "r"); /* { dg-message "opened here" } */
   if (fp == NULL)
     return 0;
 
@@ -54,9 +52,7 @@ read_alias_file (const char *fname, char *cp)
     ++cp;
 
   if (cp[0] != '\0')
-    return 42; /* { dg-warning "leak of FILE 'fp'" "" { xfail *-*-* } } */
-  /* XFAIL: PR analyzer/96374
-     Use -fno-analyzer-feasibility to see the path.  */
+    return 42; /* { dg-warning "leak of FILE 'fp'" } */
 
   fclose(fp);
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/unknown-fns-4.c b/gcc/testsuite/gcc.dg/analyzer/unknown-fns-4.c
index 3d8f82ee290..bd1ab1e5476 100644
--- a/gcc/testsuite/gcc.dg/analyzer/unknown-fns-4.c
+++ b/gcc/testsuite/gcc.dg/analyzer/unknown-fns-4.c
@@ -10,6 +10,6 @@ void test (void)
 	got = 1;
       else
 	if (got)
-	  __analyzer_dump_path (); /* { dg-message "path" "" { xfail *-*-* } } */
+	  __analyzer_dump_path (); /* { dg-message "path" } */
     }
 }
-- 
2.26.2

