[10/11,analyzer] Fix issues in diagnostic_manager::prune_path
diff mbox series

Message ID 1574284430-8776-11-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series
  • Static analysis v2
Related show

Commit Message

David Malcolm Nov. 20, 2019, 9:13 p.m. UTC
There was a bad upper limit in diagnostic_manager::prune_path's test for
pruning redundant interprocedural pass information, leading to an ICE.

Additional, the test for pruning
  [..., call, function-entry, return, ...]
triples doesn't work for -fanalyzer-verbosity=0, which doesn't generate
function-entry events, leading to cases where -fanalyzer-verbosity=0 could
be more verbose than higher limits, e.g.:

  'test_3': events 1-4
    |
    |   NN |   free (ptr);
    |      |   ^~~~~~~~~~
    |      |   |
    |      |   (1) first 'free' here
    |   NN |   called_by_test_3 ();
    |      |   ~~~~~~~~~~~~~~~~~~~
    |      |   |
    |      |   (2) calling 'called_by_test_3' from 'test_3'
    |      |   (3) returning to 'test_3' from 'called_by_test_3'
    |   NN |   free (ptr); /* { dg-warning "double-'free' of 'ptr'" } */
    |      |   ~~~~~~~~~~
    |      |   |
    |      |   (4) second 'free' here; first 'free' was at (1)
    |

where the call/return would be pruned at higher verbosities.

This patch fixes the limits, and prunes the case of a [call, return] pair.

gcc/ChangeLog:
	* analyzer/diagnostic-manager.cc (diagnostic_manager::prune_path):
	When pruning [..., call, function-entry, return, ...] triples,
	fix the bounds test and move it inside the loop.  Also test for
	[..., call, return, ...] pairs.

gcc/testsuite/ChangeLog:
	* gcc.dg/analyzer/analyzer-verbosity-0.c: Add coverage for pruning
	a call/return to an empty function.
	* gcc.dg/analyzer/analyzer-verbosity-1.c: Likewise.
	* gcc.dg/analyzer/analyzer-verbosity-2.c: Likewise.
---
 gcc/analyzer/diagnostic-manager.cc                 | 32 ++++++++++++++++++++--
 .../gcc.dg/analyzer/analyzer-verbosity-0.c         | 29 ++++++++++++++++++++
 .../gcc.dg/analyzer/analyzer-verbosity-1.c         | 30 ++++++++++++++++++++
 .../gcc.dg/analyzer/analyzer-verbosity-2.c         | 30 ++++++++++++++++++++
 4 files changed, 118 insertions(+), 3 deletions(-)

Patch
diff mbox series

diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 926900b..8cd4507 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -1076,10 +1076,12 @@  diagnostic_manager::prune_path (checker_path *path,
   do
     {
       changed = false;
-      int idx = path->m_events.length () - 3;
-      while (idx >= 0 && idx < (signed)path->m_events.length ())
+      int idx = path->m_events.length () - 1;
+      while (idx >= 0)
 	{
-	  if (path->m_events[idx]->is_call_p ()
+	  /* Prune [..., call, function-entry, return, ...] triples.  */
+	  if (idx + 2 < (signed)path->m_events.length ()
+	      && path->m_events[idx]->is_call_p ()
 	      && path->m_events[idx + 1]->is_function_entry_p ()
 	      && path->m_events[idx + 2]->is_return_p ())
 	    {
@@ -1095,7 +1097,31 @@  diagnostic_manager::prune_path (checker_path *path,
 	      path->delete_event (idx + 1);
 	      path->delete_event (idx);
 	      changed = true;
+	      idx--;
+	      continue;
 	    }
+
+	  /* Prune [..., call, return, ...] pairs
+	     (for -fanalyzer-verbosity=0).  */
+	  if (idx + 1 < (signed)path->m_events.length ()
+	      && path->m_events[idx]->is_call_p ()
+	      && path->m_events[idx + 1]->is_return_p ())
+	    {
+	      if (get_logger ())
+		{
+		  label_text desc (path->m_events[idx]->get_desc (false));
+		  log ("filtering events %i-%i:"
+		       " irrelevant call/return: %s",
+		       idx, idx + 1, desc.m_buffer);
+		  desc.maybe_free ();
+		}
+	      path->delete_event (idx + 1);
+	      path->delete_event (idx);
+	      changed = true;
+	      idx--;
+	      continue;
+	    }
+
 	  idx--;
 	}
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-0.c b/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-0.c
index 98200b3..1103cc6 100644
--- a/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-0.c
+++ b/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-0.c
@@ -131,3 +131,32 @@  void test_2 (void *ptr, int a, int b)
 
 // TODO: range cases
 
+/* The call/return to this function shouldn't appear in the path.  */
+
+void called_by_test_3 (void)
+{
+}
+
+void test_3 (void *ptr)
+{
+  free (ptr);
+  called_by_test_3 ();
+  free (ptr); /* { dg-warning "double-'free' of 'ptr'" } */
+}
+
+/* { dg-begin-multiline-output "" }
+   NN |   free (ptr);
+      |   ^~~~~~~~~~
+  'test_3': events 1-2
+    |
+    |   NN |   free (ptr);
+    |      |   ^~~~~~~~~~
+    |      |   |
+    |      |   (1) first 'free' here
+    |   NN |   called_by_test_3 ();
+    |   NN |   free (ptr);
+    |      |   ~~~~~~~~~~
+    |      |   |
+    |      |   (2) second 'free' here; first 'free' was at (1)
+    |
+  { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-1.c b/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-1.c
index 7e437bb..80039d5 100644
--- a/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-1.c
@@ -158,3 +158,33 @@  void test_2 (void *ptr, int a, int b)
            |      |   (8) second 'free' here; first 'free' was at (4)
            |
   { dg-end-multiline-output "" } */
+
+/* The call/return to this function shouldn't appear in the path.  */
+
+void called_by_test_3 (void)
+{
+}
+
+void test_3 (void *ptr)
+{
+  free (ptr);
+  called_by_test_3 ();
+  free (ptr); /* { dg-warning "double-'free' of 'ptr'" } */
+}
+
+/* { dg-begin-multiline-output "" }
+   NN |   free (ptr);
+      |   ^~~~~~~~~~
+  'test_3': events 1-2
+    |
+    |   NN |   free (ptr);
+    |      |   ^~~~~~~~~~
+    |      |   |
+    |      |   (1) first 'free' here
+    |   NN |   called_by_test_3 ();
+    |   NN |   free (ptr);
+    |      |   ~~~~~~~~~~
+    |      |   |
+    |      |   (2) second 'free' here; first 'free' was at (1)
+    |
+  { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-2.c b/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-2.c
index 2c9e5da..9b87d43 100644
--- a/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/analyzer-verbosity-2.c
@@ -189,3 +189,33 @@  void test_2 (void *ptr, int a, int b)
   { dg-end-multiline-output "" } */
 
 // TODO: range cases
+
+/* The call/return to this function shouldn't appear in the path.  */
+
+void called_by_test_3 (void)
+{
+}
+
+void test_3 (void *ptr)
+{
+  free (ptr);
+  called_by_test_3 ();
+  free (ptr); /* { dg-warning "double-'free' of 'ptr'" } */
+}
+
+/* { dg-begin-multiline-output "" }
+   NN |   free (ptr);
+      |   ^~~~~~~~~~
+  'test_3': events 1-2
+    |
+    |   NN |   free (ptr);
+    |      |   ^~~~~~~~~~
+    |      |   |
+    |      |   (1) first 'free' here
+    |   NN |   called_by_test_3 ();
+    |   NN |   free (ptr);
+    |      |   ~~~~~~~~~~
+    |      |   |
+    |      |   (2) second 'free' here; first 'free' was at (1)
+    |
+  { dg-end-multiline-output "" } */