[analyzer] Function pointer support

Message ID 20191203223144.17535-1-dmalcolm@redhat.com
State New
Headers show
Series
  • [analyzer] Function pointer support
Related show

Commit Message

David Malcolm Dec. 3, 2019, 10:31 p.m.
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

-- 
2.21.0

Patch

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