gimple_call_fndecl (and gimple_call_addr_fndecl) fail to identify functions
for non-trivial uses of function pointers.
This patch eliminates most uses of these functions from the analyzer in
favor of resolving the rvalue for the fn ptr in the region_model, so that
with the patch it can e.g. detect the double free here:
#include <stdlib.h>
typedef void (*deallocator_t) (void *);
void test_1 (void *ptr)
{
deallocator_t dealloc_fn = free;
dealloc_fn (ptr);
dealloc_fn (ptr);
}
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to branch "dmalcolm/analyzer" on the GCC git mirror.
gcc/ChangeLog:
* analyzer/analyzer.cc (is_named_call_p): Reorganize arguments,
adding a fndecl. Drop second overload, and rename first overload
to...
(is_special_named_call_p): ...this.
(is_setjmp_call_p): Update for rename of is_named_call_p to
is_special_named_call_p.
(is_longjmp_call_p): Likewise.
* analyzer/analyzer.h (is_named_call_p): Reorganize arguments,
adding a fndecl. Drop second overload, and rename first overload
to...
(is_special_named_call_p): ...this.
* analyzer/engine.cc (impl_sm_context::get_fndecl_for_call): New
vfunc implementation.
(exploded_node::on_stmt): Update for rename of is_named_call_p to
is_special_named_call_p.
(stmt_requires_new_enode_p): Likewise.
(exploded_graph::dump_exploded_nodes): Likewise.
* analyzer/region-model.cc (region_model::on_call_pre): Likewise.
Replace most calls to is_named_call_p with the new overload taking
a fndecl, calling get_fndecl_for_call to resolve function pointers.
(region_model::on_call_post): Likewise.
(region_model::get_fndecl_for_call): New member function.
* analyzer/region-model.h (region_model::get_fndecl_for_call): New
decl.
* analyzer/sm-file.cc (fileptr_state_machine::on_stmt): Call
get_fndecl_for_call to resolve function pointers, and update all
uses of is_named_call_p to use the result.
* analyzer/sm-malloc.cc (free_of_non_heap::describe_state_change):
Update for rename of is_named_call_p to is_special_named_call_p.
(malloc_state_machine::on_stmt): Call get_fndecl_for_call to
resolve function pointers, and update all uses of is_named_call_p
to use the result.
* analyzer/sm-sensitive.cc (sensitive_state_machine::on_stmt):
Likewise.
* analyzer/sm-taint.cc (taint_state_machine::on_stmt): Likewise.
* analyzer/sm.h (sm_context::get_fndecl_for_call): New vfunc.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/attribute-nonnull.c: Add coverage for detecting
passing NULL to a __attribute__((nonnull)) function called via a
function pointer.
* gcc.dg/analyzer/malloc-callbacks.c: New test.
---
gcc/analyzer/analyzer.cc | 38 +++-
gcc/analyzer/analyzer.h | 8 +-
gcc/analyzer/engine.cc | 24 ++-
gcc/analyzer/region-model.cc | 187 ++++++++++--------
gcc/analyzer/region-model.h | 3 +
gcc/analyzer/sm-file.cc | 78 ++++----
gcc/analyzer/sm-malloc.cc | 104 +++++-----
gcc/analyzer/sm-sensitive.cc | 55 +++---
gcc/analyzer/sm-taint.cc | 33 ++--
gcc/analyzer/sm.h | 6 +
.../gcc.dg/analyzer/attribute-nonnull.c | 24 +++
.../gcc.dg/analyzer/malloc-callbacks.c | 84 ++++++++
12 files changed, 415 insertions(+), 229 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/malloc-callbacks.c
diff --git a/gcc/analyzer/analyzer.cc b/gcc/analyzer/analyzer.cc
index 399925c96fc9..5405facf5e86 100644
--- a/gcc/analyzer/analyzer.cc
+++ b/gcc/analyzer/analyzer.cc
@@ -28,10 +28,18 @@ along with GCC; see the file COPYING3. If not see
#include "intl.h"
#include "analyzer/analyzer.h"
-/* Helper function for checkers. Is the CALL to the given function name? */
+/* Helper function for checkers. Is the CALL to the given function name,
+ and with the given number of arguments?
+
+ This doesn't resolve function pointers via the region model;
+ is_named_call_p should be used instead, using a fndecl from
+ get_fndecl_for_call; this function should only be used for special cases
+ where it's not practical to get at the region model, or for special
+ analyzer functions such as __analyzer_dump. */
bool
-is_named_call_p (const gcall *call, const char *funcname)
+is_special_named_call_p (const gcall *call, const char *funcname,
+ unsigned int num_args)
{
gcc_assert (funcname);
@@ -39,19 +47,31 @@ is_named_call_p (const gcall *call, const char *funcname)
if (!fndecl)
return false;
+ return is_named_call_p (fndecl, funcname, call, num_args);
+}
+
+/* Helper function for checkers. Does FNDECL have the given FUNCNAME? */
+
+bool
+is_named_call_p (tree fndecl, const char *funcname)
+{
+ gcc_assert (fndecl);
+ gcc_assert (funcname);
+
return 0 == strcmp (IDENTIFIER_POINTER (DECL_NAME (fndecl)), funcname);
}
-/* Helper function for checkers. Is the CALL to the given function name,
- and with the given number of arguments? */
+/* Helper function for checkers. Does FNDECL have the given FUNCNAME, and
+ does CALL have the given number of arguments? */
bool
-is_named_call_p (const gcall *call, const char *funcname,
- unsigned int num_args)
+is_named_call_p (tree fndecl, const char *funcname,
+ const gcall *call, unsigned int num_args)
{
+ gcc_assert (fndecl);
gcc_assert (funcname);
- if (!is_named_call_p (call, funcname))
+ if (!is_named_call_p (fndecl, funcname))
return false;
if (gimple_call_num_args (call) != num_args)
@@ -67,7 +87,7 @@ is_setjmp_call_p (const gimple *stmt)
{
/* TODO: is there a less hacky way to check for "setjmp"? */
if (const gcall *call = dyn_cast <const gcall *> (stmt))
- if (is_named_call_p (call, "_setjmp", 1))
+ if (is_special_named_call_p (call, "_setjmp", 1))
return true;
return false;
@@ -79,7 +99,7 @@ bool
is_longjmp_call_p (const gcall *call)
{
/* TODO: is there a less hacky way to check for "longjmp"? */
- if (is_named_call_p (call, "longjmp", 2))
+ if (is_special_named_call_p (call, "longjmp", 2))
return true;
return false;
diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h
index ace8924662f2..90da44b1a00a 100644
--- a/gcc/analyzer/analyzer.h
+++ b/gcc/analyzer/analyzer.h
@@ -68,9 +68,11 @@ class state_change;
////////////////////////////////////////////////////////////////////////////
-extern bool is_named_call_p (const gcall *call, const char *funcname);
-extern bool is_named_call_p (const gcall *call, const char *funcname,
- unsigned int num_args);
+extern bool is_special_named_call_p (const gcall *call, const char *funcname,
+ unsigned int num_args);
+extern bool is_named_call_p (tree fndecl, const char *funcname);
+extern bool is_named_call_p (tree fndecl, const char *funcname,
+ const gcall *call, unsigned int num_args);
extern bool is_setjmp_call_p (const gimple *stmt);
extern bool is_longjmp_call_p (const gcall *call);
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index b1624777a221..eed2be091c93 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -177,6 +177,15 @@ public:
{
}
+ tree get_fndecl_for_call (const gcall *call) FINAL OVERRIDE
+ {
+ impl_region_model_context old_ctxt
+ (m_eg, m_enode_for_diag, NULL, NULL/*m_enode->get_state ()*/,
+ m_change, call);
+ region_model *model = m_new_state->m_region_model;
+ return model->get_fndecl_for_call (call, &old_ctxt);
+ }
+
void on_transition (const supernode *node ATTRIBUTE_UNUSED,
const gimple *stmt ATTRIBUTE_UNUSED,
tree var,
@@ -881,25 +890,25 @@ exploded_node::on_stmt (exploded_graph &eg,
if (const gcall *call = dyn_cast <const gcall *> (stmt))
{
/* Debugging/test support. */
- if (is_named_call_p (call, "__analyzer_dump", 0))
+ if (is_special_named_call_p (call, "__analyzer_dump", 0))
{
/* Handle the builtin "__analyzer_dump" by dumping state
to stderr. */
dump (eg.get_ext_state ());
}
- else if (is_named_call_p (call, "__analyzer_dump_path", 0))
+ else if (is_special_named_call_p (call, "__analyzer_dump_path", 0))
{
/* Handle the builtin "__analyzer_dump_path" by queuing a
diagnostic at this exploded_node. */
ctxt.warn (new dump_path_diagnostic ());
}
- else if (is_named_call_p (call, "__analyzer_dump_region_model", 0))
+ else if (is_special_named_call_p (call, "__analyzer_dump_region_model",
0))
{
/* Handle the builtin "__analyzer_dump_region_model" by dumping
the region model's state to stderr. */
state->m_region_model->dump (false);
}
- else if (is_named_call_p (call, "__analyzer_eval", 1))
+ else if (is_special_named_call_p (call, "__analyzer_eval", 1))
{
/* Handle the builtin "__analyzer_eval" by evaluating the input
and dumping as a dummy warning, so that test cases can use
@@ -913,7 +922,7 @@ exploded_node::on_stmt (exploded_graph &eg,
&ctxt);
warning_at (call->location, 0, "%s", t.as_string ());
}
- else if (is_named_call_p (call, "__analyzer_break", 0))
+ else if (is_special_named_call_p (call, "__analyzer_break", 0))
{
/* Handle the builtin "__analyzer_break" by triggering a
breakpoint. */
@@ -2151,7 +2160,7 @@ stmt_requires_new_enode_p (const gimple *stmt,
"__analyzer_dump_exploded_nodes", so they always appear at the
start of an exploded_node. */
if (const gcall *call = dyn_cast <const gcall *> (stmt))
- if (is_named_call_p (call, "__analyzer_dump_exploded_nodes",
+ if (is_special_named_call_p (call, "__analyzer_dump_exploded_nodes",
1))
return true;
@@ -3065,7 +3074,8 @@ exploded_graph::dump_exploded_nodes () const
if (const gimple *stmt = enode->get_stmt ())
if (const gcall *call = dyn_cast <const gcall *> (stmt))
- if (is_named_call_p (call, "__analyzer_dump_exploded_nodes", 1))
+ if (is_special_named_call_p (call, "__analyzer_dump_exploded_nodes",
+ 1))
{
if (seen.contains (stmt))
continue;
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 11804dc116b4..dccd4eec37c7 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -4125,69 +4125,75 @@ region_model::on_call_pre (const gcall *call,
region_model_context *ctxt)
/* Check for uses of poisoned values.
For now, special-case "free", to avoid warning about "use-after-free"
when "double free" would be more precise. */
- if (!is_named_call_p (call, "free", 1))
+ if (!is_special_named_call_p (call, "free", 1))
for (unsigned i = 0; i < gimple_call_num_args (call); i++)
check_for_poison (gimple_call_arg (call, i), ctxt);
- if (is_named_call_p (call, "malloc", 1))
+ if (tree callee_fndecl = get_fndecl_for_call (call, ctxt))
{
- // TODO: capture size as a svalue?
- region_id new_rid = add_new_malloc_region ();
- if (!lhs_rid.null_p ())
+ if (is_named_call_p (callee_fndecl, "malloc", call, 1))
{
- svalue_id ptr_sid
- = get_or_create_ptr_svalue (lhs_type, new_rid);
- set_value (lhs_rid, ptr_sid, ctxt);
+ // TODO: capture size as a svalue?
+ region_id new_rid = add_new_malloc_region ();
+ if (!lhs_rid.null_p ())
+ {
+ svalue_id ptr_sid
+ = get_or_create_ptr_svalue (lhs_type, new_rid);
+ set_value (lhs_rid, ptr_sid, ctxt);
+ }
+ return;
}
- return;
- }
- else if (is_named_call_p (call, "__builtin_alloca", 1))
- {
- region_id frame_rid = get_current_frame_id ();
- region_id new_rid = add_region (new symbolic_region (frame_rid, false));
- if (!lhs_rid.null_p ())
+ else if (is_named_call_p (callee_fndecl, "__builtin_alloca", call, 1))
{
- svalue_id ptr_sid
- = get_or_create_ptr_svalue (lhs_type, new_rid);
- set_value (lhs_rid, ptr_sid, ctxt);
+ region_id frame_rid = get_current_frame_id ();
+ region_id new_rid
+ = add_region (new symbolic_region (frame_rid, false));
+ if (!lhs_rid.null_p ())
+ {
+ svalue_id ptr_sid
+ = get_or_create_ptr_svalue (lhs_type, new_rid);
+ set_value (lhs_rid, ptr_sid, ctxt);
+ }
+ return;
}
- return;
- }
- else if (is_named_call_p (call, "strlen", 1))
- {
- region_id buf_rid = deref_rvalue (gimple_call_arg (call, 0), ctxt);
- svalue_id buf_sid = get_region (buf_rid)->get_value (*this, true, ctxt);
- if (tree cst_expr = maybe_get_constant (buf_sid))
+ else if (is_named_call_p (callee_fndecl, "strlen", call, 1))
{
- if (TREE_CODE (cst_expr) == STRING_CST
- && !lhs_rid.null_p ())
+ region_id buf_rid = deref_rvalue (gimple_call_arg (call, 0), ctxt);
+ svalue_id buf_sid
+ = get_region (buf_rid)->get_value (*this, true, ctxt);
+ if (tree cst_expr = maybe_get_constant (buf_sid))
{
- /* TREE_STRING_LENGTH is sizeof, not strlen. */
- int sizeof_cst = TREE_STRING_LENGTH (cst_expr);
- int strlen_cst = sizeof_cst - 1;
- tree t_cst = build_int_cst (lhs_type, strlen_cst);
- svalue_id result_sid
- = get_or_create_constant_svalue (t_cst);
- set_value (lhs_rid, result_sid, ctxt);
- return;
+ if (TREE_CODE (cst_expr) == STRING_CST
+ && !lhs_rid.null_p ())
+ {
+ /* TREE_STRING_LENGTH is sizeof, not strlen. */
+ int sizeof_cst = TREE_STRING_LENGTH (cst_expr);
+ int strlen_cst = sizeof_cst - 1;
+ tree t_cst = build_int_cst (lhs_type, strlen_cst);
+ svalue_id result_sid
+ = get_or_create_constant_svalue (t_cst);
+ set_value (lhs_rid, result_sid, ctxt);
+ return;
+ }
}
+ /* Otherwise an unknown value. */
+ }
+ else if (is_named_call_p (callee_fndecl,
+ "__analyzer_dump_num_heap_regions", call, 0))
+ {
+ /* Handle the builtin "__analyzer_dump_num_heap_regions" by emitting
+ a warning (for use in DejaGnu tests). */
+ int num_heap_regions = 0;
+ region_id heap_rid = get_root_region ()->ensure_heap_region (this);
+ unsigned i;
+ region *region;
+ FOR_EACH_VEC_ELT (m_regions, i, region)
+ if (region->get_parent () == heap_rid)
+ num_heap_regions++;
+ /* Use quotes to ensure the output isn't truncated. */
+ warning_at (call->location, 0,
+ "num heap regions: %qi", num_heap_regions);
}
- /* Otherwise an unknown value. */
- }
- else if (is_named_call_p (call, "__analyzer_dump_num_heap_regions", 0))
- {
- /* Handle the builtin "__analyzer_dump_num_heap_regions" by emitting
- a warning (for use in DejaGnu tests). */
- int num_heap_regions = 0;
- region_id heap_rid = get_root_region ()->ensure_heap_region (this);
- unsigned i;
- region *region;
- FOR_EACH_VEC_ELT (m_regions, i, region)
- if (region->get_parent () == heap_rid)
- num_heap_regions++;
- /* Use quotes to ensure the output isn't truncated. */
- warning_at (call->location, 0,
- "num heap regions: %qi", num_heap_regions);
}
/* Unrecognized call. */
@@ -4225,32 +4231,33 @@ region_model::on_call_post (const gcall *call,
region_model_context *ctxt)
(in region_model::eval_condition_without_cm), and thus transition
all pointers to the region to the "freed" state together, regardless
of casts. */
- if (is_named_call_p (call, "free", 1))
- {
- tree ptr = gimple_call_arg (call, 0);
- svalue_id ptr_sid = get_rvalue (ptr, ctxt);
- svalue *ptr_sval = get_svalue (ptr_sid);
- if (region_svalue *ptr_to_region_sval
- = ptr_sval->dyn_cast_region_svalue ())
- {
- /* If the ptr points to an underlying heap region, delete it,
- poisoning pointers. */
- region_id pointee_rid = ptr_to_region_sval->get_pointee ();
- region_id heap_rid = get_root_region ()->ensure_heap_region (this);
- if (!pointee_rid.null_p ()
- && get_region (pointee_rid)->get_parent () == heap_rid)
- {
- purge_stats stats;
- delete_region_and_descendents (pointee_rid,
- POISON_KIND_FREED,
- &stats, ctxt->get_logger ());
- purge_unused_svalues (&stats, ctxt);
- validate ();
- // TODO: do anything with stats?
- }
- }
- return;
- }
+ if (tree callee_fndecl = get_fndecl_for_call (call, ctxt))
+ if (is_named_call_p (callee_fndecl, "free", call, 1))
+ {
+ tree ptr = gimple_call_arg (call, 0);
+ svalue_id ptr_sid = get_rvalue (ptr, ctxt);
+ svalue *ptr_sval = get_svalue (ptr_sid);
+ if (region_svalue *ptr_to_region_sval
+ = ptr_sval->dyn_cast_region_svalue ())
+ {
+ /* If the ptr points to an underlying heap region, delete it,
+ poisoning pointers. */
+ region_id pointee_rid = ptr_to_region_sval->get_pointee ();
+ region_id heap_rid = get_root_region ()->ensure_heap_region (this);
+ if (!pointee_rid.null_p ()
+ && get_region (pointee_rid)->get_parent () == heap_rid)
+ {
+ purge_stats stats;
+ delete_region_and_descendents (pointee_rid,
+ POISON_KIND_FREED,
+ &stats, ctxt->get_logger ());
+ purge_unused_svalues (&stats, ctxt);
+ validate ();
+ // TODO: do anything with stats?
+ }
+ }
+ return;
+ }
}
/* Update this model for the RETURN_STMT, using CTXT to report any
@@ -6356,6 +6363,32 @@ region_model::get_or_create_view (region_id raw_rid,
tree type)
return raw_rid;
}
+/* Attempt to get the fndecl used at CALL, if known, or NULL_TREE
+ otherwise. */
+
+tree
+region_model::get_fndecl_for_call (const gcall *call,
+ region_model_context *ctxt)
+{
+ tree fn_ptr = gimple_call_fn (call);
+ if (fn_ptr == NULL_TREE)
+ return NULL_TREE;
+ svalue_id fn_ptr_sid = get_rvalue (fn_ptr, ctxt);
+ svalue *fn_ptr_sval = get_svalue (fn_ptr_sid);
+ if (region_svalue *fn_ptr_ptr = fn_ptr_sval->dyn_cast_region_svalue ())
+ {
+ region_id fn_rid = fn_ptr_ptr->get_pointee ();
+ code_region *code = get_root_region ()->get_code_region (this);
+ if (code)
+ {
+ tree fn_decl = code->get_tree_for_child_region (fn_rid);
+ return fn_decl;
+ }
+ }
+
+ return NULL_TREE;
+}
+
////////////////////////////////////////////////////////////////////////////
/* struct model_merger. */
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 0b8d366a2e51..5cb3a9a15ab3 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -1749,6 +1749,9 @@ class region_model
region_model_context *ctxt);
region_id get_or_create_view (region_id raw_rid, tree type);
+ tree get_fndecl_for_call (const gcall *call,
+ region_model_context *ctxt);
+
private:
region_id get_lvalue_1 (path_var pv, region_model_context *ctxt);
svalue_id get_rvalue_1 (path_var pv, region_model_context *ctxt);
diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc
index f2e8030a49b1..a96971464af7 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -217,44 +217,46 @@ fileptr_state_machine::on_stmt (sm_context *sm_ctxt,
const gimple *stmt) const
{
if (const gcall *call = dyn_cast <const gcall *> (stmt))
- {
- if (is_named_call_p (call, "fopen", 2))
- {
- tree lhs = gimple_call_lhs (call);
- if (lhs)
- {
- lhs = sm_ctxt->get_readable_tree (lhs);
- sm_ctxt->on_transition (node, stmt, lhs, m_start, m_unchecked);
- }
- else
- {
- /* TODO: report leak. */
- }
- return true;
- }
-
- if (is_named_call_p (call, "fclose", 1))
- {
- tree arg = gimple_call_arg (call, 0);
- arg = sm_ctxt->get_readable_tree (arg);
-
- sm_ctxt->on_transition (node, stmt, arg, m_start, m_closed);
-
- // TODO: is it safe to call fclose (NULL) ?
- sm_ctxt->on_transition (node, stmt, arg, m_unchecked, m_closed);
- sm_ctxt->on_transition (node, stmt, arg, m_null, m_closed);
-
- sm_ctxt->on_transition (node, stmt , arg, m_nonnull, m_closed);
-
- sm_ctxt->warn_for_state (node, stmt, arg, m_closed,
- new double_fclose (*this, arg));
- sm_ctxt->on_transition (node, stmt, arg, m_closed, m_stop);
- return true;
- }
-
- // TODO: operations on closed file
- // etc
- }
+ if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call))
+ {
+ if (is_named_call_p (callee_fndecl, "fopen", call, 2))
+ {
+ tree lhs = gimple_call_lhs (call);
+ if (lhs)
+ {
+ lhs = sm_ctxt->get_readable_tree (lhs);
+ sm_ctxt->on_transition (node, stmt, lhs, m_start, m_unchecked);
+ }
+ else
+ {
+ /* TODO: report leak. */
+ }
+ return true;
+ }
+
+ if (is_named_call_p (callee_fndecl, "fclose", call, 1))
+ {
+ tree arg = gimple_call_arg (call, 0);
+ arg = sm_ctxt->get_readable_tree (arg);
+
+ sm_ctxt->on_transition (node, stmt, arg, m_start, m_closed);
+
+ // TODO: is it safe to call fclose (NULL) ?
+ sm_ctxt->on_transition (node, stmt, arg, m_unchecked, m_closed);
+ sm_ctxt->on_transition (node, stmt, arg, m_null, m_closed);
+
+ sm_ctxt->on_transition (node, stmt , arg, m_nonnull, m_closed);
+
+ sm_ctxt->warn_for_state (node, stmt, arg, m_closed,
+ new double_fclose (*this, arg));
+ sm_ctxt->on_transition (node, stmt, arg, m_closed, m_stop);
+ return true;
+ }
+
+ // TODO: operations on closed file
+ // etc
+ }
+
return false;
}
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index d3d91474276c..4253d2b42535 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -532,8 +532,8 @@ public:
gimple *def_stmt = SSA_NAME_DEF_STMT (change.m_expr);
if (gcall *call = dyn_cast <gcall *> (def_stmt))
{
- if (is_named_call_p (call, "alloca", 1)
- || is_named_call_p (call, "__builtin_alloca", 1))
+ if (is_special_named_call_p (call, "alloca", 1)
+ || is_special_named_call_p (call, "__builtin_alloca", 1))
{
m_kind = KIND_ALLOCA;
return label_text::borrow
@@ -582,63 +582,63 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
const gimple *stmt) const
{
if (const gcall *call = dyn_cast <const gcall *> (stmt))
- {
- if (is_named_call_p (call, "malloc", 1)
- || is_named_call_p (call, "calloc", 2))
- {
- tree lhs = gimple_call_lhs (call);
- if (lhs)
- {
- lhs = sm_ctxt->get_readable_tree (lhs);
- sm_ctxt->on_transition (node, stmt, lhs, m_start, m_unchecked);
- }
- else
- {
- /* TODO: report leak. */
- }
- return true;
- }
+ if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call))
+ {
+ if (is_named_call_p (callee_fndecl, "malloc", call, 1)
+ || is_named_call_p (callee_fndecl, "calloc", call, 2))
+ {
+ tree lhs = gimple_call_lhs (call);
+ if (lhs)
+ {
+ lhs = sm_ctxt->get_readable_tree (lhs);
+ sm_ctxt->on_transition (node, stmt, lhs, m_start, m_unchecked);
+ }
+ else
+ {
+ /* TODO: report leak. */
+ }
+ return true;
+ }
- if (is_named_call_p (call, "alloca", 1)
- || is_named_call_p (call, "__builtin_alloca", 1))
- {
- tree lhs = gimple_call_lhs (call);
- if (lhs)
- {
- lhs = sm_ctxt->get_readable_tree (lhs);
- sm_ctxt->on_transition (node, stmt, lhs, m_start, m_non_heap);
- }
- return true;
- }
+ if (is_named_call_p (callee_fndecl, "alloca", call, 1)
+ || is_named_call_p (callee_fndecl, "__builtin_alloca", call, 1))
+ {
+ tree lhs = gimple_call_lhs (call);
+ if (lhs)
+ {
+ lhs = sm_ctxt->get_readable_tree (lhs);
+ sm_ctxt->on_transition (node, stmt, lhs, m_start, m_non_heap);
+ }
+ return true;
+ }
- if (is_named_call_p (call, "free", 1))
- {
- tree arg = gimple_call_arg (call, 0);
+ if (is_named_call_p (callee_fndecl, "free", call, 1))
+ {
+ tree arg = gimple_call_arg (call, 0);
- arg = sm_ctxt->get_readable_tree (arg);
+ arg = sm_ctxt->get_readable_tree (arg);
- /* start/unchecked/nonnull -> freed. */
- sm_ctxt->on_transition (node, stmt, arg, m_start, m_freed);
- sm_ctxt->on_transition (node, stmt, arg, m_unchecked, m_freed);
- sm_ctxt->on_transition (node, stmt, arg, m_nonnull, m_freed);
+ /* start/unchecked/nonnull -> freed. */
+ sm_ctxt->on_transition (node, stmt, arg, m_start, m_freed);
+ sm_ctxt->on_transition (node, stmt, arg, m_unchecked, m_freed);
+ sm_ctxt->on_transition (node, stmt, arg, m_nonnull, m_freed);
- /* Keep state "null" as-is, rather than transitioning to "free";
- we don't want want to complain about double-free of NULL. */
+ /* Keep state "null" as-is, rather than transitioning to "free";
+ we don't want want to complain about double-free of NULL. */
- /* freed -> stop, with warning. */
- sm_ctxt->warn_for_state (node, stmt, arg, m_freed,
- new double_free (*this, arg));
- sm_ctxt->on_transition (node, stmt, arg, m_freed, m_stop);
+ /* freed -> stop, with warning. */
+ sm_ctxt->warn_for_state (node, stmt, arg, m_freed,
+ new double_free (*this, arg));
+ sm_ctxt->on_transition (node, stmt, arg, m_freed, m_stop);
- /* non-heap -> stop, with warning. */
- sm_ctxt->warn_for_state (node, stmt, arg, m_non_heap,
- new free_of_non_heap (*this, arg));
- sm_ctxt->on_transition (node, stmt, arg, m_non_heap, m_stop);
- return true;
- }
+ /* non-heap -> stop, with warning. */
+ sm_ctxt->warn_for_state (node, stmt, arg, m_non_heap,
+ new free_of_non_heap (*this, arg));
+ sm_ctxt->on_transition (node, stmt, arg, m_non_heap, m_stop);
+ return true;
+ }
- /* Handle "__attribute__((nonnull))". */
- if (tree callee_fndecl = gimple_call_fndecl (stmt))
+ /* Handle "__attribute__((nonnull))". */
{
tree fntype = TREE_TYPE (callee_fndecl);
bitmap nonnull_args = get_nonnull_args (fntype);
@@ -669,7 +669,7 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
BITMAP_FREE (nonnull_args);
}
}
- }
+ }
if (tree lhs = is_zero_assignment (stmt))
{
diff --git a/gcc/analyzer/sm-sensitive.cc b/gcc/analyzer/sm-sensitive.cc
index 414dd56948d5..08b1b32b7933 100644
--- a/gcc/analyzer/sm-sensitive.cc
+++ b/gcc/analyzer/sm-sensitive.cc
@@ -135,33 +135,34 @@ sensitive_state_machine::on_stmt (sm_context *sm_ctxt,
const gimple *stmt) const
{
if (const gcall *call = dyn_cast <const gcall *> (stmt))
- {
- if (is_named_call_p (call, "getpass", 1))
- {
- tree lhs = gimple_call_lhs (call);
- if (lhs)
- sm_ctxt->on_transition (node, stmt, lhs, m_start, m_sensitive);
- return true;
- }
- else if (is_named_call_p (call, "fprintf")
- || is_named_call_p (call, "printf"))
- {
- /* Handle a match at any position in varargs. */
- for (unsigned idx = 1; idx < gimple_call_num_args (call); idx++)
- {
- tree arg = gimple_call_arg (call, idx);
- warn_for_any_exposure (sm_ctxt, node, stmt, arg);
- }
- return true;
- }
- else if (is_named_call_p (call, "fwrite", 4))
- {
- tree arg = gimple_call_arg (call, 0);
- warn_for_any_exposure (sm_ctxt, node, stmt, arg);
- return true;
- }
- // TODO: ...etc. This is just a proof-of-concept at this point.
- }
+ if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call))
+ {
+ if (is_named_call_p (callee_fndecl, "getpass", call, 1))
+ {
+ tree lhs = gimple_call_lhs (call);
+ if (lhs)
+ sm_ctxt->on_transition (node, stmt, lhs, m_start, m_sensitive);
+ return true;
+ }
+ else if (is_named_call_p (callee_fndecl, "fprintf")
+ || is_named_call_p (callee_fndecl, "printf"))
+ {
+ /* Handle a match at any position in varargs. */
+ for (unsigned idx = 1; idx < gimple_call_num_args (call); idx++)
+ {
+ tree arg = gimple_call_arg (call, idx);
+ warn_for_any_exposure (sm_ctxt, node, stmt, arg);
+ }
+ return true;
+ }
+ else if (is_named_call_p (callee_fndecl, "fwrite", call, 4))
+ {
+ tree arg = gimple_call_arg (call, 0);
+ warn_for_any_exposure (sm_ctxt, node, stmt, arg);
+ return true;
+ }
+ // TODO: ...etc. This is just a proof-of-concept at this point.
+ }
return false;
}
diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc
index 59b9171a58e1..1c110119b2d6 100644
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -198,22 +198,23 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt,
const gimple *stmt) const
{
if (const gcall *call = dyn_cast <const gcall *> (stmt))
- {
- if (is_named_call_p (call, "fread", 4))
- {
- tree arg = gimple_call_arg (call, 0);
- arg = sm_ctxt->get_readable_tree (arg);
-
- sm_ctxt->on_transition (node, stmt, arg, m_start, m_tainted);
-
- /* Dereference an ADDR_EXPR. */
- // TODO: should the engine do this?
- if (TREE_CODE (arg) == ADDR_EXPR)
- sm_ctxt->on_transition (node, stmt, TREE_OPERAND (arg, 0),
- m_start, m_tainted);
- return true;
- }
- }
+ if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call))
+ {
+ if (is_named_call_p (callee_fndecl, "fread", call, 4))
+ {
+ tree arg = gimple_call_arg (call, 0);
+ arg = sm_ctxt->get_readable_tree (arg);
+
+ sm_ctxt->on_transition (node, stmt, arg, m_start, m_tainted);
+
+ /* Dereference an ADDR_EXPR. */
+ // TODO: should the engine do this?
+ if (TREE_CODE (arg) == ADDR_EXPR)
+ sm_ctxt->on_transition (node, stmt, TREE_OPERAND (arg, 0),
+ m_start, m_tainted);
+ return true;
+ }
+ }
// TODO: ...etc; many other sources of untrusted data
if (const gassign *assign = dyn_cast <const gassign *> (stmt))
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index b35d8c535c2a..fe67c6b4b979 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -112,6 +112,12 @@ class sm_context
public:
virtual ~sm_context () {}
+ /* Get the fndecl used at call, or NULL_TREE.
+ Use in preference to gimple_call_fndecl (and gimple_call_addr_fndecl),
+ since it can look through function pointer assignments and
+ other callback handling. */
+ virtual tree get_fndecl_for_call (const gcall *call) = 0;
+
/* Called by state_machine in response to pattern matches:
if VAR is in state FROM, transition it to state TO, potentially
recording the "origin" of the state as ORIGIN.
diff --git a/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c
b/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c
index b3673fb7e756..8c27b3ae7a45 100644
--- a/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c
+++ b/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c
@@ -55,3 +55,27 @@ void test_4 (void *q, void *r)
free(p);
}
+
+/* Verify that we detect passing NULL to a __attribute__((nonnull)) function
+ when it's called via a function pointer. */
+
+typedef void (*bar_t)(void *ptrA, void *ptrB, void *ptrC);
+
+static bar_t __attribute__((noinline))
+get_bar (void)
+{
+ return bar;
+}
+
+void test_5 (void *q, void *r)
+{
+ void *p = malloc(1024); /* { dg-message "\\(1\\) this call could return
NULL" } */
+ bar_t cb = get_bar ();
+ cb(p, q, r); /* { dg-warning "use of possibly-NULL 'p' where non-null
expected" } */
+ /* { dg-message "argument 1 \\('p'\\) from \\(1\\) could be NULL where
non-null expected" "" { target *-*-* } .-1 } */
+ /* TODO: do we want an event showing where cb is assigned "bar"? */
+
+ cb(p, q, r);
+
+ free(p);
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-callbacks.c
b/gcc/testsuite/gcc.dg/analyzer/malloc-callbacks.c
new file mode 100644
index 000000000000..eb5545e5da00
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-callbacks.c
@@ -0,0 +1,84 @@
+#include <stdlib.h>
+
+typedef void *(*allocator_t) (size_t);
+typedef void (*deallocator_t) (void *);
+
+static allocator_t __attribute__((noinline))
+get_malloc (void)
+{
+ return malloc;
+}
+
+static allocator_t __attribute__((noinline))
+get_alloca (void)
+{
+ return alloca;
+}
+
+static deallocator_t __attribute__((noinline))
+get_free (void)
+{
+ return free;
+}
+
+void test_1 (void *ptr)
+{
+ deallocator_t dealloc_fn = free;
+ dealloc_fn (ptr); /* { dg-message "first 'free' here" } */
+ dealloc_fn (ptr); /* { dg-warning "double-'free'" } */
+}
+
+void test_2 (void *ptr)
+{
+ deallocator_t dealloc_fn = get_free ();
+ dealloc_fn (ptr); /* { dg-message "first 'free' here" } */
+ dealloc_fn (ptr); /* { dg-warning "double-'free'" } */
+}
+
+static void __attribute__((noinline))
+called_by_test_3 (void *ptr, deallocator_t dealloc_fn)
+{
+ dealloc_fn (ptr); /* { dg-warning "double-'free'" } */
+}
+
+void test_3 (void *ptr)
+{
+ called_by_test_3 (ptr, free);
+ called_by_test_3 (ptr, free);
+}
+
+int *test_4 (void)
+{
+ allocator_t alloc_fn = get_malloc ();
+ int *ptr = alloc_fn (sizeof (int)); /* { dg-message "this call could return
NULL" } */
+ *ptr = 42; /* { dg-warning "dereference of possibly-NULL 'ptr'" } */
+ return ptr;
+}
+
+int *test_5 (void)
+{
+ allocator_t alloc_fn = get_alloca ();
+ deallocator_t dealloc_fn = get_free ();
+ int *ptr = alloc_fn (sizeof (int)); /* { dg-message "pointer is from here" }
*/
+ /* TODO: message should read "memory is allocated on the stack here". */
+ dealloc_fn (ptr); /* { dg-warning "'free' of 'ptr' which points to memory
not on the heap" } */
+}
+
+static void __attribute__((noinline))
+called_by_test_6a (void *ptr)
+{
+ free (ptr); /* { dg-warning "double-'free'" "" { xfail *-*-* } } */
+}
+
+static deallocator_t __attribute__((noinline))
+called_by_test_6b (void)
+{
+ return called_by_test_6a;
+}
+
+void test_6 (void *ptr)
+{
+ deallocator_t dealloc_fn = called_by_test_6b ();
+ dealloc_fn (ptr);
+ dealloc_fn (ptr);
+}
--
2.21.0
