summaryrefslogtreecommitdiff
path: root/gcc/analyzer/engine.cc
diff options
context:
space:
mode:
authorDavid Malcolm <dmalcolm@redhat.com>2021-03-24 20:47:57 -0400
committerDavid Malcolm <dmalcolm@redhat.com>2021-03-24 20:47:57 -0400
commit71fc4655ab86ab66b40165b2cb49c1395ca82a9a (patch)
treecd0439f92df03d57f9d79983cdd9b76ead8f09fc /gcc/analyzer/engine.cc
parent8bf52ffa92f7d1539cbb82fbc0e95389e084ec31 (diff)
analyzer; reset sm-state for SSA names at def-stmts [PR93695,PR99044,PR99716]
Various false positives from -fanalyzer involve SSA names in loops, where sm-state associated with an SSA name from one iteration is erroneously reused in a subsequent iteration. For example, PR analyzer/99716 describes a false "double 'fclose' of FILE 'fp'" on: for (i = 0; i < 2; ++i) { FILE *fp = fopen ("/tmp/test", "w"); fprintf (fp, "hello"); fclose (fp); } where the gimple of the loop body is: fp_7 = fopen ("/tmp/test", "w"); __builtin_fwrite ("hello", 1, 5, fp_7); fclose (fp_7); i_10 = i_1 + 1; where fp_7 transitions to "closed" at the fclose, but is not reset at the subsequent fopen, leading to the false positive when the fclose is re-reached. The fix is to reset sm-state for svalues that involve an SSA name at the SSA name's def-stmt, since the def-stmt effectively changes the meaning of those related svalues. gcc/analyzer/ChangeLog: PR analyzer/93695 PR analyzer/99044 PR analyzer/99716 * engine.cc (exploded_node::on_stmt): Clear sm-state involving an SSA name at the def-stmt of that SSA name. * program-state.cc (sm_state_map::purge_state_involving): New. * program-state.h (sm_state_map::purge_state_involving): New decl. * region-model.cc (selftest::test_involves_p): New. (selftest::analyzer_region_model_cc_tests): Call it. * svalue.cc (class involvement_visitor): New class (svalue::involves_p): New. * svalue.h (svalue::involves_p): New decl. gcc/testsuite/ChangeLog: PR analyzer/93695 PR analyzer/99044 PR analyzer/99716 * gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c: Remove xfail. * gcc.dg/analyzer/pr93695-1.c: New test. * gcc.dg/analyzer/pr99044-1.c: New test. * gcc.dg/analyzer/pr99044-2.c: New test. * gcc.dg/analyzer/pr99716-1.c: New test. * gcc.dg/analyzer/pr99716-2.c: New test. * gcc.dg/analyzer/pr99716-3.c: New test.
Diffstat (limited to 'gcc/analyzer/engine.cc')
-rw-r--r--gcc/analyzer/engine.cc25
1 files changed, 25 insertions, 0 deletions
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 5792c142847..d7866b5598b 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -1244,6 +1244,31 @@ exploded_node::on_stmt (exploded_graph &eg,
sm_state_map *new_smap = state->m_checker_states[sm_idx];
impl_sm_context sm_ctxt (eg, sm_idx, sm, this, &old_state, state,
old_smap, new_smap);
+
+ /* If we're at the def-stmt of an SSA name, then potentially purge
+ any sm-state for svalues that involve that SSA name. This avoids
+ false positives in loops, since a symbolic value referring to the
+ SSA name will be referring to the previous value of that SSA name.
+ For example, in:
+ while ((e = hashmap_iter_next(&iter))) {
+ struct oid2strbuf *e_strbuf = (struct oid2strbuf *)e;
+ free (e_strbuf->value);
+ }
+ at the def-stmt of e_8:
+ e_8 = hashmap_iter_next (&iter);
+ we should purge the "freed" state of:
+ INIT_VAL(CAST_REG(‘struct oid2strbuf’, (*INIT_VAL(e_8))).value)
+ which is the "e_strbuf->value" value from the previous iteration,
+ or we will erroneously report a double-free - the "e_8" within it
+ refers to the previous value. */
+ if (tree lhs = gimple_get_lhs (stmt))
+ if (TREE_CODE (lhs) == SSA_NAME)
+ {
+ const svalue *sval
+ = old_state.m_region_model->get_rvalue (lhs, &ctxt);
+ new_smap->purge_state_involving (sval, eg.get_ext_state ());
+ }
+
/* Allow the state_machine to handle the stmt. */
if (sm.on_stmt (&sm_ctxt, snode, stmt))
unknown_side_effects = false;