analyzer: directly explore within static functions [PR93355,PR96374]

PR analyzer/93355 tracks that -fanalyzer fails to report the FILE *
leak in read_alias_file in intl/localealias.c.

One reason for the failure is that read_alias_file is marked as
"static", and the path leading to the single call of
read_alias_file is falsely rejected as infeasible due to
PR analyzer/96374.  I have been attempting to fix that bug, but
don't have a good solution yet.

Previously, -fanalyzer only directly explored "static" functions
if they were needed for call summaries, instead forcing them to
be indirectly explored, but if we have a feasibility bug like
above, we will fail to report any issues in a function that's
only called by such a falsely infeasible path.

It now seems wrong to me to reject directly exploring static
functions: even if there is currently no way to call a function,
it seems reasonable to warn about bugs within them, since
otherwise these latent bugs are a timebomb in the code.

Hence this patch reworks toplevel_function_p to directly explore
almost all functions, working around these feasiblity issues.
It introduces a naming convention that "__analyzer_"-prefixed
function names don't get directly explored, since this is
useful in the analyzer's DejaGnu-based tests.

This workaround gets PR analyzer/93355 closer to working, but
unfortunately there is a second instance of PR analyzer/96374
within read_alias_file itself which means even with this patch
-fanalyzer falsely rejects the path as infeasible.

Still, this ought to help in other cases, and simplifies the
implementation.

gcc/analyzer/ChangeLog:
	PR analyzer/93355
	PR analyzer/96374
	* engine.cc (toplevel_function_p): Simplify so that
	we only reject functions with a "__analyzer_" prefix.
	(add_any_callbacks): Delete.
	(exploded_graph::build_initial_worklist): Update for
	dropped param of toplevel_function_p.
	(exploded_graph::build_initial_worklist): Don't bother
	looking for callbacks that are reachable from global
	initializers.

gcc/testsuite/ChangeLog:
	PR analyzer/93355
	PR analyzer/96374
	* gcc.dg/analyzer/conditionals-3.c: Add "__analyzer_"
	prefix to support subroutines where necessary.
	* gcc.dg/analyzer/data-model-1.c: Likewise.
	* gcc.dg/analyzer/feasibility-1.c (called_by_test_6a): New.
	(test_6a): New.
	* gcc.dg/analyzer/params.c: Add "__analyzer_" prefix to support
	subroutines where necessary.
	* gcc.dg/analyzer/pr96651-2.c: Likewise.
	* gcc.dg/analyzer/signal-4b.c: Likewise.
	* gcc.dg/analyzer/single-field.c: Likewise.
	* gcc.dg/analyzer/torture/conditionals-2.c: Likewise.

9 files changed, 69 insertions, 79 deletions

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index fc81e75..45aed8f 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -2348,38 +2348,27 @@ exploded_graph::get_per_function_data (function *fun) const
   return NULL;
 }
 
-/* Return true if NODE and FUN should be traversed directly, rather than
+/* Return true if FUN should be traversed directly, rather than only as
    called via other functions.  */
 
 static bool
-toplevel_function_p (cgraph_node *node, function *fun, logger *logger)
+toplevel_function_p (function *fun, logger *logger)
 {
-  /* TODO: better logic here
-     e.g. only if more than one caller, and significantly complicated.
-     Perhaps some whole-callgraph analysis to decide if it's worth summarizing
-     an edge, and if so, we need summaries.  */
-  if (flag_analyzer_call_summaries)
-    {
-      int num_call_sites = 0;
-      for (cgraph_edge *edge = node->callers; edge; edge = edge->next_caller)
-	++num_call_sites;
-
-      /* For now, if there's more than one in-edge, and we want call
-	 summaries, do it at the top level so that there's a chance
-	 we'll have a summary when we need one.  */
-      if (num_call_sites > 1)
-	{
-	  if (logger)
-	    logger->log ("traversing %qE (%i call sites)",
-			 fun->decl, num_call_sites);
-	  return true;
-	}
-    }
-
-  if (!TREE_PUBLIC (fun->decl))
+  /* Don't directly traverse into functions that have an "__analyzer_"
+     prefix.  Doing so is useful for the analyzer test suite, allowing
+     us to have functions that are called in traversals, but not directly
+     explored, thus testing how the analyzer handles calls and returns.
+     With this, we can have DejaGnu directives that cover just the case
+     of where a function is called by another function, without generating
+     excess messages from the case of the first function being traversed
+     directly.  */
+#define ANALYZER_PREFIX "__analyzer_"
+  if (!strncmp (IDENTIFIER_POINTER (DECL_NAME (fun->decl)), ANALYZER_PREFIX,
+		strlen (ANALYZER_PREFIX)))
     {
       if (logger)
-	logger->log ("not traversing %qE (static)", fun->decl);
+	logger->log ("not traversing %qE (starts with %qs)",
+		     fun->decl, ANALYZER_PREFIX);
       return false;
     }
 
@@ -2389,18 +2378,6 @@ toplevel_function_p (cgraph_node *node, function *fun, logger *logger)
   return true;
 }
 
-/* Callback for walk_tree for finding callbacks within initializers;
-   ensure they are treated as possible entrypoints to the analysis.  */
-
-static tree
-add_any_callbacks (tree *tp, int *, void *data)
-{
-  exploded_graph *eg = (exploded_graph *)data;
-  if (TREE_CODE (*tp) == FUNCTION_DECL)
-    eg->on_escaped_function (*tp);
-  return NULL_TREE;
-}
-
 /* Add initial nodes to EG, with entrypoints for externally-callable
    functions.  */
 
@@ -2414,7 +2391,7 @@ exploded_graph::build_initial_worklist ()
   FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
   {
     function *fun = node->get_fun ();
-    if (!toplevel_function_p (node, fun, logger))
+    if (!toplevel_function_p (fun, logger))
       continue;
     exploded_node *enode = add_function_entry (fun);
     if (logger)
@@ -2426,19 +2403,6 @@ exploded_graph::build_initial_worklist ()
 	  logger->log ("did not create enode for %qE entrypoint", fun->decl);
       }
   }
-
-  /* Find callbacks that are reachable from global initializers.  */
-  varpool_node *vpnode;
-  FOR_EACH_VARIABLE (vpnode)
-    {
-      tree decl = vpnode->decl;
-      if (!TREE_PUBLIC (decl))
-	continue;
-      tree init = DECL_INITIAL (decl);
-      if (!init)
-	continue;
-      walk_tree (&init, add_any_callbacks, this, NULL);
-    }
 }
 
 /* The main loop of the analysis.
diff --git a/gcc/testsuite/gcc.dg/analyzer/conditionals-3.c b/gcc/testsuite/gcc.dg/analyzer/conditionals-3.c
index 5f29f21..f1c6c20 100644
--- a/gcc/testsuite/gcc.dg/analyzer/conditionals-3.c
+++ b/gcc/testsuite/gcc.dg/analyzer/conditionals-3.c
@@ -2,12 +2,12 @@
 
 #include "analyzer-decls.h"
 
-static void only_called_when_flag_a_true (int i)
+static void __analyzer_only_called_when_flag_a_true (int i)
 {
   __analyzer_eval (i == 42); /* { dg-warning "TRUE" } */
 }
 
-static void only_called_when_flag_b_true (int i)
+static void __analyzer_only_called_when_flag_b_true (int i)
 {
   __analyzer_eval (i == 17); /* { dg-warning "TRUE" } */
 }
@@ -34,7 +34,7 @@ int test_1 (int flag_a, int flag_b)
       __analyzer_eval (flag_b); /* { dg-warning "UNKNOWN" } */
       __analyzer_eval (i == 42); /* { dg-warning "TRUE" } */
       __analyzer_eval (i == 17); /* { dg-warning "FALSE" } */
-      only_called_when_flag_a_true (i);
+      __analyzer_only_called_when_flag_a_true (i);
     }  
   else
     {
@@ -42,6 +42,6 @@ int test_1 (int flag_a, int flag_b)
       __analyzer_eval (flag_b); /* { dg-warning "UNKNOWN" } */
       __analyzer_eval (i == 42); /* { dg-warning "FALSE" } */
       __analyzer_eval (i == 17); /* { dg-warning "TRUE" } */
-      only_called_when_flag_b_true (i);
+      __analyzer_only_called_when_flag_b_true (i);
     }
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
index f6681b6..afd1556 100644
--- a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
@@ -782,7 +782,7 @@ void test_33 (void)
 }
 
 static int __attribute__((noinline))
-only_called_by_test_34 (int parm)
+__analyzer_only_called_by_test_34 (int parm)
 {
   __analyzer_eval (parm == 42); /* { dg-warning "TRUE" } */
 
@@ -791,7 +791,7 @@ only_called_by_test_34 (int parm)
 
 void test_34 (void)
 {
-  int result = only_called_by_test_34 (42);
+  int result = __analyzer_only_called_by_test_34 (42);
   __analyzer_eval (result == 84); /* { dg-warning "TRUE" } */
 }
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/feasibility-1.c b/gcc/testsuite/gcc.dg/analyzer/feasibility-1.c
index f2a8a4c..c968444 100644
--- a/gcc/testsuite/gcc.dg/analyzer/feasibility-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/feasibility-1.c
@@ -60,3 +60,29 @@ int test_6 (int a, int b)
     }
   return problem;
 }
+
+/* As above, but call a static function.
+   Even if the path to the call of called_by_test_6a is falsely rejected
+   as infeasible, it still makes sense to complain about errors within
+   the called function.  */
+
+static void __attribute__((noinline))
+called_by_test_6a (void *ptr)
+{
+  __builtin_free (ptr);
+  __builtin_free (ptr); /* { dg-message "double-'free'" } */
+}
+
+int test_6a (int a, int b, void *ptr)
+{
+  int problem = 0;
+  if (a)
+    problem = 1;
+  if (b)
+    {
+      if (!problem)
+	problem = 2;
+      called_by_test_6a (ptr);
+    }
+  return problem;
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/params.c b/gcc/testsuite/gcc.dg/analyzer/params.c
index f8331dd..12bba70 100644
--- a/gcc/testsuite/gcc.dg/analyzer/params.c
+++ b/gcc/testsuite/gcc.dg/analyzer/params.c
@@ -1,6 +1,6 @@
 #include "analyzer-decls.h"
 
-static int called_function(int j)
+static int __analyzer_called_function(int j)
 {
   int k;
 
@@ -23,7 +23,7 @@ void test(int i)
 
     __analyzer_eval (i > 4); /* { dg-warning "TRUE" } */
 
-    i = called_function(i);
+    i = __analyzer_called_function(i);
 
     __analyzer_eval (i > 3); /* { dg-warning "TRUE" "desired" { xfail *-*-* } } */
     /* { dg-warning "UNKNOWN" "status quo" { target *-*-* } .-1 } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr96651-2.c b/gcc/testsuite/gcc.dg/analyzer/pr96651-2.c
index 249a32b..25cda37 100644
--- a/gcc/testsuite/gcc.dg/analyzer/pr96651-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr96651-2.c
@@ -26,7 +26,7 @@ void test (void)
 }
 
 static void __attribute__((noinline))
-called_from_main (void)
+__analyzer_called_from_main (void)
 {
   /* When accessed from main, the vars still have their initializer values.  */
   __analyzer_eval (a == 0); /* { dg-warning "TRUE" } */
@@ -53,7 +53,7 @@ int main (void)
      before "main").  */
   __analyzer_eval (stderr == 0); /* { dg-warning "UNKNOWN" } */
 
-  called_from_main ();
+  __analyzer_called_from_main ();
 
   unknown_fn (&a, &c);
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/signal-4b.c b/gcc/testsuite/gcc.dg/analyzer/signal-4b.c
index cb1e7e47..5a2ccb1 100644
--- a/gcc/testsuite/gcc.dg/analyzer/signal-4b.c
+++ b/gcc/testsuite/gcc.dg/analyzer/signal-4b.c
@@ -20,14 +20,14 @@ static void int_handler(int signum)
   custom_logger("got signal");
 }
 
-static void register_handler ()
+static void __analyzer_register_handler ()
 {
   signal(SIGINT, int_handler);
 }
 
 void test (void)
 {
-  register_handler ();
+  __analyzer_register_handler ();
   body_of_program();
 }
 
@@ -42,17 +42,17 @@ void test (void)
     |      |      |
     |      |      (1) entry to 'test'
     |   NN | {
-    |   NN |   register_handler ();
-    |      |   ~~~~~~~~~~~~~~~~~~~
+    |   NN |   __analyzer_register_handler ();
+    |      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     |      |   |
-    |      |   (2) calling 'register_handler' from 'test'
+    |      |   (2) calling '__analyzer_register_handler' from 'test'
     |
-    +--> 'register_handler': events 3-4
+    +--> '__analyzer_register_handler': events 3-4
            |
-           |   NN | static void register_handler ()
-           |      |             ^~~~~~~~~~~~~~~~
+           |   NN | static void __analyzer_register_handler ()
+           |      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~
            |      |             |
-           |      |             (3) entry to 'register_handler'
+           |      |             (3) entry to '__analyzer_register_handler'
            |   NN | {
            |   NN |   signal(SIGINT, int_handler);
            |      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/gcc/testsuite/gcc.dg/analyzer/single-field.c b/gcc/testsuite/gcc.dg/analyzer/single-field.c
index d54cfb0..31c6fee 100644
--- a/gcc/testsuite/gcc.dg/analyzer/single-field.c
+++ b/gcc/testsuite/gcc.dg/analyzer/single-field.c
@@ -11,14 +11,14 @@ void test_1 (struct foo f)
   __analyzer_describe (0, f.ptr); /* { dg-warning "svalue: 'INIT_VAL\\(f.ptr\\)'" } */
 }
 
-static void called_by_test_2 (struct foo f_inner)
+static void __analyzer_called_by_test_2 (struct foo f_inner)
 {
   free (f_inner.ptr);
   free (f_inner.ptr); /* { dg-warning "double-'free' of 'f_outer.ptr'" } */
 }
 void test_2 (struct foo f_outer)
 {
-  called_by_test_2 (f_outer);
+  __analyzer_called_by_test_2 (f_outer);
 }
 
 struct nested
@@ -26,12 +26,12 @@ struct nested
   struct foo f;
 };
 
-static void called_by_test_3 (struct nested n_inner)
+static void __analyzer_called_by_test_3 (struct nested n_inner)
 {
   free (n_inner.f.ptr);
   free (n_inner.f.ptr); /* { dg-warning "double-'free' of 'n_outer.f.ptr'" } */
 }
 void test_3 (struct nested n_outer)
 {
-  called_by_test_3 (n_outer);
+  __analyzer_called_by_test_3 (n_outer);
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/conditionals-2.c b/gcc/testsuite/gcc.dg/analyzer/torture/conditionals-2.c
index 35b0a05f..278a2a5 100644
--- a/gcc/testsuite/gcc.dg/analyzer/torture/conditionals-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/torture/conditionals-2.c
@@ -5,7 +5,7 @@
 #define Z_NULL 0
 
 static void __attribute__((noinline))
-test_1_callee (void *p, void *q)
+__analyzer_test_1_callee (void *p, void *q)
 {
   __analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */
 
@@ -21,11 +21,11 @@ void test_1 (void *p, void *q)
   if (p == Z_NULL || q == Z_NULL)
     return;
 
-  test_1_callee (p, q);
+  __analyzer_test_1_callee (p, q);
 }
 
 static void __attribute__((noinline))
-test_2_callee (void *p, void *q)
+__analyzer_test_2_callee (void *p, void *q)
 {
   __analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */
 
@@ -39,5 +39,5 @@ test_2_callee (void *p, void *q)
 void test_2 (void *p, void *q)
 {
   if (p != Z_NULL && q != Z_NULL)
-    test_2_callee (p, q);
+    __analyzer_test_2_callee (p, q);
 }