MatCat.OpenSource/8sa1-gcc
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
David Malcolm 2021-02-01 21:54:11 -05:00
parent f2f639c4a7
commit 8a2750086d
9 changed files with 69 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
gcc/analyzer/engine.cc
View File

@ -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.

8
gcc/testsuite/gcc.dg/analyzer/conditionals-3.c
View File

@ -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);
}
}

4
gcc/testsuite/gcc.dg/analyzer/data-model-1.c
View File

@ -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" } */
}

26
gcc/testsuite/gcc.dg/analyzer/feasibility-1.c
View File

@ -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;
}

4
gcc/testsuite/gcc.dg/analyzer/params.c
View File

@ -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 } */

4
gcc/testsuite/gcc.dg/analyzer/pr96651-2.c
View File

@ -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);

18
gcc/testsuite/gcc.dg/analyzer/signal-4b.c
View File

@ -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);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~

8
gcc/testsuite/gcc.dg/analyzer/single-field.c
View File

@ -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);
}

8
gcc/testsuite/gcc.dg/analyzer/torture/conditionals-2.c
View File

@ -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);
}