diff mbox series

[committed] analyzer: use known_function to simplify region_model::on_call_{pre, post}

Message ID 20221116132942.2958189-1-dmalcolm@redhat.com
State New
Headers show
Series [committed] analyzer: use known_function to simplify region_model::on_call_{pre, post} | expand

Commit Message

David Malcolm Nov. 16, 2022, 1:29 p.m. UTC 
Replace lots of repeated checks against strings with a hash_map lookup.
Add some missing type-checking for handling known functions (e.g. checks
for pointer types).

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r13-4088-g21501ec751c102.

gcc/analyzer/ChangeLog:
	* analyzer.h (known_function::matches_call_types_p): New vfunc.
	(known_function::impl_call_pre): Provide base implementation.
	(known_function::impl_call_post): New vfunc.
	(register_known_functions): New.
	* engine.cc (impl_run_checkers): Call register_known_functions.
	* region-model-impl-calls.cc (region_model::impl_call_accept):
	Convert to...
	(class known_function_accept): ...this.
	(region_model::impl_call_bind): Convert to...
	(class known_function_bind): ...this.
	(region_model::impl_call_connect): Convert to...
	(class known_function_connect): ...this.
	(region_model::impl_call_listen): Convert to...
	(class known_function_listen): ...this.
	(region_model::impl_call_socket): Convert to...
	(class known_function_socket): ...this.
	(register_known_functions): New.
	* region-model.cc (region_model::on_call_pre): Remove special
	case for "bind" in favor of the known_function-handling dispatch.
	Add call to known_function::matches_call_types_p to latter.
	(region_model::on_call_post): Remove special cases for "accept",
	"bind", "connect", "listen", and "socket" in favor of dispatch
	to known_function::impl_call_post.
	* region-model.h (region_model::impl_call_accept): Delete decl.
	(region_model::impl_call_bind): Delete decl.
	(region_model::impl_call_connect): Delete decl.
	(region_model::impl_call_listen): Delete decl.
	(region_model::impl_call_socket): Delete decl.
	* sm-fd.cc: Update comments.

gcc/testsuite/ChangeLog:
	* gcc.dg/plugin/analyzer_kernel_plugin.c
	(copy_across_boundary_fn::matches_call_types_p): New.
	* gcc.dg/plugin/analyzer_known_fns_plugin.c
	(known_function_returns_42::matches_call_types_p): New.
	(known_function_attempt_to_copy::matches_call_types_p): New.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/analyzer/analyzer.h                       |  14 +-
 gcc/analyzer/engine.cc                        |   2 +
 gcc/analyzer/region-model-impl-calls.cc       | 162 ++++++++++++------
 gcc/analyzer/region-model.cc                  |  45 ++---
 gcc/analyzer/region-model.h                   |   5 -
 gcc/analyzer/sm-fd.cc                         |  10 +-
 .../gcc.dg/plugin/analyzer_kernel_plugin.c    |   5 +
 .../gcc.dg/plugin/analyzer_known_fns_plugin.c |  10 ++
 8 files changed, 154 insertions(+), 99 deletions(-)
diff mbox series

Patch

diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h
index 9cf8d98fabe..99a1d0690d5 100644
--- a/gcc/analyzer/analyzer.h
+++ b/gcc/analyzer/analyzer.h
@@ -228,15 +228,25 @@  extern location_t get_stmt_location (const gimple *stmt, function *fun);
 extern bool compat_types_p (tree src_type, tree dst_type);
 
 /* Abstract base class for simulating the behavior of known functions,
-   supplied by plugins.  */
+   supplied by the core of the analyzer, or by plugins.  */
 
 class known_function
 {
 public:
   virtual ~known_function () {}
-  virtual void impl_call_pre (const call_details &cd) const = 0;
+  virtual bool matches_call_types_p (const call_details &cd) const = 0;
+  virtual void impl_call_pre (const call_details &) const
+  {
+    return;
+  }
+  virtual void impl_call_post (const call_details &) const
+  {
+    return;
+  }
 };
 
+extern void register_known_functions (known_function_manager &mgr);
+
 /* Passed by pointer to PLUGIN_ANALYZER_INIT callbacks.  */
 
 class plugin_analyzer_init_iface
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 3ef411cae93..fe2b9c69221 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -6073,6 +6073,8 @@  impl_run_checkers (logger *logger)
   auto_delete_vec <state_machine> checkers;
   make_checkers (checkers, logger);
 
+  register_known_functions (*eng.get_known_function_manager ());
+
   plugin_analyzer_init_impl data (&checkers,
 				  eng.get_known_function_manager (),
 				  logger);
diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc
index 99597e0667a..7a039c75c03 100644
--- a/gcc/analyzer/region-model-impl-calls.cc
+++ b/gcc/analyzer/region-model-impl-calls.cc
@@ -407,10 +407,10 @@  region_model::impl_call_analyzer_get_unknown_ptr (const call_details &cd)
   cd.maybe_set_lhs (ptr_sval);
 }
 
-/* Handle the on_call_post part of "accept".  */
+/* Handle calls to "accept".
+   See e.g. https://man7.org/linux/man-pages/man3/accept.3p.html  */
 
-void
-region_model::impl_call_accept (const call_details &cd)
+class known_function_accept : public known_function
 {
   class outcome_of_accept : public succeed_or_fail_call_info
   {
@@ -428,20 +428,28 @@  region_model::impl_call_accept (const call_details &cd)
     }
   };
 
-  /* Body of region_model::impl_call_accept.  */
-  if (cd.get_ctxt ())
-    {
-      cd.get_ctxt ()->bifurcate (make_unique<outcome_of_accept> (cd, false));
-      cd.get_ctxt ()->bifurcate (make_unique<outcome_of_accept> (cd, true));
-      cd.get_ctxt ()->terminate_path ();
-    }
-}
+  bool matches_call_types_p (const call_details &cd) const final override
+  {
+    return cd.num_args () == 3;
+  }
+
+  void impl_call_post (const call_details &cd) const final override
+  {
+    if (cd.get_ctxt ())
+      {
+	cd.get_ctxt ()->bifurcate (make_unique<outcome_of_accept> (cd, false));
+	cd.get_ctxt ()->bifurcate (make_unique<outcome_of_accept> (cd, true));
+	cd.get_ctxt ()->terminate_path ();
+      }
+  }
+};
 
-/* Handle the on_call_post part of "bind".  */
+/* Handle calls to "bind".
+   See e.g. https://man7.org/linux/man-pages/man3/bind.3p.html  */
 
-void
-region_model::impl_call_bind (const call_details &cd)
+class known_function_bind : public known_function
 {
+public:
   class outcome_of_bind : public succeed_or_fail_call_info
   {
   public:
@@ -458,14 +466,21 @@  region_model::impl_call_bind (const call_details &cd)
     }
   };
 
-  /* Body of region_model::impl_call_bind.  */
-  if (cd.get_ctxt ())
-    {
-      cd.get_ctxt ()->bifurcate (make_unique<outcome_of_bind> (cd, false));
-      cd.get_ctxt ()->bifurcate (make_unique<outcome_of_bind> (cd, true));
-      cd.get_ctxt ()->terminate_path ();
-    }
-}
+  bool matches_call_types_p (const call_details &cd) const final override
+  {
+    return cd.num_args () == 3;
+  }
+
+  void impl_call_post (const call_details &cd) const final override
+  {
+    if (cd.get_ctxt ())
+      {
+	cd.get_ctxt ()->bifurcate (make_unique<outcome_of_bind> (cd, false));
+	cd.get_ctxt ()->bifurcate (make_unique<outcome_of_bind> (cd, true));
+	cd.get_ctxt ()->terminate_path ();
+      }
+  }
+};
 
 /* Handle the on_call_pre part of "__builtin_expect" etc.  */
 
@@ -501,11 +516,12 @@  region_model::impl_call_calloc (const call_details &cd)
     }
 }
 
-/* Handle the on_call_post part of "connect".  */
+/* Handle calls to "connect".
+   See e.g. https://man7.org/linux/man-pages/man3/connect.3p.html  */
 
-void
-region_model::impl_call_connect (const call_details &cd)
+class known_function_connect : public known_function
 {
+public:
   class outcome_of_connect : public succeed_or_fail_call_info
   {
   public:
@@ -522,14 +538,22 @@  region_model::impl_call_connect (const call_details &cd)
     }
   };
 
-  /* Body of region_model::impl_call_connect.  */
-  if (cd.get_ctxt ())
-    {
-      cd.get_ctxt ()->bifurcate (make_unique<outcome_of_connect> (cd, false));
-      cd.get_ctxt ()->bifurcate (make_unique<outcome_of_connect> (cd, true));
-      cd.get_ctxt ()->terminate_path ();
-    }
-}
+  bool matches_call_types_p (const call_details &cd) const final override
+  {
+    return (cd.num_args () == 3
+	    && POINTER_TYPE_P (cd.get_arg_type (1)));
+  }
+
+  void impl_call_post (const call_details &cd) const final override
+  {
+    if (cd.get_ctxt ())
+      {
+	cd.get_ctxt ()->bifurcate (make_unique<outcome_of_connect> (cd, false));
+	cd.get_ctxt ()->bifurcate (make_unique<outcome_of_connect> (cd, true));
+	cd.get_ctxt ()->terminate_path ();
+      }
+  }
+};
 
 /* Handle the on_call_pre part of "__errno_location".  */
 
@@ -633,10 +657,10 @@  region_model::impl_call_free (const call_details &cd)
     }
 }
 
-/* Handle the on_call_post part of "listen".  */
+/* Handle calls to "listen".
+   See e.g. https://man7.org/linux/man-pages/man3/listen.3p.html  */
 
-void
-region_model::impl_call_listen (const call_details &cd)
+class known_function_listen : public known_function
 {
   class outcome_of_listen : public succeed_or_fail_call_info
   {
@@ -654,14 +678,21 @@  region_model::impl_call_listen (const call_details &cd)
     }
   };
 
-  /* Body of region_model::impl_call_listen.  */
-  if (cd.get_ctxt ())
-    {
-      cd.get_ctxt ()->bifurcate (make_unique<outcome_of_listen> (cd, false));
-      cd.get_ctxt ()->bifurcate (make_unique<outcome_of_listen> (cd, true));
-      cd.get_ctxt ()->terminate_path ();
-    }
-}
+  bool matches_call_types_p (const call_details &cd) const final override
+  {
+    return cd.num_args () == 2;
+  }
+
+  void impl_call_post (const call_details &cd) const final override
+  {
+    if (cd.get_ctxt ())
+      {
+	cd.get_ctxt ()->bifurcate (make_unique<outcome_of_listen> (cd, false));
+	cd.get_ctxt ()->bifurcate (make_unique<outcome_of_listen> (cd, true));
+	cd.get_ctxt ()->terminate_path ();
+      }
+  }
+};
 
 /* Handle the on_call_pre part of "malloc".  */
 
@@ -1175,11 +1206,12 @@  region_model::impl_call_realloc (const call_details &cd)
     }
 }
 
-/* Handle the on_call_post part of "socket".  */
+/* Handle calls to "socket".
+   See e.g. https://man7.org/linux/man-pages/man3/socket.3p.html  */
 
-void
-region_model::impl_call_socket (const call_details &cd)
+class known_function_socket : public known_function
 {
+public:
   class outcome_of_socket : public succeed_or_fail_call_info
   {
   public:
@@ -1196,14 +1228,21 @@  region_model::impl_call_socket (const call_details &cd)
     }
   };
 
-  /* Body of region_model::impl_call_socket.  */
-  if (cd.get_ctxt ())
-    {
-      cd.get_ctxt ()->bifurcate (make_unique<outcome_of_socket> (cd, false));
-      cd.get_ctxt ()->bifurcate (make_unique<outcome_of_socket> (cd, true));
-      cd.get_ctxt ()->terminate_path ();
-    }
-}
+  bool matches_call_types_p (const call_details &cd) const final override
+  {
+    return cd.num_args () == 3;
+  }
+
+  void impl_call_post (const call_details &cd) const final override
+  {
+    if (cd.get_ctxt ())
+      {
+	cd.get_ctxt ()->bifurcate (make_unique<outcome_of_socket> (cd, false));
+	cd.get_ctxt ()->bifurcate (make_unique<outcome_of_socket> (cd, true));
+	cd.get_ctxt ()->terminate_path ();
+      }
+  }
+};
 
 /* Handle the on_call_post part of "strchr" and "__builtin_strchr".  */
 
@@ -1339,6 +1378,19 @@  region_model::impl_deallocation_call (const call_details &cd)
   impl_call_free (cd);
 }
 
+/* Add instances to MGR of known functions supported by the core of the
+   analyzer (as opposed to plugins).  */
+
+void
+register_known_functions (known_function_manager &mgr)
+{
+  mgr.add ("accept", make_unique<known_function_accept> ());
+  mgr.add ("bind", make_unique<known_function_bind> ());
+  mgr.add ("connect", make_unique<known_function_connect> ());
+  mgr.add ("listen", make_unique<known_function_listen> ());
+  mgr.add ("socket", make_unique<known_function_socket> ());
+}
+
 } // namespace ana
 
 #endif /* #if ENABLE_ANALYZER */
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 5f1dd0112d1..e16f66bbbc3 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -2293,11 +2293,6 @@  region_model::on_call_pre (const gcall *call, region_model_context *ctxt,
 	  impl_call_realloc (cd);
 	  return false;
 	}
-      else if (is_named_call_p (callee_fndecl, "bind", call, 3))
-	{
-	  /* Handle in "on_call_post".  */
-	  return false;
-	}
       else if (is_named_call_p (callee_fndecl, "__errno_location", call, 0))
 	{
 	  impl_call_errno_location (cd);
@@ -2383,8 +2378,11 @@  region_model::on_call_pre (const gcall *call, region_model_context *ctxt,
 	}
       else if (const known_function *kf = get_known_function (callee_fndecl))
 	{
-	  kf->impl_call_pre (cd);
-	  return false;
+	  if (kf->matches_call_types_p (cd))
+	    {
+	      kf->impl_call_pre (cd);
+	      return false;
+	    }
 	}
       else if (!fndecl_has_gimple_body_p (callee_fndecl)
 	       && (!(callee_fndecl_flags & (ECF_CONST | ECF_PURE)))
@@ -2427,43 +2425,26 @@  region_model::on_call_post (const gcall *call,
 	  impl_call_operator_delete (cd);
 	  return;
 	}
-      else if (is_named_call_p (callee_fndecl, "accept", call, 3))
-	{
-	  impl_call_accept (cd);
-	  return;
-	}
-      else if (is_named_call_p (callee_fndecl, "bind", call, 3))
-	{
-	  impl_call_bind (cd);
-	  return;
-	}
-      else if (is_named_call_p (callee_fndecl, "connect", call, 3))
-	{
-	  impl_call_connect (cd);
-	  return;
-	}
-      else if (is_named_call_p (callee_fndecl, "listen", call, 2))
-	{
-	  impl_call_listen (cd);
-	  return;
-	}
       else if (is_pipe_call_p (callee_fndecl, "pipe", call, 1)
 	       || is_pipe_call_p (callee_fndecl, "pipe2", call, 2))
 	{
 	  impl_call_pipe (cd);
 	  return;
 	}
-      else if (is_named_call_p (callee_fndecl, "socket", call, 3))
-	{
-	  impl_call_socket (cd);
-	  return;
-	}
       else if (is_named_call_p (callee_fndecl, "strchr", call, 2)
 	       && POINTER_TYPE_P (cd.get_arg_type (0)))
 	{
 	  impl_call_strchr (cd);
 	  return;
 	}
+      else if (const known_function *kf = get_known_function (callee_fndecl))
+	{
+	  if (kf->matches_call_types_p (cd))
+	    {
+	      kf->impl_call_post (cd);
+	      return;
+	    }
+	}
       /* Was this fndecl referenced by
 	 __attribute__((malloc(FOO)))?  */
       if (lookup_attribute ("*dealloc", DECL_ATTRIBUTES (callee_fndecl)))
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 1e72c551dfa..bf06271c626 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -338,7 +338,6 @@  class region_model
   void purge_state_involving (const svalue *sval, region_model_context *ctxt);
 
   /* Specific handling for on_call_pre.  */
-  void impl_call_accept (const call_details &cd);
   void impl_call_alloca (const call_details &cd);
   void impl_call_analyzer_describe (const gcall *call,
 				    region_model_context *ctxt);
@@ -350,24 +349,20 @@  class region_model
   void impl_call_analyzer_eval (const gcall *call,
 				region_model_context *ctxt);
   void impl_call_analyzer_get_unknown_ptr (const call_details &cd);
-  void impl_call_bind (const call_details &cd);
   void impl_call_builtin_expect (const call_details &cd);
   void impl_call_calloc (const call_details &cd);
-  void impl_call_connect (const call_details &cd);
   void impl_call_errno_location (const call_details &cd);
   bool impl_call_error (const call_details &cd, unsigned min_args,
 			bool *out_terminate_path);
   void impl_call_fgets (const call_details &cd);
   void impl_call_fread (const call_details &cd);
   void impl_call_free (const call_details &cd);
-  void impl_call_listen (const call_details &cd);
   void impl_call_malloc (const call_details &cd);
   void impl_call_memcpy (const call_details &cd);
   void impl_call_memset (const call_details &cd);
   void impl_call_pipe (const call_details &cd);
   void impl_call_putenv (const call_details &cd);
   void impl_call_realloc (const call_details &cd);
-  void impl_call_socket (const call_details &cd);
   void impl_call_strchr (const call_details &cd);
   void impl_call_strcpy (const call_details &cd);
   void impl_call_strlen (const call_details &cd);
diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
index d0b587143d0..1f479b6b38c 100644
--- a/gcc/analyzer/sm-fd.cc
+++ b/gcc/analyzer/sm-fd.cc
@@ -2249,7 +2249,7 @@  region_model::mark_as_valid_fd (const svalue *sval, region_model_context *ctxt)
 }
 
 /* Specialcase hook for handling "socket", for use by
-   region_model::impl_call_socket::outcome_of_socket::update_model.  */
+   known_function_socket::outcome_of_socket::update_model.  */
 
 bool
 region_model::on_socket (const call_details &cd, bool successful)
@@ -2267,7 +2267,7 @@  region_model::on_socket (const call_details &cd, bool successful)
 }
 
 /* Specialcase hook for handling "bind", for use by
-   region_model::impl_call_bind::outcome_of_bind::update_model.  */
+   known_function_bind::outcome_of_bind::update_model.  */
 
 bool
 region_model::on_bind (const call_details &cd, bool successful)
@@ -2285,7 +2285,7 @@  region_model::on_bind (const call_details &cd, bool successful)
 }
 
 /* Specialcase hook for handling "listen", for use by
-   region_model::impl_call_listen::outcome_of_listen::update_model.  */
+   known_function_listen::outcome_of_listen::update_model.  */
 
 bool
 region_model::on_listen (const call_details &cd, bool successful)
@@ -2303,7 +2303,7 @@  region_model::on_listen (const call_details &cd, bool successful)
 }
 
 /* Specialcase hook for handling "accept", for use by
-   region_model::impl_call_accept::outcome_of_accept::update_model.  */
+   known_function_accept::outcome_of_accept::update_model.  */
 
 bool
 region_model::on_accept (const call_details &cd, bool successful)
@@ -2321,7 +2321,7 @@  region_model::on_accept (const call_details &cd, bool successful)
 }
 
 /* Specialcase hook for handling "connect", for use by
-   region_model::impl_call_connect::outcome_of_connect::update_model.  */
+   known_function_connect::outcome_of_connect::update_model.  */
 
 bool
 region_model::on_connect (const call_details &cd, bool successful)
diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_kernel_plugin.c b/gcc/testsuite/gcc.dg/plugin/analyzer_kernel_plugin.c
index 92b4dfbd4d0..b424337aad1 100644
--- a/gcc/testsuite/gcc.dg/plugin/analyzer_kernel_plugin.c
+++ b/gcc/testsuite/gcc.dg/plugin/analyzer_kernel_plugin.c
@@ -58,6 +58,11 @@  class copy_across_boundary_fn : public known_function
   virtual bool untrusted_source_p () const = 0;
   virtual bool untrusted_destination_p () const = 0;
 
+  bool matches_call_types_p (const call_details &cd) const final override
+  {
+    return cd.num_args () == 3;
+  }
+
   void impl_call_pre (const call_details &cd) const final override
   {
     region_model_manager *mgr = cd.get_manager ();
diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_known_fns_plugin.c b/gcc/testsuite/gcc.dg/plugin/analyzer_known_fns_plugin.c
index e9f607f58fe..1435b383674 100644
--- a/gcc/testsuite/gcc.dg/plugin/analyzer_known_fns_plugin.c
+++ b/gcc/testsuite/gcc.dg/plugin/analyzer_known_fns_plugin.c
@@ -55,6 +55,11 @@  namespace ana {
 class known_function_returns_42 : public known_function
 {
 public:
+  bool matches_call_types_p (const call_details &) const final override
+  {
+    return true;
+  }
+
   void impl_call_pre (const call_details &cd) const final override
   {
     if (cd.get_lhs_type ())
@@ -115,6 +120,11 @@  public:
     }
   };
 
+  bool matches_call_types_p (const call_details &cd) const
+  {
+    return cd.num_args () == 3;
+  }
+
   void impl_call_pre (const call_details &cd) const final override
   {
     region_model_manager *mgr = cd.get_manager ();