* [pushed] analyzer: fix false +ves from -Wanalyzer-deref-before-check due to inlining [PR109239]
@ 2023-03-22 12:43 David Malcolm
0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2023-03-22 12:43 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
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 <dmalcolm@redhat.com>
---
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
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2023-03-22 12:43 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-22 12:43 [pushed] analyzer: fix false +ves from -Wanalyzer-deref-before-check due to inlining [PR109239] David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).