From patchwork Tue Feb 7 21:13:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 54083 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp3080370wrn; Tue, 7 Feb 2023 13:14:47 -0800 (PST) X-Google-Smtp-Source: AK7set8JsRnhEim5tFBc6dTp7kJYAxbFLT9RlhxH9vZsOHJm23y3DSub0vxWdjL3xfUnXAePo6oz X-Received: by 2002:a17:906:3792:b0:88d:5081:e9f8 with SMTP id n18-20020a170906379200b0088d5081e9f8mr4666538ejc.15.1675804487713; Tue, 07 Feb 2023 13:14:47 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675804487; cv=none; d=google.com; s=arc-20160816; b=KURgq8qbZl3OC8+XiwKiTwAP/3bdyYByG2V1RcNwHF5YqOcG2sC6WrQNNsCHoa8qVk NJrF8N56uYkX6IV5t0MoEj3zskMVIDrBbLN1a8RLvGuR30vul1jc42bOBuSh5puc/8j8 PenxViPEIMX60le2TkDZYag8Eo3lm+sJfwtG0mRb9RfqSm5yoeAVwoMXg+8yB8K66zA+ lX5H9OTOm5cXc1GsUBFCNCA0hA21klmeAo2+/DUDHXFwEdZixPurl5Gmjt0m+Mlem8DJ dXwT+M4MjwCS6rMuvMRNm8c8odyoql13fXE4Z4ZfiSrSagc5OpcHTC9JAraYTU7cX1M9 YlVA== 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=vpORZ9bbX/mUUysw7M196Oy/Pxwbx9M/uN0QsKLMJaA=; b=Nw5KBXAFsj1jAgK5sQ92fge6WHWmtXexNpCEyR+ErbbsXRxeIRxF7u3nUIu1CWAzlX zd8zXr1yuGPwp802tLheYvPmb587PoQ+WZP3RATIdFflyTXFDiwWgUDkiQe3RkixJbrh pXpwthmM3gm17xNsBDp8of49s1rZ80aIbg81IypnRvcJjX+Qu1hi4mOgjGEp8uibmMWz 1awge9p1oCFulZMCXha023SY5pHXCY/UiYY43HRi4ZS0mJ4cerg5ldseDjlziO2mEX7M KkOpZucZGLjZ0alDjVmLDhE962mFObBuhyKJigBBVJMpQYxCY8TQkd8bBGJEiqu330qV P2bw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=uhJygpO2; 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 sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id wg10-20020a1709078f0a00b00895ee930c9bsi10791986ejc.877.2023.02.07.13.14.47 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Feb 2023 13:14:47 -0800 (PST) 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=uhJygpO2; 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 ADC2C3858410 for ; Tue, 7 Feb 2023 21:14:46 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org ADC2C3858410 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1675804486; bh=vpORZ9bbX/mUUysw7M196Oy/Pxwbx9M/uN0QsKLMJaA=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=uhJygpO2f7vddptjWDcf4MBKdQhIMICLTgtg6/hXkhxjI3gifxLUCPQOv4PEBTOQN DPy26cNpBOOGmQb1DIuhQGpAA7TPmD5HroUDgX7lGYGKsIcBOFD2w67uyo1cXaURFS bH90xj6TygdZ7SH/D3pgcPzvBNew0s8yK+uIFOSE= 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 F41C83858D33 for ; Tue, 7 Feb 2023 21:14:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F41C83858D33 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-292-Ij2ZwazmObWU3WiyPxFE8A-1; Tue, 07 Feb 2023 16:13:59 -0500 X-MC-Unique: Ij2ZwazmObWU3WiyPxFE8A-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 40C74811E6E for ; Tue, 7 Feb 2023 21:13:59 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.16.227]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1B7B61121314; Tue, 7 Feb 2023 21:13:59 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [pushed] analyzer: fix -Wanalyzer-use-of-uninitialized-value false +ve on "read" [PR108661] Date: Tue, 7 Feb 2023 16:13:57 -0500 Message-Id: <20230207211357.1226071-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.5 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+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1757208366543955091?= X-GMAIL-MSGID: =?utf-8?q?1757208366543955091?= My integration testing shows many false positives from -Wanalyzer-use-of-uninitialized-value. One cause turns out to be that as of r13-1404-g97baacba963c06 fd_state_machine::on_stmt recognizes calls to "read", and returns true, so that region_model::on_call_post doesn't call handle_unrecognized_call on them, and so the analyzer erroneously "thinks" that the buffer pointed to by "read" is never touched by the "read" call. This works for "fread" because sm-file.cc implements kf_fread, which handles calls to "fread" by clobbering the buffer pointed to. In the long term we should probably be smarter about this and bifurcate the analysis to consider e.g. errors vs full reads vs partial reads, etc (which I'm tracking in PR analyzer/108689). In the meantime, this patch adds a kf_read for "read" analogous to the one for "fread", fixing 6 false positives seen in git-2.39.0 and 2 in haproxy-2.7.1. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-5733-gc300e251f5b22d. gcc/analyzer/ChangeLog: PR analyzer/108661 * sm-fd.cc (class kf_read): New. (register_known_fd_functions): Register "read". * sm-file.cc (class kf_fread): Update comment. gcc/testsuite/ChangeLog: PR analyzer/108661 * gcc.dg/analyzer/fread-pr108661.c: New test. * gcc.dg/analyzer/read-pr108661.c: New test. Signed-off-by: David Malcolm --- gcc/analyzer/sm-fd.cc | 33 +++++++++++++++ gcc/analyzer/sm-file.cc | 10 ++++- .../gcc.dg/analyzer/fread-pr108661.c | 40 +++++++++++++++++++ gcc/testsuite/gcc.dg/analyzer/read-pr108661.c | 33 +++++++++++++++ 4 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/fread-pr108661.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/read-pr108661.c diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc index 494d802a1d4..d107390bc00 100644 --- a/gcc/analyzer/sm-fd.cc +++ b/gcc/analyzer/sm-fd.cc @@ -2659,6 +2659,38 @@ private: unsigned m_num_args; }; +/* Handler for "read". + ssize_t read(int fildes, void *buf, size_t nbyte); + See e.g. https://man7.org/linux/man-pages/man2/read.2.html */ + +class kf_read : public known_function +{ +public: + bool matches_call_types_p (const call_details &cd) const final override + { + return (cd.num_args () == 3 + && cd.arg_is_pointer_p (1) + && cd.arg_is_size_p (2)); + } + + /* For now, assume that any call to "read" fully clobbers the buffer + passed in. This isn't quite correct (e.g. errors, partial reads; + see PR analyzer/108689), but at least stops us falsely complaining + about the buffer being uninitialized. */ + void impl_call_pre (const call_details &cd) const final override + { + region_model *model = cd.get_model (); + const svalue *ptr_sval = cd.get_arg_svalue (1); + if (const region *reg = ptr_sval->maybe_get_region ()) + { + const region *base_reg = reg->get_base_region (); + const svalue *new_sval = cd.get_or_create_conjured_svalue (base_reg); + model->set_value (base_reg, new_sval, cd.get_ctxt ()); + } + } +}; + + /* Populate KFM with instances of known functions relating to file descriptors. */ @@ -2672,6 +2704,7 @@ register_known_fd_functions (known_function_manager &kfm) kfm.add ("listen", make_unique ()); kfm.add ("pipe", make_unique (1)); kfm.add ("pipe2", make_unique (2)); + kfm.add ("read", make_unique ()); kfm.add ("socket", make_unique ()); } diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc index 9cb4e32e286..d99a09b76c4 100644 --- a/gcc/analyzer/sm-file.cc +++ b/gcc/analyzer/sm-file.cc @@ -560,7 +560,11 @@ public: } }; -/* Handler for "fread"". */ +/* Handler for "fread". + size_t fread(void *restrict buffer, size_t size, size_t count, + FILE *restrict stream); + See e.g. https://en.cppreference.com/w/c/io/fread + and https://www.man7.org/linux/man-pages/man3/fread.3.html */ class kf_fread : public known_function { @@ -574,6 +578,10 @@ public: && cd.arg_is_pointer_p (3)); } + /* For now, assume that any call to "fread" fully clobbers the buffer + passed in. This isn't quite correct (e.g. errors, partial reads; + see PR analyzer/108689), but at least stops us falsely complaining + about the buffer being uninitialized. */ void impl_call_pre (const call_details &cd) const final override { region_model *model = cd.get_model (); diff --git a/gcc/testsuite/gcc.dg/analyzer/fread-pr108661.c b/gcc/testsuite/gcc.dg/analyzer/fread-pr108661.c new file mode 100644 index 00000000000..b51cf41ec2a --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/fread-pr108661.c @@ -0,0 +1,40 @@ +typedef __SIZE_TYPE__ size_t; + +extern size_t fread (void *, size_t, size_t, void *); + +struct ring +{ + char buf[1024]; +}; + +int +test_one_large_item (void *fp) +{ + struct ring ring; + int ret; + + ret = fread(&ring, sizeof(ring), 1, fp); + + if (ret != 1) + return 1; + + if (ring.buf[0] > 1) /* { dg-bogus "use of uninitialized value" } */ + return 2; + return 3; +} + +int +test_many_small_items (void *fp) +{ + struct ring ring; + int ret; + + ret = fread(&ring, 1, sizeof(ring), fp); + + if (ret != sizeof(ring)) + return 1; + + if (ring.buf[0] > 1) /* { dg-bogus "use of uninitialized value" } */ + return 2; + return 3; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/read-pr108661.c b/gcc/testsuite/gcc.dg/analyzer/read-pr108661.c new file mode 100644 index 00000000000..70335e6fef1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/read-pr108661.c @@ -0,0 +1,33 @@ +typedef long int ssize_t; +typedef long unsigned int size_t; + +extern int open(const char* __file, int __oflag, ...) __attribute__((__nonnull__(1))); +extern int close(int __fd); +extern ssize_t read(int __fd, void* __buf, size_t __nbytes); + +struct ring +{ + char buf[1024]; +}; + +int +test(const char* name) +{ + struct ring ring; + int fd; + int ret; + + fd = open(name, 00); + if (fd < 0) + return 0; + + ret = read(fd, &ring, sizeof(ring)); + close(fd); + + if (ret != sizeof(ring)) + return 1; + + if (ring.buf[0] > 1) /* { dg-bogus "use of uninitialized value" } */ + return 2; + return 3; +}