From patchwork Tue Aug 22 22:39:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 1824379 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=s7TCqFQp; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4RVknx68f7z1yZs for ; Wed, 23 Aug 2023 08:40:27 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4F2EA3858426 for ; Tue, 22 Aug 2023 22:40:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4F2EA3858426 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1692744024; bh=5zuG6bQiHmflDHIB53O075q3C5Q0gVrUQbYH/a1jNkE=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=s7TCqFQpKIBKeX4HontZOyNwSSRw7yZ/sAnzlA6vGcHx51nePDQQfmmbkoOVHbT54 eCW41nmTalKOAL3vZa8m68hKusbg54h37RQKZ0hPnDiI0X4tXX4J9RcM06OSQui/hF pusTMKB7I6ZUMKZK0G2UuaqAvlqr2G+PFOToD9YU= 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.133.124]) by sourceware.org (Postfix) with ESMTPS id D2F433858D37 for ; Tue, 22 Aug 2023 22:40:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D2F433858D37 Received: from mimecast-mx02.redhat.com (66.187.233.73 [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-478-JtMMs_zwM2iM2S2Ilg2GRA-1; Tue, 22 Aug 2023 18:40:01 -0400 X-MC-Unique: JtMMs_zwM2iM2S2Ilg2GRA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id BA93C3814946 for ; Tue, 22 Aug 2023 22:40:00 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.22.32.117]) by smtp.corp.redhat.com (Postfix) with ESMTP id 70CD81121314; Tue, 22 Aug 2023 22:40:00 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [pushed] analyzer: reimplement kf_strlen [PR105899] Date: Tue, 22 Aug 2023 18:39:58 -0400 Message-Id: <20230822223958.2936206-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, 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+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Reimplement kf_strlen in terms of the new string scanning implementation, sharing strlen's implementation with __analyzer_get_strlen. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r14-3391-g3242fb533d48ab. gcc/analyzer/ChangeLog: PR analyzer/105899 * kf-analyzer.cc (class kf_analyzer_get_strlen): Move to kf.cc. (register_known_analyzer_functions): Use make_kf_strlen. * kf.cc (class kf_strlen::impl_call_pre): Replace with implementation of kf_analyzer_get_strlen from kf-analyzer.cc. Handle "UNKNOWN" return from check_for_null_terminated_string_arg by falling back to a conjured svalue. (make_kf_strlen): New. (register_known_functions): Use make_kf_strlen. * known-function-manager.h (make_kf_strlen): New decl. gcc/testsuite/ChangeLog: PR analyzer/105899 * gcc.dg/analyzer/null-terminated-strings-1.c: Update expected results on symbolic values. * gcc.dg/analyzer/strlen-1.c: New test. --- gcc/analyzer/kf-analyzer.cc | 30 +--------- gcc/analyzer/kf.cc | 56 +++++++++---------- gcc/analyzer/known-function-manager.h | 2 + .../analyzer/null-terminated-strings-1.c | 4 +- gcc/testsuite/gcc.dg/analyzer/strlen-1.c | 54 ++++++++++++++++++ 5 files changed, 85 insertions(+), 61 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/strlen-1.c diff --git a/gcc/analyzer/kf-analyzer.cc b/gcc/analyzer/kf-analyzer.cc index c767ebcb6615..7ae598a89123 100644 --- a/gcc/analyzer/kf-analyzer.cc +++ b/gcc/analyzer/kf-analyzer.cc @@ -358,33 +358,6 @@ public: } }; -/* Handler for "__analyzer_get_strlen". */ - -class kf_analyzer_get_strlen : public known_function -{ -public: - bool matches_call_types_p (const call_details &cd) const final override - { - return cd.num_args () == 1 && cd.arg_is_pointer_p (0); - } - void impl_call_pre (const call_details &cd) const final override - { - if (const svalue *bytes_read = cd.check_for_null_terminated_string_arg (0)) - { - region_model_manager *mgr = cd.get_manager (); - /* strlen is (bytes_read - 1). */ - const svalue *strlen_sval - = mgr->get_or_create_binop (size_type_node, - MINUS_EXPR, - bytes_read, - mgr->get_or_create_int_cst (size_type_node, 1)); - cd.maybe_set_lhs (strlen_sval); - } - else - cd.set_any_lhs_with_defaults (); - } -}; - /* Populate KFM with instances of known functions used for debugging the analyzer and for writing DejaGnu tests, all with a "__analyzer_" prefix. */ @@ -406,8 +379,7 @@ register_known_analyzer_functions (known_function_manager &kfm) kfm.add ("__analyzer_eval", make_unique ()); kfm.add ("__analyzer_get_unknown_ptr", make_unique ()); - kfm.add ("__analyzer_get_strlen", - make_unique ()); + kfm.add ("__analyzer_get_strlen", make_kf_strlen ()); } } // namespace ana diff --git a/gcc/analyzer/kf.cc b/gcc/analyzer/kf.cc index 1601cf15c685..59f46bab581c 100644 --- a/gcc/analyzer/kf.cc +++ b/gcc/analyzer/kf.cc @@ -1187,7 +1187,7 @@ public: } }; -/* Handle the on_call_pre part of "strlen". */ +/* Handler for "strlen" and for "__analyzer_get_strlen". */ class kf_strlen : public known_function { @@ -1196,37 +1196,33 @@ public: { return (cd.num_args () == 1 && cd.arg_is_pointer_p (0)); } - void impl_call_pre (const call_details &cd) const final override; -}; - -void -kf_strlen::impl_call_pre (const call_details &cd) const -{ - region_model_context *ctxt = cd.get_ctxt (); - region_model *model = cd.get_model (); - region_model_manager *mgr = cd.get_manager (); - - const svalue *arg_sval = cd.get_arg_svalue (0); - const region *buf_reg - = model->deref_rvalue (arg_sval, cd.get_arg_tree (0), ctxt); - if (const string_region *str_reg - = buf_reg->dyn_cast_string_region ()) - { - tree str_cst = str_reg->get_string_cst (); - /* TREE_STRING_LENGTH is sizeof, not strlen. */ - int sizeof_cst = TREE_STRING_LENGTH (str_cst); - int strlen_cst = sizeof_cst - 1; - if (cd.get_lhs_type ()) + void impl_call_pre (const call_details &cd) const final override + { + if (const svalue *bytes_read = cd.check_for_null_terminated_string_arg (0)) + if (bytes_read->get_kind () != SK_UNKNOWN) { - tree t_cst = build_int_cst (cd.get_lhs_type (), strlen_cst); - const svalue *result_sval - = mgr->get_or_create_constant_svalue (t_cst); - cd.maybe_set_lhs (result_sval); + region_model_manager *mgr = cd.get_manager (); + /* strlen is (bytes_read - 1). */ + const svalue *one = mgr->get_or_create_int_cst (size_type_node, 1); + const svalue *strlen_sval = mgr->get_or_create_binop (size_type_node, + MINUS_EXPR, + bytes_read, + one); + cd.maybe_set_lhs (strlen_sval); return; } - } - /* Otherwise a conjured value. */ - cd.set_any_lhs_with_defaults (); + + /* Use a conjured svalue. */ + cd.set_any_lhs_with_defaults (); + } +}; + +/* Factory function, so that kf-analyzer.cc can use this class. */ + +std::unique_ptr +make_kf_strlen () +{ + return make_unique (); } /* Handler for "strndup" and "__builtin_strndup". */ @@ -1434,7 +1430,7 @@ register_known_functions (known_function_manager &kfm) kfm.add (BUILT_IN_STRCPY_CHK, make_unique (3)); kfm.add (BUILT_IN_STRDUP, make_unique ()); kfm.add (BUILT_IN_STRNDUP, make_unique ()); - kfm.add (BUILT_IN_STRLEN, make_unique ()); + kfm.add (BUILT_IN_STRLEN, make_kf_strlen ()); register_atomic_builtins (kfm); register_varargs_builtins (kfm); diff --git a/gcc/analyzer/known-function-manager.h b/gcc/analyzer/known-function-manager.h index 4a76136f2594..1432e548acb9 100644 --- a/gcc/analyzer/known-function-manager.h +++ b/gcc/analyzer/known-function-manager.h @@ -64,6 +64,8 @@ private: known_function *m_combined_fns_arr[CFN_LAST]; }; +extern std::unique_ptr make_kf_strlen (); + } // namespace ana #endif /* GCC_ANALYZER_KNOWN_FUNCTION_MANAGER_H */ diff --git a/gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c b/gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c index 1db82a76d3b3..ecd794a2337a 100644 --- a/gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c @@ -129,12 +129,12 @@ char *test_dynamic_4 (const char *src) void test_symbolic_ptr (const char *ptr) { - __analyzer_describe (0, __analyzer_get_strlen (ptr)); /* { dg-warning "UNKNOWN" } */ + __analyzer_describe (0, __analyzer_get_strlen (ptr)); /* { dg-warning "CONJURED" } */ } void test_symbolic_offset (size_t idx) { - __analyzer_describe (0, __analyzer_get_strlen ("abc" + idx)); /* { dg-warning "UNKNOWN" } */ + __analyzer_describe (0, __analyzer_get_strlen ("abc" + idx)); /* { dg-warning "CONJURED" } */ } void test_casts (void) diff --git a/gcc/testsuite/gcc.dg/analyzer/strlen-1.c b/gcc/testsuite/gcc.dg/analyzer/strlen-1.c new file mode 100644 index 000000000000..99ce3aa297b5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/strlen-1.c @@ -0,0 +1,54 @@ +/* See e.g. https://en.cppreference.com/w/c/string/byte/strlen */ + +#include "analyzer-decls.h" + +typedef __SIZE_TYPE__ size_t; + +static size_t __attribute__((noinline)) +call_strlen_1 (const char *p) +{ + return __builtin_strlen (p); +} + +void test_string (void) +{ + __analyzer_eval (call_strlen_1 ("abc") == 3); /* { dg-warning "TRUE" } */ +} + +static size_t __attribute__((noinline)) +call_strlen_2 (const char *p) +{ + return __builtin_strlen (p); /* { dg-warning "stack-based buffer over-read" } */ +} + +void test_unterminated (void) +{ + const char buf[3] = "abc"; + __analyzer_eval (call_strlen_2 (buf) == 3); /* { dg-warning "UNKNOWN" } */ +} + +void test_uninitialized (void) +{ + char buf[16]; + __builtin_strlen (buf); /* { dg-warning "use of uninitialized value 'buf\\\[0\\\]'" } */ +} + +void test_partially_initialized (void) +{ + char buf[16]; + buf[0] = 'a'; + __builtin_strlen (buf); /* { dg-warning "use of uninitialized value 'buf\\\[1\\\]'" } */ +} + +extern size_t strlen (const char *str); + +size_t +test_passthrough (const char *str) +{ + return strlen (str); +} + +/* TODO + - complain if NULL str + - should it be tainted if str's bytes are tainted? +*/