From 21501ec751c102ce06ff3483375eb922c5c9cee3 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Wed, 16 Nov 2022 08:23:02 -0500 Subject: [PATCH] analyzer: use known_function to simplify region_model::on_call_{pre,post} 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). 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 --- 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 --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 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 (cd, false)); - cd.get_ctxt ()->bifurcate (make_unique (cd, true)); - cd.get_ctxt ()->terminate_path (); - } -} + bool matches_call_types_p (const call_details &cd) const final override + { + return cd.num_args () == 3; + } -/* Handle the on_call_post part of "bind". */ + void impl_call_post (const call_details &cd) const final override + { + if (cd.get_ctxt ()) + { + cd.get_ctxt ()->bifurcate (make_unique (cd, false)); + cd.get_ctxt ()->bifurcate (make_unique (cd, true)); + cd.get_ctxt ()->terminate_path (); + } + } +}; -void -region_model::impl_call_bind (const call_details &cd) +/* Handle calls to "bind". + See e.g. https://man7.org/linux/man-pages/man3/bind.3p.html */ + +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 (cd, false)); - cd.get_ctxt ()->bifurcate (make_unique (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 (cd, false)); + cd.get_ctxt ()->bifurcate (make_unique (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 (cd, false)); - cd.get_ctxt ()->bifurcate (make_unique (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 (cd, false)); + cd.get_ctxt ()->bifurcate (make_unique (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 (cd, false)); - cd.get_ctxt ()->bifurcate (make_unique (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 (cd, false)); + cd.get_ctxt ()->bifurcate (make_unique (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 (cd, false)); - cd.get_ctxt ()->bifurcate (make_unique (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 (cd, false)); + cd.get_ctxt ()->bifurcate (make_unique (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 ()); + mgr.add ("bind", make_unique ()); + mgr.add ("connect", make_unique ()); + mgr.add ("listen", make_unique ()); + mgr.add ("socket", make_unique ()); +} + } // 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 ();