The analyzer's initial worklist was only populated with non-static
functions in the TU (along with those that look promising for call
summaries). Hence some static functions that were never explicitly
called but could be called via function pointers were not being
analyzed.
This patch remedies this by ensuring that functions that escape as
function pointers get added to the worklist, if they haven't been
already. Another fix would be to simply analyze all functions that
we have a body for, but too much of the testsuite relies on static
test functions not being directly analyzed.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as r11-3840-gaf66094d037793773eb8a49597866457f2f6a104.
gcc/analyzer/ChangeLog:
PR analyzer/97258
* engine.cc (impl_region_model_context::on_escaped_function): New
vfunc.
(exploded_graph::add_function_entry): Use m_functions_with_enodes
to implement idempotency.
(add_any_callbacks): New.
(exploded_graph::build_initial_worklist): Use the above to find
callbacks that are reachable from global initializers.
(exploded_graph::on_escaped_function): New.
* exploded-graph.h
(impl_region_model_context::on_escaped_function): New decl.
(exploded_graph::on_escaped_function): New decl.
(exploded_graph::m_functions_with_enodes): New field.
* region-model-reachability.cc
(reachable_regions::reachable_regions): Replace "store" param with
"model" param; use it to initialize m_model.
(reachable_regions::add): When getting the svalue for the region,
call get_store_value on the model rather than using an initial
value.
(reachable_regions::mark_escaped_clusters): Add ctxt param and
use it to call on_escaped_function when a function_region escapes.
* region-model-reachability.h
(reachable_regions::reachable_regions): Replace "store" param with
"model" param.
(reachable_regions::mark_escaped_clusters): Add ctxt param.
(reachable_regions::m_model): New field.
* region-model.cc (region_model::handle_unrecognized_call): Update
for change in reachable_regions ctor.
(region_model::handle_unrecognized_call): Pass ctxt to
mark_escaped_clusters.
(region_model::get_reachable_svalues): Update for change in
reachable_regions ctor.
(region_model::get_initial_value_for_global): Read-only variables
keep their initial values.
* region-model.h (region_model_context::on_escaped_function): New
vfunc.
(noop_region_model_context::on_escaped_function): New.
gcc/testsuite/ChangeLog:
PR analyzer/97258
* gcc.dg/analyzer/callbacks-1.c: New test.
* gcc.dg/analyzer/callbacks-2.c: New test.
* gcc.dg/analyzer/callbacks-3.c: New test.
---
gcc/analyzer/engine.cc | 70 +++++++++++++++++++++
gcc/analyzer/exploded-graph.h | 8 +++
gcc/analyzer/region-model-reachability.cc | 19 ++++--
gcc/analyzer/region-model-reachability.h | 8 ++-
gcc/analyzer/region-model.cc | 13 ++--
gcc/analyzer/region-model.h | 5 ++
gcc/testsuite/gcc.dg/analyzer/callbacks-1.c | 25 ++++++++
gcc/testsuite/gcc.dg/analyzer/callbacks-2.c | 22 +++++++
gcc/testsuite/gcc.dg/analyzer/callbacks-3.c | 19 ++++++
9 files changed, 175 insertions(+), 14 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/callbacks-1.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/callbacks-2.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/callbacks-3.c
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 0e79254ad60..65d7495f26f 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -143,6 +143,12 @@ impl_region_model_context::on_unknown_change (const svalue
*sval,
smap->on_unknown_change (sval, is_mutable, m_ext_state);
}
+void
+impl_region_model_context::on_escaped_function (tree fndecl)
+{
+ m_eg->on_escaped_function (fndecl);
+}
+
/* class setjmp_svalue : public svalue. */
/* Implementation of svalue::accept vfunc for setjmp_svalue. */
@@ -1931,6 +1937,15 @@ exploded_graph::~exploded_graph ()
exploded_node *
exploded_graph::add_function_entry (function *fun)
{
+ /* Be idempotent. */
+ if (m_functions_with_enodes.contains (fun))
+ {
+ logger * const logger = get_logger ();
+ if (logger)
+ logger->log ("entrypoint for %qE already exists", fun->decl);
+ return NULL;
+ }
+
program_point point = program_point::from_function_entry (m_sg, fun);
program_state state (m_ext_state);
state.push_frame (m_ext_state, fun);
@@ -1942,6 +1957,9 @@ exploded_graph::add_function_entry (function *fun)
/* We should never fail to add such a node. */
gcc_assert (enode);
add_edge (m_origin, enode, NULL);
+
+ m_functions_with_enodes.add (fun);
+
return enode;
}
@@ -2261,6 +2279,18 @@ 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. */
@@ -2286,6 +2316,19 @@ 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.
@@ -3923,6 +3966,33 @@ exploded_graph::get_node_by_index (int idx) const
return enode;
}
+/* Ensure that there is an exploded_node for a top-level call to FNDECL. */
+
+void
+exploded_graph::on_escaped_function (tree fndecl)
+{
+ logger * const logger = get_logger ();
+ LOG_FUNC_1 (logger, "%qE", fndecl);
+
+ cgraph_node *cgnode = cgraph_node::get (fndecl);
+ if (!cgnode)
+ return;
+
+ function *fun = cgnode->get_fun ();
+ if (!fun)
+ return;
+
+ exploded_node *enode = add_function_entry (fun);
+ if (logger)
+ {
+ if (enode)
+ logger->log ("created EN %i for %qE entrypoint",
+ enode->m_index, fun->decl);
+ else
+ logger->log ("did not create enode for %qE entrypoint", fun->decl);
+ }
+}
+
/* A collection of classes for visualizing the callgraph in .dot form
(as represented in the supergraph). */
diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h
index a6ca4b9a99d..b207b1d0762 100644
--- a/gcc/analyzer/exploded-graph.h
+++ b/gcc/analyzer/exploded-graph.h
@@ -66,6 +66,8 @@ class impl_region_model_context : public region_model_context
void on_unexpected_tree_code (tree t,
const dump_location_t &loc) FINAL OVERRIDE;
+ void on_escaped_function (tree fndecl) FINAL OVERRIDE;
+
exploded_graph *m_eg;
log_user m_logger;
const exploded_node *m_enode_for_diag;
@@ -799,6 +801,8 @@ public:
return m_worklist.get_scc_id (node);
}
+ void on_escaped_function (tree fndecl);
+
private:
void print_bar_charts (pretty_printer *pp) const;
@@ -845,6 +849,10 @@ private:
call_string_data_map_t m_per_call_string_data;
auto_vec<int> m_PK_AFTER_SUPERNODE_per_snode;
+
+ /* Functions with a top-level enode, to make add_function_entry
+ be idempotent, for use in handling callbacks. */
+ hash_set<function *> m_functions_with_enodes;
};
/* A path within an exploded_graph: a sequence of edges. */
diff --git a/gcc/analyzer/region-model-reachability.cc
b/gcc/analyzer/region-model-reachability.cc
index c1b3b2db630..3a6b312a8df 100644
--- a/gcc/analyzer/region-model-reachability.cc
+++ b/gcc/analyzer/region-model-reachability.cc
@@ -58,9 +58,9 @@ along with GCC; see the file COPYING3. If not see
namespace ana {
-reachable_regions::reachable_regions (store *store,
+reachable_regions::reachable_regions (region_model *model,
region_model_manager *mgr)
-: m_store (store), m_mgr (mgr),
+: m_model (model), m_store (model->get_store ()), m_mgr (mgr),
m_reachable_base_regs (), m_mutable_base_regs ()
{
}
@@ -135,7 +135,7 @@ reachable_regions::add (const region *reg, bool is_mutable)
if (binding_cluster *bind_cluster = m_store->get_cluster (base_reg))
bind_cluster->for_each_value (handle_sval_cb, this);
else
- handle_sval (m_mgr->get_or_create_initial_value (base_reg));
+ handle_sval (m_model->get_store_value (reg));
}
void
@@ -206,17 +206,24 @@ reachable_regions::handle_parm (const svalue *sval, tree
param_type)
}
}
-/* Update m_store to mark the clusters that were found to be mutable
- as having escaped. */
+/* Update the store to mark the clusters that were found to be mutable
+ as having escaped.
+ Notify CTXT about escaping function_decls. */
void
-reachable_regions::mark_escaped_clusters ()
+reachable_regions::mark_escaped_clusters (region_model_context *ctxt)
{
+ gcc_assert (ctxt);
for (hash_set<const region *>::iterator iter = m_mutable_base_regs.begin ();
iter != m_mutable_base_regs.end (); ++iter)
{
const region *base_reg = *iter;
m_store->mark_as_escaped (base_reg);
+
+ /* If we have a function that's escaped, potentially add
+ it to the worklist. */
+ if (const function_region *fn_reg = base_reg->dyn_cast_function_region
())
+ ctxt->on_escaped_function (fn_reg->get_fndecl ());
}
}
diff --git a/gcc/analyzer/region-model-reachability.h
b/gcc/analyzer/region-model-reachability.h
index aba1a58ec7f..fe305b0ce78 100644
--- a/gcc/analyzer/region-model-reachability.h
+++ b/gcc/analyzer/region-model-reachability.h
@@ -35,7 +35,7 @@ namespace ana {
class reachable_regions
{
public:
- reachable_regions (store *store, region_model_manager *mgr);
+ reachable_regions (region_model *model, region_model_manager *mgr);
/* Callback called for each cluster when initializing this object. */
static void init_cluster_cb (const region *base_reg,
@@ -59,8 +59,9 @@ public:
void handle_parm (const svalue *sval, tree param_type);
/* Update the store to mark the clusters that were found to be mutable
- as having escaped. */
- void mark_escaped_clusters ();
+ as having escaped.
+ Notify CTXT about escaping function_decls. */
+ void mark_escaped_clusters (region_model_context *ctxt);
/* Iteration over reachable base regions. */
hash_set<const region *>::iterator begin ()
@@ -94,6 +95,7 @@ public:
DEBUG_FUNCTION void dump () const;
private:
+ region_model *m_model;
store *m_store;
region_model_manager *m_mgr;
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 480f25a3a4b..922e0361e59 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -836,7 +836,7 @@ region_model::handle_unrecognized_call (const gcall *call,
{
tree fndecl = get_fndecl_for_call (call, ctxt);
- reachable_regions reachable_regs (&m_store, m_mgr);
+ reachable_regions reachable_regs (this, m_mgr);
/* Determine the reachable regions and their mutability. */
{
@@ -884,7 +884,7 @@ region_model::handle_unrecognized_call (const gcall *call,
}
/* Mark any clusters that have escaped. */
- reachable_regs.mark_escaped_clusters ();
+ reachable_regs.mark_escaped_clusters (ctxt);
/* Update bindings for all clusters that have escaped, whether above,
or previously. */
@@ -904,7 +904,7 @@ void
region_model::get_reachable_svalues (svalue_set *out,
const svalue *extra_sval)
{
- reachable_regions reachable_regs (&m_store, m_mgr);
+ reachable_regions reachable_regs (this, m_mgr);
/* Add globals and regions that already escaped in previous
unknown calls. */
@@ -1333,14 +1333,17 @@ region_model::get_initial_value_for_global (const
region *reg) const
an unknown value if an unknown call has occurred, unless this is
static to-this-TU and hasn't escaped. Globals that have escaped
are explicitly tracked, so we shouldn't hit this case for them. */
- if (m_store.called_unknown_fn_p () && TREE_PUBLIC (decl))
+ if (m_store.called_unknown_fn_p ()
+ && TREE_PUBLIC (decl)
+ && !TREE_READONLY (decl))
return m_mgr->get_or_create_unknown_svalue (reg->get_type ());
/* If we are on a path from the entrypoint from "main" and we have a
global decl defined in this TU that hasn't been touched yet, then
the initial value of REG can be taken from the initialization value
of the decl. */
- if (called_from_main_p () && !DECL_EXTERNAL (decl))
+ if ((called_from_main_p () && !DECL_EXTERNAL (decl))
+ || TREE_READONLY (decl))
{
/* Get the initializer value for base_reg. */
const svalue *base_reg_init
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 234ca16bcef..5ad4a492f4f 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -2795,6 +2795,9 @@ class region_model_context
know how to handle the tree code of T at LOC. */
virtual void on_unexpected_tree_code (tree t,
const dump_location_t &loc) = 0;
+
+ /* Hook for clients to be notified when a function_decl escapes. */
+ virtual void on_escaped_function (tree fndecl) = 0;
};
/* A "do nothing" subclass of region_model_context. */
@@ -2821,6 +2824,8 @@ public:
{
}
void on_unexpected_tree_code (tree, const dump_location_t &) OVERRIDE {}
+
+ void on_escaped_function (tree) OVERRIDE {}
};
/* A subclass of region_model_context for determining if operations fail
diff --git a/gcc/testsuite/gcc.dg/analyzer/callbacks-1.c
b/gcc/testsuite/gcc.dg/analyzer/callbacks-1.c
new file mode 100644
index 00000000000..52c8fde540a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/callbacks-1.c
@@ -0,0 +1,25 @@
+/* Reproducer for PR analyzer/97258: we should report the double-free
+ inside a static callback if the callback escapes. */
+
+#include <stdlib.h>
+
+static void callback_1 (void *p)
+{
+ free (p);
+ free (p); /* { dg-warning "double-'free' of 'p'" } */
+}
+
+struct ops {
+ void (*cb) (void *);
+};
+
+static const struct ops ops_1 = {
+ .cb = callback_1
+};
+
+extern void registration (const void *);
+
+void register_1 (void)
+{
+ registration (&ops_1);
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/callbacks-2.c
b/gcc/testsuite/gcc.dg/analyzer/callbacks-2.c
new file mode 100644
index 00000000000..98915ee617b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/callbacks-2.c
@@ -0,0 +1,22 @@
+/* Reproducer for PR analyzer/97258: we should report the double-free
+ inside a static callback if the callback is accessible via a global
+ initializer. */
+
+#include <stdlib.h>
+
+static void callback_1 (void *p)
+{
+ free (p);
+ free (p); /* { dg-warning "double-'free' of 'p'" } */
+}
+
+struct ops {
+ void (*cb) (void *);
+};
+
+/* Callback struct is not static, and so could be accessed via
+ another TU. */
+
+const struct ops ops_1 = {
+ .cb = callback_1
+};
diff --git a/gcc/testsuite/gcc.dg/analyzer/callbacks-3.c
b/gcc/testsuite/gcc.dg/analyzer/callbacks-3.c
new file mode 100644
index 00000000000..5f12c2a28d3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/callbacks-3.c
@@ -0,0 +1,19 @@
+#include "analyzer-decls.h"
+
+typedef __SIZE_TYPE__ size_t;
+typedef int (*__compar_fn_t)(const void *, const void *);
+extern void qsort(void *__base, size_t __nmemb, size_t __size,
+ __compar_fn_t __compar)
+ __attribute__((__nonnull__(1, 4)));
+
+static int
+test_1_callback (const void *p1, const void *p2)
+{
+ __analyzer_dump_path (); /* { dg-message "here" } */
+ return 0;
+}
+
+void test_1_caller (int *arr, size_t n)
+{
+ qsort (arr, n, sizeof (int), test_1_callback);
+}
--
2.26.2
