From patchwork Wed Mar 22 12:43:41 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 73402 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:604a:0:0:0:0:0 with SMTP id j10csp2320103wrt; Wed, 22 Mar 2023 05:44:29 -0700 (PDT) X-Google-Smtp-Source: AK7set+8drILmwtXs/ROMnSZuRkIpuxfgn9uiyf4DC9cZw7cmNfBLPGV5jrcq9evJOWozacgL3+X X-Received: by 2002:a17:906:538e:b0:926:7d96:9434 with SMTP id g14-20020a170906538e00b009267d969434mr6472036ejo.51.1679489069056; Wed, 22 Mar 2023 05:44:29 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1679489069; cv=none; d=google.com; s=arc-20160816; b=OI2iz2zuEnlO1lAv5BrV1+KyqeKqucbBq+Zwq4WshGYiXJZkkkjcc8w4EpRsPlsXBy W/ZvdNJsqRljPnxS8dQGEFYyXJkOvFOtcJ999RFT32H+GFP1cCOJL/TQrNCe7ZnLvBQt h4XW5Zy1KejnCebXp5K3guOEy80cckLh/wWW7DH9lG7FcL98BybWwTRhPqJXM6q0x8wv zgnzXvo56QdbTVE+Zb3xahnYSJM4Brb4V4prndlQZGiz0K8xNgAuK9+01hsJWeI24Zx5 cO2sXGlGsgmbOHh3eO2nzegHG+zl1kBC2eL5oHzMDmG7t4hYNr1B0rNrxjE3L4r4/o6V Eauw== 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=eT6B45b25Mcy5NqL2aRI+0IIr4NE4avHr8QD+t+BKR0=; b=alVt2DtoXR8nz9Y8Q5gRZF9wQ/PcHCgcd4SVna45Df7T54p2Bt1O2pbdngAwA6BCWh 4BHntVx9UVlSgbBCqS15ALVm3gikfc2tJoFP7Alco02CFapjcXHfiHso3g11oMD5urYY p/mShAShKVshjp/0jvzj3eUkhd8a8gXoF47XkF0RByBswmV6CoxwYEUR/NZ9CRIuvkvG EQNY4hjIT81ZwJn9LUam3UJ4cMOF6GkJ56IPfhydUHPqDvRSral8nSPgfbKwDsUUKjTO OwboFnqSg2Q+3DhnMHTI1tSfRbRjiasgouykhtED4pbKv/CLaRznjtZEvwI2uNCiPzKh 6QCw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=LrfbtDAJ; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c 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. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id gg6-20020a170906e28600b00927fa026899si14368712ejb.327.2023.03.22.05.44.28 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Mar 2023 05:44:29 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=LrfbtDAJ; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c 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 D60C3385840E for ; Wed, 22 Mar 2023 12:44:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D60C3385840E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1679489067; bh=eT6B45b25Mcy5NqL2aRI+0IIr4NE4avHr8QD+t+BKR0=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=LrfbtDAJJ/86/13tM9zY0BKI/j1+bEU8ouOz1RD29EA3UOmckchNFQaSP8sQQ5BnJ 7RFt2jaRjIgPpLzQvbCrT/35GNaeyuexjsPilIrYw7ZJBI+QkNFaCfKD6aTIzA+G9f ahF/L/uSAZvBFCR+l0LYASkJF7bRQVTRkYSxgI8A= 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 D490B3858C52 for ; Wed, 22 Mar 2023 12:43:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D490B3858C52 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-480-d4x0F4D2PI2uErWMD4wKkw-1; Wed, 22 Mar 2023 08:43:43 -0400 X-MC-Unique: d4x0F4D2PI2uErWMD4wKkw-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3BC21101157A for ; Wed, 22 Mar 2023 12:43:43 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.17.70]) by smtp.corp.redhat.com (Postfix) with ESMTP id 132984021B1; Wed, 22 Mar 2023 12:43:43 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [pushed] analyzer: fix false +ves from -Wanalyzer-deref-before-check due to inlining [PR109239] Date: Wed, 22 Mar 2023 08:43:41 -0400 Message-Id: <20230322124341.3976040-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.4 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_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?1761071929782674935?= X-GMAIL-MSGID: =?utf-8?q?1761071929782674935?= The patch has this effect on my integration tests of -fanalyzer: Comparison: GOOD: 129 (17.70% -> 17.92%) BAD: 600 -> 591 (-9) which is purely due to improvements to -Wanalyzer-deref-before-check on the Linux kernel: -Wanalyzer-deref-before-check: GOOD: 1 (4.55% -> 7.69%) BAD: 21 -> 12 (-9) Known false positives: 16 -> 10 (-6) linux-5.10.162: 7 -> 1 (-6) Suspected false positives: 3 -> 0 (-3) linux-5.10.162: 3 -> 0 (-3) Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-6800-g0c652ebbf79bd1. gcc/analyzer/ChangeLog: PR analyzer/109239 * program-point.cc: Include "analyzer/inlining-iterator.h". (program_point::effectively_intraprocedural_p): New function. * program-point.h (program_point::effectively_intraprocedural_p): New decl. * sm-malloc.cc (deref_before_check::emit): Use it when rejecting interprocedural cases, so that we reject interprocedural cases that have become intraprocedural due to inlining. gcc/testsuite/ChangeLog: PR analyzer/109239 * gcc.dg/analyzer/deref-before-check-pr109239-linux-bus.c: New test. Signed-off-by: David Malcolm --- gcc/analyzer/program-point.cc | 42 +++++ gcc/analyzer/program-point.h | 3 + gcc/analyzer/sm-malloc.cc | 9 +- .../deref-before-check-pr109239-linux-bus.c | 153 ++++++++++++++++++ 4 files changed, 203 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr109239-linux-bus.c diff --git a/gcc/analyzer/program-point.cc b/gcc/analyzer/program-point.cc index 0ab55face16..f2d6490f0c0 100644 --- a/gcc/analyzer/program-point.cc +++ b/gcc/analyzer/program-point.cc @@ -52,6 +52,7 @@ along with GCC; see the file COPYING3. If not see #include "shortest-paths.h" #include "analyzer/exploded-graph.h" #include "analyzer/analysis-plan.h" +#include "analyzer/inlining-iterator.h" #if ENABLE_ANALYZER @@ -719,6 +720,47 @@ program_point::get_next () const } } +/* Return true iff POINT_A and POINT_B share the same function and + call_string, both directly, and when attempting to undo inlining + information. */ + +bool +program_point::effectively_intraprocedural_p (const program_point &point_a, + const program_point &point_b) +{ + /* First, compare without considering inlining info. */ + if (point_a.get_function () + != point_b.get_function ()) + return false; + if (&point_a.get_call_string () + != &point_b.get_call_string ()) + return false; + + /* Consider inlining info; they must have originally come from + the same function and have been inlined in the same way. */ + location_t loc_a = point_a.get_location (); + location_t loc_b = point_b.get_location (); + inlining_iterator iter_a (loc_a); + inlining_iterator iter_b (loc_b); + while (!(iter_a.done_p () || iter_b.done_p ())) + { + if (iter_a.done_p () || iter_b.done_p ()) + return false; + + if (iter_a.get_fndecl () != iter_b.get_fndecl ()) + return false; + if (iter_a.get_callsite () != iter_b.get_callsite ()) + return false; + if (iter_a.get_block () != iter_b.get_block ()) + return false; + + iter_a.next (); + iter_b.next (); + } + + return true; +} + #if CHECKING_P namespace selftest { diff --git a/gcc/analyzer/program-point.h b/gcc/analyzer/program-point.h index d1f8480fa8c..7df3b69c513 100644 --- a/gcc/analyzer/program-point.h +++ b/gcc/analyzer/program-point.h @@ -299,6 +299,9 @@ public: program_point get_next () const; + static bool effectively_intraprocedural_p (const program_point &point_a, + const program_point &point_b); + private: program_point (const function_point &fn_point) : m_function_point (fn_point), diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc index 16883d301d5..74701375409 100644 --- a/gcc/analyzer/sm-malloc.cc +++ b/gcc/analyzer/sm-malloc.cc @@ -1520,10 +1520,11 @@ public: if (!m_check_enode) return false; /* Only emit the warning for intraprocedural cases. */ - if (m_deref_enode->get_function () != m_check_enode->get_function ()) - return false; - if (&m_deref_enode->get_point ().get_call_string () - != &m_check_enode->get_point ().get_call_string ()) + const program_point &deref_point = m_deref_enode->get_point (); + const program_point &check_point = m_check_enode->get_point (); + + if (!program_point::effectively_intraprocedural_p (deref_point, + check_point)) return false; /* Reject the warning if the check occurs within a macro defintion. diff --git a/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr109239-linux-bus.c b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr109239-linux-bus.c new file mode 100644 index 00000000000..49b6420cc6b --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr109239-linux-bus.c @@ -0,0 +1,153 @@ +/* Reduced from linux-5.10.162's drivers-base-bus.c */ +/* { dg-additional-options "-fno-delete-null-pointer-checks -O2" } */ + +#define NULL ((void*)0) + +typedef unsigned int __kernel_size_t; +typedef int __kernel_ssize_t; +typedef __kernel_size_t size_t; +typedef __kernel_ssize_t ssize_t; + +struct list_head +{ + struct list_head *next, *prev; +}; + +struct kobject +{ + /* [...snip...] */ +}; + +struct attribute +{ + /* [...snip...] */ +}; + +static inline +void +sysfs_remove_file_ns(struct kobject* kobj, + const struct attribute* attr, + const void* ns) +{ +} + +static inline +void +sysfs_remove_file(struct kobject* kobj, const struct attribute* attr) +{ + sysfs_remove_file_ns(kobj, attr, NULL); +} + +extern struct kobject* +kobject_get(struct kobject* kobj); + +extern void +kobject_put(struct kobject* kobj); + +struct kset +{ + struct list_head list; + /* [...snip...] */ + struct kobject kobj; + /* [...snip...] */ +} __attribute__((__designated_init__)); + +static inline +struct kset* +to_kset(struct kobject* kobj) +{ + return kobj ? ({ + void* __mptr = (void*)(kobj); + ((struct kset*)(__mptr - __builtin_offsetof(struct kset, kobj))); + }) : NULL; +} + +static inline +struct kset* +kset_get(struct kset* k) +{ + return k ? to_kset(kobject_get(&k->kobj)) : NULL; +} + +static inline +void +kset_put(struct kset* k) +{ + kobject_put(&k->kobj); +} + +struct bus_type +{ + /* [...snip...] */ + struct device* dev_root; + /* [...snip...] */ + struct subsys_private* p; + /* [...snip...] */ +}; + +struct bus_attribute +{ + struct attribute attr; + /* [...snip...] */ +}; + +extern void +device_unregister(struct device* dev); + +struct subsys_private +{ + struct kset subsys; + /* [...snip...] */ +}; + +static struct bus_type* +bus_get(struct bus_type* bus) +{ + if (bus) { /* { dg-bogus "check of 'bus' for NULL after already dereferencing it" } */ + kset_get(&bus->p->subsys); + return bus; + } + return NULL; +} + +static void +bus_put(struct bus_type* bus) +{ + if (bus) + kset_put(&bus->p->subsys); +} + +void +bus_remove_file(struct bus_type* bus, struct bus_attribute* attr) +{ + if (bus_get(bus)) { + sysfs_remove_file(&bus->p->subsys.kobj, &attr->attr); + bus_put(bus); + } +} + +extern ssize_t +drivers_autoprobe_show(struct bus_type* bus, char* buf); + +extern ssize_t +drivers_autoprobe_store(struct bus_type* bus, const char* buf, size_t count); + +extern struct bus_attribute bus_attr_drivers_autoprobe; + +static void +remove_probe_files(struct bus_type* bus) +{ + bus_remove_file(bus, &bus_attr_drivers_autoprobe); + /* [...snip...] */ +} + +void +bus_unregister(struct bus_type* bus) +{ + /* [...snip...] */ + if (bus->dev_root) /* { dg-bogus "pointer 'bus' is dereferenced here" } */ + device_unregister(bus->dev_root); + /* [...snip...] */ + remove_probe_files(bus); + /* [...snip...] */ +}