public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] analyzer: call off a superseding when diagnostics are unrelated [PR110830]
@ 2023-09-01 19:59 priour.be
  2023-09-01 23:48 ` David Malcolm
  0 siblings, 1 reply; 6+ messages in thread
From: priour.be @ 2023-09-01 19:59 UTC (permalink / raw)
  To: gcc-patches; +Cc: dmalcolm, benjamin priour

From: benjamin priour <vultkayn@gcc.gnu.org>

Hi,

Patch succesfully regstrapped off trunk 7f2ed06ddc825e8a4e0edfd1d66b5156e6dc1d34
on x86_64-linux-gnu.

Is it OK for trunk ?

Thanks,
Benjamin.

Patch below.
---

Before this patch, a saved_diagnostic would supersede another at
the same statement if and only its vfunc supercedes_p returned true
for the other diagnostic's kind.
That both warning were unrelated, that is resolving one would not fix
the other was not considered in making the above choice.

This patch makes it so that two saved_diagnostics taking a different
outcome of at least one common conditional branching cannot supersede
each other.

Signed-off-by: benjamin priour <vultkayn@gcc.gnu.org>

gcc/analyzer/ChangeLog:

	PR analyzer/110830
	* diagnostic-manager.cc
	(compatible_epaths_p): New function.
	(saved_diagnostic::supercedes_p): Now calls the above
	to determine if the diagnostics do overlap and the superseding
	may proceed.

gcc/testsuite/ChangeLog:

	PR analyzer/110830
	* c-c++-common/analyzer/pr110830.c: New test.
---
 gcc/analyzer/diagnostic-manager.cc            |  89 +++++++++++++-
 .../c-c++-common/analyzer/pr110830.c          | 111 ++++++++++++++++++
 2 files changed, 199 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/c-c++-common/analyzer/pr110830.c

diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 10fea486b8c..7cf181e7972 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -887,6 +887,87 @@ saved_diagnostic::add_duplicate (saved_diagnostic *other)
   m_duplicates.safe_push (other);
 }
 
+/* Walk up the two paths to each of their common conditional
+   branching.  At each branching, make sure both diagnostics'
+   paths branched similarly.  If there is at least one where
+   both paths go down a different outcome, then the paths
+   are incompatible and this function returns FALSE.
+   Otherwise return TRUE.
+
+   Incompatible paths:
+
+       <cond Y>
+       /      \
+      /        \
+    true      false
+     |          |
+    ...        ...
+     |          |
+    ...       stmt x
+     |
+   stmt x
+
+   Both LHS_PATH and RHS_PATH final enodes should be
+   over the same gimple statement.  */
+
+static bool
+compatible_epath_p (const exploded_path *lhs_path,
+		    const exploded_path *rhs_path)
+{
+  gcc_assert (lhs_path);
+  gcc_assert (rhs_path);
+  int i;
+  const exploded_edge *outer_eedge;
+  FOR_EACH_VEC_ELT_REVERSE (lhs_path->m_edges, i, outer_eedge)
+    {
+      const superedge *outer_sedge = outer_eedge->m_sedge;
+      if (!outer_sedge || !outer_eedge->m_src)
+	continue;
+      const program_point &outer_src_point = outer_eedge->m_src->get_point ();
+      switch (outer_src_point.get_kind ())
+	{
+	  case PK_AFTER_SUPERNODE:
+	    if (const cfg_superedge *cfg_outer_sedge
+		= outer_sedge->dyn_cast_cfg_superedge ())
+	      {
+		int j;
+		const exploded_edge *inner_eedge;
+		FOR_EACH_VEC_ELT_REVERSE (rhs_path->m_edges, j, inner_eedge)
+		  {
+		    const superedge *inner_sedge = inner_eedge->m_sedge;
+		    if (!inner_sedge || !inner_eedge->m_src)
+		      continue;
+		    const program_point &inner_src_point
+		      = inner_eedge->m_src->get_point ();
+		    switch (inner_src_point.get_kind ())
+		      {
+			case PK_AFTER_SUPERNODE:
+			  if (inner_src_point.get_stmt ()
+			      != outer_src_point.get_stmt ())
+			    continue;
+			  if (const cfg_superedge *cfg_inner_sedge
+			      = inner_sedge->dyn_cast_cfg_superedge ())
+			    {
+			      if (cfg_inner_sedge->true_value_p ()
+				  != cfg_outer_sedge->true_value_p ())
+				return false;
+			    }
+			  break;
+			default:
+			  break;
+		      }
+		  }
+	      }
+	    break;
+
+	  default:
+	    break;
+	}
+    }
+    return true;
+}
+
+
 /* Return true if this diagnostic supercedes OTHER, and that OTHER should
    therefore not be emitted.  */
 
@@ -896,7 +977,13 @@ saved_diagnostic::supercedes_p (const saved_diagnostic &other) const
   /* They should be at the same stmt.  */
   if (m_stmt != other.m_stmt)
     return false;
-  return m_d->supercedes_p (*other.m_d);
+  /* return early if OTHER won't be superseded anyway.  */
+  if (!m_d->supercedes_p (*other.m_d))
+    return false;
+
+  /* If the two saved_diagnostics' path are not compatible
+     then they cannot supersede one another.  */
+  return compatible_epath_p (m_best_epath.get (), other.m_best_epath.get ());
 }
 
 /* Move any saved checker_events from this saved_diagnostic to
diff --git a/gcc/testsuite/c-c++-common/analyzer/pr110830.c b/gcc/testsuite/c-c++-common/analyzer/pr110830.c
new file mode 100644
index 00000000000..9f6675ab693
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/pr110830.c
@@ -0,0 +1,111 @@
+typedef __SIZE_TYPE__ size_t;
+
+void free(void *);
+void *malloc(__SIZE_TYPE__);
+
+extern int ext();
+
+void test_supersedes ()
+{
+  int *p = (int *)malloc(sizeof(int));
+  free(p);
+  int x = *p + 4; /* { dg-warning "use after 'free' of 'p'" } */
+  /* { dg-bogus "use of uninitialized value '\\*p" "" { target *-*-* } .-1 } */
+}
+
+int *called_by_test0()
+{
+  int *p = 0;
+  if (ext())
+  {
+    p = (int *)malloc(sizeof(int));
+    free(p);
+    return p;
+  }
+  else
+    return (int *)malloc(sizeof(int));
+}
+
+void test0()
+{
+  int *y = called_by_test0();
+  int x = 0;
+  if (y != 0)
+    x = *y; /* { dg-warning "use after 'free' of 'y'" } */
+    /* { dg-warning "use of uninitialized value '\\*y" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+  free(y); /* { dg-warning "double-'free'" }  */
+}
+
+void test1()
+{
+  int *p = 0;
+  if (ext())
+  {
+    p = (int *)malloc(sizeof(int));
+    free(p);
+  }
+  else
+    p = (int *)malloc(sizeof(int));
+
+  int x = 0;
+  if (p != 0)
+    x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+    /* { dg-warning "use of uninitialized value '\\*p" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+  free(p); /* { dg-warning "double-'free'" }  */
+}
+
+void test2()
+{
+  int *p = 0;
+  p = (int *)malloc(sizeof(int));
+  if (ext())
+    free(p);
+
+  int x = 0;
+  if (p != 0)
+    x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+    /* { dg-warning "use of uninitialized value '\\*p" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+  free(p); /* { dg-warning "double-'free'" }  */
+}
+
+void test3()
+{
+  int *p = 0;
+  p = (int *)malloc(sizeof(int));
+  int i = 100;
+  while (i--)
+  {
+    int x = 0;
+    if (p != 0)
+      x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+      /* { dg-warning "use of uninitialized value '\\*p" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+    p = (int *)malloc(sizeof(int));
+    free(p);
+  }
+
+  free(p); /* { dg-warning "double-'free'" }  */
+}
+
+
+void test4()
+{
+  int *p = 0;
+  if (ext())
+  {
+    p = (int *) malloc(sizeof(int));
+    if (ext () > 5)
+    {
+      mal:
+      free (p);
+    }
+  }
+  else {
+    goto mal;
+  }
+
+  int x = 0;
+  if (p != 0)
+    x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+    /* { dg-warning "use of uninitialized value '\\*p" "" { target *-*-* } .-1 } */
+  free(p); /* { dg-warning "double-'free'" }  */
+}
-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-14 20:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-01 19:59 [PATCH] analyzer: call off a superseding when diagnostics are unrelated [PR110830] priour.be
2023-09-01 23:48 ` David Malcolm
2023-09-06 19:16   ` [PATCH v2] analyzer: Call " priour.be
2023-09-06 20:30     ` David Malcolm
2023-09-11  8:23     ` Andreas Schwab
2023-09-14 20:39       ` [pushed] analyzer: fix missing return in compatible_epath_p 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).