From patchwork Wed Nov 16 13:29:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 60696 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 83106395A43F for ; Wed, 16 Nov 2022 13:30:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 83106395A43F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1668605419; bh=mVqAPp+C+F25rJcuC4xNEQgW+YQfvmICa7Gs+NU8g6c=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=DSAcQojmuvVTe6+QALzGSsBSh78p3pl1CTgpqDovjqS7jp2H4+zb8vcAG2VzNON+J CQna4ttshFBDQmXHUtHuwXgctprbBL1dEJarv1fDELRoaZQB0VhoXf3aMtOrtuwKR+ Itwt6i4yMFAzpawxChQepe3dhguc7GMT8Ww86thk= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 0DA1A395A06D for ; Wed, 16 Nov 2022 13:29:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0DA1A395A06D Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-609-XTtkW2zpNlSGNTqLhtdPmg-1; Wed, 16 Nov 2022 08:29:46 -0500 X-MC-Unique: XTtkW2zpNlSGNTqLhtdPmg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 207B53C0F23E for ; Wed, 16 Nov 2022 13:29:46 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.17.189]) by smtp.corp.redhat.com (Postfix) with ESMTP id E27D649BB60; Wed, 16 Nov 2022 13:29:45 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed] analyzer: use known_function to simplify region_model::on_call_{pre, post} Date: Wed, 16 Nov 2022 08:29:42 -0500 Message-Id: <20221116132942.2958189-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" 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 --- 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; + } + + 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 "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 (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 ();