analyzer: fix tests for UNKNOWN_LOCATION

In the reproducer for PR analyzer/58237 I noticed that some events were
missing locations (and text); for example event 3 here:

    |   15 |   while (fgets(buf, 10, fp) != NULL)
    |      |         ~
    |      |         |
    |      |         (2) following 'false' branch...
    |
  'f1': event 3
    |
    |cc1:
    |
  'f1': event 4
    |
    |<source>:19:1:
    |   19 | }
    |      | ^
    |      | |
    |      | (4) 'fp' leaks here; was opened at (1)
    |

The root cause is that various places in the analyzer compare locations
against UNKNOWN_LOCATION, which fails to detect an unknown location for
the case where an unknown_location has been wrapped into an ad-hoc
location to record a block.

This patch fixes the issue by using get_pure_location whenever testing
against UNKNOWN_LOCATION to look through ad-hoc wrappers.

For the case above, it thus picks a better location in
supernode::get_start_location for event (3) above, improving it to:

    |   15 |   while (fgets(buf, 10, fp) != NULL)
    |      |         ~
    |      |         |
    |      |         (2) following 'false' branch...
    |......
    |   19 | }
    |      | ~
    |      | |
    |      | (3) ...to here
    |      | (4) 'fp' leaks here; was opened at (1)
    |

gcc/analyzer/ChangeLog:
	PR analyzer/58237
	* engine.cc (leak_stmt_finder::find_stmt): Use get_pure_location
	when comparing against UNKNOWN_LOCATION.
	(stmt_requires_new_enode_p): Likewise.
	(exploded_graph::dump_exploded_nodes): Likewise.
	* supergraph.cc (supernode::get_start_location): Likewise.
	(supernode::get_end_location): Likewise.

gcc/testsuite/ChangeLog:
	PR analyzer/58237
	* gcc.dg/analyzer/file-paths-1.c: New test.

diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog
index aad8528d86d47a2a36ed309ef606ae00509164b6..2f91cd116e22b2d9f6e46dd949441bc5415da70b 100644 (file)
--- a/gcc/analyzer/ChangeLog
+++ b/gcc/analyzer/ChangeLog
@@ -1,3 +1,13 @@
+2020-01-14  David Malcolm  <dmalcolm@redhat.com>
+
+       PR analyzer/58237
+       * engine.cc (leak_stmt_finder::find_stmt): Use get_pure_location
+       when comparing against UNKNOWN_LOCATION.
+       (stmt_requires_new_enode_p): Likewise.
+       (exploded_graph::dump_exploded_nodes): Likewise.
+       * supergraph.cc (supernode::get_start_location): Likewise.
+       (supernode::get_end_location): Likewise.
+
 2020-01-14  David Malcolm  <dmalcolm@redhat.com>
 
        PR analyzer/58237

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 573f9f6e35bd7d2f5c80c29d85758c9a1d716c92..9092024d3a4009ae3ec7e43b07d0c67f492a1e9c 100644 (file)
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -417,7 +417,7 @@ public:
        const program_point &dst_point = dst_node->get_point ();
        const gimple *stmt = dst_point.get_stmt ();
        if (stmt)
-         if (stmt->location != UNKNOWN_LOCATION)
+         if (get_pure_location (stmt->location) != UNKNOWN_LOCATION)
            return stmt;
       }
 
@@ -2300,8 +2300,8 @@ stmt_requires_new_enode_p (const gimple *stmt,
      could be consolidated into PREV_STMT, giving us an event with
      no location.  Ensure that STMT gets its own exploded_node to
      avoid this.  */
-  if (prev_stmt->location == UNKNOWN_LOCATION
-      && stmt->location != UNKNOWN_LOCATION)
+  if (get_pure_location (prev_stmt->location) == UNKNOWN_LOCATION
+      && get_pure_location (stmt->location) != UNKNOWN_LOCATION)
     return true;
 
   return false;
@@ -3098,7 +3098,7 @@ exploded_graph::dump_exploded_nodes () const
        {
          if (const gimple *stmt = enode->get_stmt ())
            {
-             if (richloc.get_loc () == UNKNOWN_LOCATION)
+             if (get_pure_location (richloc.get_loc ()) == UNKNOWN_LOCATION)
                richloc.set_range (0, stmt->location, SHOW_RANGE_WITH_CARET);
              else
                richloc.add_range (stmt->location,

diff --git a/gcc/analyzer/supergraph.cc b/gcc/analyzer/supergraph.cc
index 24f83c9b719b5d96c74c8e4f7c40131a0c95896d..fd905ea05f4b50347c13f4efcab362a7dbf0e0b2 100644 (file)
--- a/gcc/analyzer/supergraph.cc
+++ b/gcc/analyzer/supergraph.cc
@@ -531,13 +531,14 @@ supernode::dump_dot_id (pretty_printer *pp) const
 location_t
 supernode::get_start_location () const
 {
-  if (m_returning_call && m_returning_call->location != UNKNOWN_LOCATION)
+  if (m_returning_call
+      && get_pure_location (m_returning_call->location) != UNKNOWN_LOCATION)
     return m_returning_call->location;
 
   int i;
   gimple *stmt;
   FOR_EACH_VEC_ELT (m_stmts, i, stmt)
-    if (stmt->location != UNKNOWN_LOCATION)
+    if (get_pure_location (stmt->location) != UNKNOWN_LOCATION)
       return stmt->location;
 
   if (entry_p ())
@@ -561,10 +562,11 @@ supernode::get_end_location () const
   int i;
   gimple *stmt;
   FOR_EACH_VEC_ELT_REVERSE (m_stmts, i, stmt)
-    if (stmt->location != UNKNOWN_LOCATION)
+    if (get_pure_location (stmt->location) != UNKNOWN_LOCATION)
       return stmt->location;
 
-  if (m_returning_call && m_returning_call->location != UNKNOWN_LOCATION)
+  if (m_returning_call
+      && get_pure_location (m_returning_call->location) != UNKNOWN_LOCATION)
     return m_returning_call->location;
 
   if (entry_p ())

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 18c6a888de694c9c66c374c3321f26339ef95a55..ac0cd2dd67f8f1e926f4a8f3d8b44b2a37089443 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-14  David Malcolm  <dmalcolm@redhat.com>
+
+       PR analyzer/58237
+       * gcc.dg/analyzer/file-paths-1.c: New test.
+
 2020-01-14  David Malcolm  <dmalcolm@redhat.com>
 
        PR analyzer/58237

diff --git a/gcc/testsuite/gcc.dg/analyzer/file-paths-1.c b/gcc/testsuite/gcc.dg/analyzer/file-paths-1.c
new file mode 100644 (file)
index 0000000..1c8bf61
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/file-paths-1.c
@@ -0,0 +1,25 @@
+#include <stdio.h>
+
+/* Verify that we correctly emit CFG events in the face of buffers
+   being clobbered in these leak reports.  */
+
+void f1 (const char *str)
+{
+  FILE * fp = fopen(str, "r"); /* { dg-message "opened here" } */
+  char buf[10];
+
+  while (fgets(buf, 10, fp) != NULL) /* { dg-message "following 'false' branch\\.\\.\\." } */
+    {
+    }
+} /* { dg-warning "leak of FILE 'fp'" } */
+/* { dg-message "\\.\\.\\.to here" "" { target *-*-* } .-1 } */
+
+void f2(const char *str, int flag)
+{
+  FILE * fp = fopen(str, "r"); /* { dg-message "opened here" } */
+  char buf[10];
+
+  if (flag) /* { dg-message "when 'flag == 0'" } */
+    fclose(fp);
+} /* { dg-warning "leak of FILE 'fp'" } */
+/* { dg-message "\\.\\.\\.to here" "" { target *-*-* } .-1 } */