From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1679489024; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=eT6B45b25Mcy5NqL2aRI+0IIr4NE4avHr8QD+t+BKR0=; b=OQdITfddWpm/vbeC1fbDYqsqORHDNTx6FFGam8oOunMyCjvn64SOeLe1RdbVkAiTE/Ilh1 PmHIHulqQPPxttETc8G3u/eiT9gIiwg1G2tdvl331GAFb+kLYk1Izv+780pek7srh1MIxY OW69urMB3EZwLikjuCSOVP9da1Wi0tE= 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) From: David Malcolm 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 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true 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 List-Id: 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...] */ +} -- 2.26.3