diff mbox series

[pushed] analyzer: fix deref-before-check false positives due to inlining [PR112790]

Message ID 20240104142716.427365-1-dmalcolm@redhat.com
State New
Headers show
Series [pushed] analyzer: fix deref-before-check false positives due to inlining [PR112790] | expand

Commit Message

David Malcolm Jan. 4, 2024, 2:27 p.m. UTC 
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Successful run of analyzer integration tests on x86_64-pc-linux-gnu.
Pushed to trunk as r14-6918-g5743e1899d5964.

gcc/analyzer/ChangeLog:
	PR analyzer/112790
	* checker-event.cc (class inlining_info): Move to...
	* inlining-iterator.h (class inlining_info): ...here.
	* sm-malloc.cc: Include "analyzer/inlining-iterator.h".
	(maybe_complain_about_deref_before_check): Reject stmts that were
	inlined from another function.

gcc/testsuite/ChangeLog:
	PR analyzer/112790
	* c-c++-common/analyzer/deref-before-check-pr112790.c: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/analyzer/checker-event.cc                 | 40 -------------------
 gcc/analyzer/inlining-iterator.h              | 40 +++++++++++++++++++
 gcc/analyzer/sm-malloc.cc                     | 10 +++++
 .../analyzer/deref-before-check-pr112790.c    | 27 +++++++++++++
 4 files changed, 77 insertions(+), 40 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr112790.c
diff mbox series

Patch

diff --git a/gcc/analyzer/checker-event.cc b/gcc/analyzer/checker-event.cc
index 3612df7bd1d3..3cb2fb9175cd 100644
--- a/gcc/analyzer/checker-event.cc
+++ b/gcc/analyzer/checker-event.cc
@@ -106,46 +106,6 @@  event_kind_to_string (enum event_kind ek)
     }
 }
 
-/* A class for fixing up fndecls and stack depths in checker_event, based
-   on inlining records.
-
-   The early inliner runs before the analyzer, which can lead to confusing
-   output.
-
-   Tne base fndecl and depth within a checker_event are from call strings
-   in program_points, which reflect the call strings after inlining.
-   This class lets us offset the depth and fix up the reported fndecl and
-   stack depth to better reflect the user's original code.  */
-
-class inlining_info
-{
-public:
-  inlining_info (location_t loc)
-  {
-    inlining_iterator iter (loc);
-    m_inner_fndecl = iter.get_fndecl ();
-    int num_frames = 0;
-    while (!iter.done_p ())
-      {
-	m_outer_fndecl = iter.get_fndecl ();
-	num_frames++;
-	iter.next ();
-      }
-    if (num_frames > 1)
-      m_extra_frames = num_frames - 1;
-    else
-      m_extra_frames = 0;
-  }
-
-  tree get_inner_fndecl () const { return m_inner_fndecl; }
-  int get_extra_frames () const { return m_extra_frames; }
-
-private:
-  tree m_outer_fndecl;
-  tree m_inner_fndecl;
-  int m_extra_frames;
-};
-
 /* class checker_event : public diagnostic_event.  */
 
 /* checker_event's ctor.  */
diff --git a/gcc/analyzer/inlining-iterator.h b/gcc/analyzer/inlining-iterator.h
index 7d4798ce6ea5..76ba1dc3cf4a 100644
--- a/gcc/analyzer/inlining-iterator.h
+++ b/gcc/analyzer/inlining-iterator.h
@@ -106,4 +106,44 @@  private:
   tree m_next_abstract_origin;
 };
 
+/* A class for fixing up fndecls and stack depths in checker_event, based
+   on inlining records.
+
+   The early inliner runs before the analyzer, which can lead to confusing
+   output.
+
+   Tne base fndecl and depth within a checker_event are from call strings
+   in program_points, which reflect the call strings after inlining.
+   This class lets us offset the depth and fix up the reported fndecl and
+   stack depth to better reflect the user's original code.  */
+
+class inlining_info
+{
+public:
+  inlining_info (location_t loc)
+  {
+    inlining_iterator iter (loc);
+    m_inner_fndecl = iter.get_fndecl ();
+    int num_frames = 0;
+    while (!iter.done_p ())
+      {
+	m_outer_fndecl = iter.get_fndecl ();
+	num_frames++;
+	iter.next ();
+      }
+    if (num_frames > 1)
+      m_extra_frames = num_frames - 1;
+    else
+      m_extra_frames = 0;
+  }
+
+  tree get_inner_fndecl () const { return m_inner_fndecl; }
+  int get_extra_frames () const { return m_extra_frames; }
+
+private:
+  tree m_outer_fndecl;
+  tree m_inner_fndecl;
+  int m_extra_frames;
+};
+
 #endif /* GCC_ANALYZER_INLINING_ITERATOR_H */
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index bb7844483979..61ee9cfc1e28 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -46,6 +46,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "analyzer/program-state.h"
 #include "analyzer/checker-event.h"
 #include "analyzer/exploded-graph.h"
+#include "analyzer/inlining-iterator.h"
 
 #if ENABLE_ANALYZER
 
@@ -2168,6 +2169,15 @@  maybe_complain_about_deref_before_check (sm_context *sm_ctxt,
   if (checked_in_frame->get_index () > assumed_nonnull_in_frame->get_index ())
     return;
 
+  /* Don't complain if STMT was inlined from another function, to avoid
+     similar false positives involving shared helper functions.  */
+  if (stmt->location)
+    {
+      inlining_info info (stmt->location);
+      if (info.get_extra_frames () > 0)
+	return;
+    }
+
   tree diag_ptr = sm_ctxt->get_diagnostic_tree (ptr);
   if (diag_ptr)
     sm_ctxt->warn
diff --git a/gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr112790.c b/gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr112790.c
new file mode 100644
index 000000000000..8f74468f849b
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr112790.c
@@ -0,0 +1,27 @@ 
+/* Reproducer for false positive from -Wanalyzer-deref-before-check
+   seen on Linux kernel's block/bdev.c due to -fanalyzer mishandling
+   inlined functions.  */
+
+/* { dg-additional-options "-O2 -g -fno-delete-null-pointer-checks" } */
+
+typedef unsigned char u8;
+struct inode {
+  void *i_mapping;
+  u8 i_blkbits;
+};
+struct block_device {
+  struct inode *bd_inode;
+};
+int sync_blockdev(struct block_device *bdev);
+int set_blocksize(struct block_device *bdev, u8 size) {
+  if (bdev->bd_inode->i_blkbits != size) { /* { dg-bogus "pointer 'bdev' is dereferenced here" } */
+    sync_blockdev(bdev);
+  }
+  return 0;
+}
+extern int filemap_write_and_wait(void *);
+int sync_blockdev(struct block_device *bdev) {
+  if (!bdev) /* { dg-bogus "check of 'bdev' for NULL after already dereferencing it" } */
+    return 0;
+  return filemap_write_and_wait(bdev->bd_inode->i_mapping);
+}