diff mbox series

[pushed] analyzer: fix -Wanalyzer-deref-before-check false positive seen in loop header macro [PR109251]

Message ID 20240320223609.864290-1-dmalcolm@redhat.com
State New
Headers show
Series [pushed] analyzer: fix -Wanalyzer-deref-before-check false positive seen in loop header macro [PR109251] | expand

Commit Message

David Malcolm March 20, 2024, 10:36 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-9586-g9093f275e0a343.

gcc/analyzer/ChangeLog:
	PR analyzer/109251
	* sm-malloc.cc (deref_before_check::emit): Reject cases where the
	check is in a loop header within a macro expansion.
	(deref_before_check::loop_header_p): New.

gcc/testsuite/ChangeLog:
	PR analyzer/109251
	* c-c++-common/analyzer/deref-before-check-pr109251-1.c: New test.
	* c-c++-common/analyzer/deref-before-check-pr109251-2.c: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/analyzer/sm-malloc.cc                     | 30 ++++++++++
 .../analyzer/deref-before-check-pr109251-1.c  | 60 +++++++++++++++++++
 .../analyzer/deref-before-check-pr109251-2.c  | 37 ++++++++++++
 3 files changed, 127 insertions(+)
 create mode 100644 gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr109251-1.c
 create mode 100644 gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr109251-2.c
diff mbox series

Patch

diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index a518816b2b8..4e11d6dfc63 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -1563,6 +1563,21 @@  public:
     if (linemap_location_from_macro_definition_p (line_table, check_loc))
       return false;
 
+    /* Reject warning if the check is in a loop header within a
+       macro expansion.  This rejects cases like:
+       |  deref of x;
+       |  [...snip...]
+       |  FOR_EACH(x) {
+       |    [...snip...]
+       |  }
+       where the FOR_EACH macro tests for non-nullness of x, since
+       the user is hoping to encapsulate the details of iteration
+       in the macro, and the extra check on the first iteration
+       would just be noise if we reported it.  */
+    if (loop_header_p (m_check_enode->get_point ())
+	&& linemap_location_from_macro_expansion_p (line_table, check_loc))
+      return false;
+
     /* Reject if m_deref_expr is sufficiently different from m_arg
        for cases where the dereference is spelled differently from
        the check, which is probably two different ways to get the
@@ -1618,6 +1633,21 @@  public:
   }
 
 private:
+  static bool loop_header_p (const program_point &point)
+  {
+    const supernode *snode = point.get_supernode ();
+    if (!snode)
+      return false;
+    for (auto &in_edge : snode->m_preds)
+      {
+	if (const cfg_superedge *cfg_in_edge
+	      = in_edge->dyn_cast_cfg_superedge ())
+	  if (cfg_in_edge->back_edge_p ())
+	    return true;
+      }
+    return false;
+  }
+
   static bool sufficiently_similar_p (tree expr_a, tree expr_b)
   {
     pretty_printer *pp_a = global_dc->printer->clone ();
diff --git a/gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr109251-1.c b/gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr109251-1.c
new file mode 100644
index 00000000000..769cffae6d7
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr109251-1.c
@@ -0,0 +1,60 @@ 
+/* Reduced from linux-5.10.162's kernel/sched/fair.c,
+   with !CONFIG_FAIR_GROUP_SCHED.  */
+
+#define NULL ((void*)0)
+
+struct load_weight
+{
+  unsigned long weight;
+  /* [...snip...] */
+};
+
+struct sched_entity
+{
+  struct load_weight load;
+  /* [...snip...] */
+  unsigned int on_rq;
+  /* [...snip...] */
+};
+
+struct cfs_rq
+{
+  /* [...snip...] */
+  unsigned int nr_running;
+  /* [...snip...] */
+};
+
+extern int
+__calc_delta(int delta_exec, unsigned long weight /* [...snip...] */);
+
+/* !CONFIG_FAIR_GROUP_SCHED */
+#define for_each_sched_entity(se) \
+  for (; se; se = (struct sched_entity *)NULL)
+
+extern struct cfs_rq*
+cfs_rq_of(struct sched_entity* se);
+
+extern int
+__sched_period(unsigned long nr_running);
+
+int
+sched_slice(struct cfs_rq* cfs_rq, struct sched_entity* se)
+{
+  unsigned int nr_running = cfs_rq->nr_running;
+  int slice;
+
+  /* [...snip...] */
+
+  slice = __sched_period(nr_running + !se->on_rq);
+
+  for_each_sched_entity(se) {
+    /* [...snip...] */
+    cfs_rq = cfs_rq_of(se);
+    /* [...snip...] */
+    slice = __calc_delta(slice, se->load.weight);
+  }
+
+  /* [...snip...] */
+
+  return slice;
+}
diff --git a/gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr109251-2.c b/gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr109251-2.c
new file mode 100644
index 00000000000..8e85a47d315
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr109251-2.c
@@ -0,0 +1,37 @@ 
+struct node
+{
+  struct node *next;
+  int val;
+};
+
+int test_loop_1 (struct node *n)
+{
+  int total = 0;
+  if (n->val = 42)
+    return -1;
+  for (struct node *iter = n; iter; iter=iter->next)
+    total += iter->val;
+  return total;
+}
+
+int test_loop_2 (struct node *n)
+{
+  int total = 0;
+  if (n->val = 42)
+    return -1;
+  for (; n; n=n->next)
+    total += n->val;
+  return total;
+}
+
+#define FOR_EACH_NODE(ITER) for (; (ITER); (ITER)=(ITER)->next)
+
+int test_loop_3 (struct node *n)
+{
+  int total = 0;
+  if (n->val = 42)
+    return -1;
+  FOR_EACH_NODE (n)
+    total += n->val;
+  return total;
+}