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: 136609 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a7d1:0:b0:3f2:4152:657d with SMTP id p17csp112105vqm; Tue, 22 Aug 2023 15:40:48 -0700 (PDT) X-Google-Smtp-Source: AGHT+IErJXp4eLVbioa0vUBHUmhzE9E0QnlBs/zJeRCu7Wv8Mvx7Tfme/BXQ2fh29AbddY0Zeb8d X-Received: by 2002:a17:906:105c:b0:9a1:72f9:49fe with SMTP id j28-20020a170906105c00b009a172f949femr9028052ejj.65.1692744048677; Tue, 22 Aug 2023 15:40:48 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1692744048; cv=none; d=google.com; s=arc-20160816; b=XAu82bQ0lEdqRJ+6R4M4g17vE4VxqNHOsGcG9XIUIXrM2Pkm0cbqBxeeEZTaA1Ujmy fS1bJtPB3aIGAeqJhpudRx/RDVCDM44lw5w034fgj9fZH5C18rKtyW3MDzBRi5H+ok5g 5Oihz2PIwjhg0y1LKQTWkmPVWyWGrIJBF3ZrGcu20elgNkcPmVLta19b47F1sVxA2ico TTVFx8iY+JXqDRe8qt5f89dQItPUoo2dmt42une4RgcivsmszsWg/MJZU42Oj1LsUMgB NhsIEoLZFQMG5Be5KHBOEnjyarLEZVrx8bBPJFKSx8PaLqZXehQYAO+r9XW/jWozdZhq kj4g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:cc :to:dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=5zuG6bQiHmflDHIB53O075q3C5Q0gVrUQbYH/a1jNkE=; fh=fa0YJSH7y4k0snNuxsKgOI6vGP7j2f0jR9F1WHC95UA=; b=HvpQeRw42B1dJenBgfP2Fqf0HSxI57+wjU/Gxng5LvuIuT1KVRUth2kZ//ozwYCKgP G+W4bm2S8snE+w+XAkSx7WOl/m1mQc+SNpqrGwRKVtRFxsIrHgKJAOLkCr53vbbWNlS2 03sfxlp5pU7ZJIFWyYcjHbsjb46EeVxDUiDosLM/n4TWjSxUZbDB+U5hjsXjuXHnYF5v qW+XXlW1MuQXsBSqUneM95Si2ixNVZqwlIdKExNtelMhcqAoIXDsGdSux/98vwhtZ/US YnFMILdhWG1AKYeDM8VihvXdLV/FrhWSMoI64LwgJlJDXKo7zTh6PYUmeTN+MZpMarMV o5jQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=YTjcB28F; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (ip-8-43-85-97.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id p17-20020a170906839100b009952c411259si8213873ejx.197.2023.08.22.15.40.48 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 22 Aug 2023 15:40:48 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=YTjcB28F; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 88E7D3858C53 for ; Tue, 22 Aug 2023 22:40:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 88E7D3858C53 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1692744047; 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=YTjcB28F937VNgzgzKiKCCA2OOuU6ys1JSkiOIq/OdaA6yLMsc9kZDOh9jIW12yfE TEW5F6OH4O3+ehzKTq7xO1YGzwBWbPQ5JbOCGmi40uid/UdRHPHW3VaZbXoSpTM+83 psuVU2kMUB8ZBJm1r4PzuM7JxtC1iZrAiHhImWfE= 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+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1774970783953606844 X-GMAIL-MSGID: 1774970783953606844 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? +*/