From 8397af8ed0db685312e44117fd52316b57c2c129 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Fri, 20 Dec 2019 10:56:28 -0500 Subject: [PATCH] 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 | |: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. --- gcc/analyzer/ChangeLog | 10 ++++++++ gcc/analyzer/engine.cc | 8 +++---- gcc/analyzer/supergraph.cc | 10 ++++---- gcc/testsuite/ChangeLog | 5 ++++ gcc/testsuite/gcc.dg/analyzer/file-paths-1.c | 25 ++++++++++++++++++++ 5 files changed, 50 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/file-paths-1.c diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog index aad8528d86d..2f91cd116e2 100644 --- a/gcc/analyzer/ChangeLog +++ b/gcc/analyzer/ChangeLog @@ -1,3 +1,13 @@ +2020-01-14 David Malcolm + + 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 PR analyzer/58237 diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 573f9f6e35b..9092024d3a4 100644 --- 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 24f83c9b719..fd905ea05f4 100644 --- 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 18c6a888de6..ac0cd2dd67f 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-01-14 David Malcolm + + PR analyzer/58237 + * gcc.dg/analyzer/file-paths-1.c: New test. + 2020-01-14 David Malcolm 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 index 00000000000..1c8bf61dd3f --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/file-paths-1.c @@ -0,0 +1,25 @@ +#include + +/* 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 } */ -- 2.30.2