aboutsummaryrefslogtreecommitdiff
path: root/gcc/analyzer/program-state.cc
diff options
context:
space:
mode:
authorDavid Malcolm <dmalcolm@redhat.com>2021-11-18 15:23:30 -0500
committerDavid Malcolm <dmalcolm@redhat.com>2021-11-19 15:25:27 -0500
commitf573d35147ca8433c102e1721d8c99fc432cb44b (patch)
tree4001b588d5e8bbcf86baaf55157b805326e9131f /gcc/analyzer/program-state.cc
parentbe08d573177b2004706759eedfdd4113f69e4c5c (diff)
downloadgcc-f573d35147ca8433c102e1721d8c99fc432cb44b.zip
gcc-f573d35147ca8433c102e1721d8c99fc432cb44b.tar.gz
gcc-f573d35147ca8433c102e1721d8c99fc432cb44b.tar.bz2
analyzer: fix false leak due to overeager state merging [PR103217]
PR analyzer/103217 reports a false positive from -Wanalyzer-malloc-leak. The root cause is due to overzealous state merger, where the state-merging code decided to merge these two states by merging the stores: state A: clusters within frame: ‘main’@1 cluster for: one_3: CONJURED(val_4 = strdup (src_2(D));, val_4) cluster for: two_4: UNKNOWN(char *) cluster for: one_21: CONJURED(val_4 = strdup (src_2(D));, val_4) state B: clusters within frame: ‘main’@1 cluster for: one_3: UNKNOWN(char *) cluster for: two_4: CONJURED(val_4 = strdup (src_2(D));, val_4) cluster for: two_18: CONJURED(val_4 = strdup (src_2(D));, val_4) into: clusters within frame: ‘main’@1 cluster for: one_3: UNKNOWN(char *) cluster for: two_4: UNKNOWN(char *) cluster for: one_21: UNKNOWN(char *) cluster for: two_18: UNKNOWN(char *) despite "CONJURED(val_4 = strdup (src_2(D));, val_4)" having sm-state, in this case malloc:nonnull ({free}), thus leading to both references to the conjured svalue being lost at merger. This patch tweaks the state merger code so that it will not consider merging two different svalues for the value of a region if either svalue has non-purgable sm-state (in the above example, malloc:nonnull). This fixes the false leak report above. Doing so uncovered an issue with explode-2a.c in which the warnings moved from the correct location to the "while" stmt. This turned out to be a missing call to detect_leaks in phi-handling, which the patch also fixes (in the PK_BEFORE_SUPERNODE case in exploded_graph::process_node). Doing this fixed the regression in explode-2a.c and also fixed the location of the leak warning in explode-1.c. The other side effect of the change is that pr94858-1.c now emits a -Wanalyzer-too-complex warning, since pertinent state is no longer being thrown away. There doesn't seem to be a good way of avoiding this, so the patch also adds -Wno-analyzer-too-complex to that test case (restoring the default). gcc/analyzer/ChangeLog: PR analyzer/103217 * engine.cc (exploded_graph::get_or_create_node): Pass in m_ext_state to program_state::can_merge_with_p. (exploded_graph::process_worklist): Likewise. (exploded_graph::maybe_process_run_of_before_supernode_enodes): Likewise. (exploded_graph::process_node): Add missing call to detect_leaks when handling phi nodes. * program-state.cc (program_state::can_merge_with_p): Add "ext_state" param. Pass it and state ptrs to region_model::can_merge_with_p. (selftest::test_program_state_merging): Update for new ext_state param of program_state::can_merge_with_p. (selftest::test_program_state_merging_2): Likewise. * program-state.h (program_state::can_purge_p): Make const. (program_state::can_merge_with_p): Add "ext_state" param. * region-model.cc: Include "analyzer/program-state.h". (region_model::can_merge_with_p): Add params "ext_state", "state_a", and "state_b", use them when creating model_merger object. (model_merger::mergeable_svalue_p): New. * region-model.h (region_model::can_merge_with_p): Add params "ext_state", "state_a", and "state_b". (model_merger::model_merger) Likewise, initializing new fields. (model_merger::mergeable_svalue_p): New decl. (model_merger::m_ext_state): New field. (model_merger::m_state_a): New field. (model_merger::m_state_b): New field. * svalue.cc (svalue::can_merge_p): Call model_merger::mergeable_svalue_p on both states and reject the merger accordingly. gcc/testsuite/ChangeLog: PR analyzer/103217 * gcc.dg/analyzer/explode-1.c: Update for improvement to location of leak warning. * gcc.dg/analyzer/pr103217.c: New test. * gcc.dg/analyzer/pr94858-1.c: Add -Wno-analyzer-too-complex. Signed-off-by: David Malcolm <dmalcolm@redhat.com>
Diffstat (limited to 'gcc/analyzer/program-state.cc')
-rw-r--r--gcc/analyzer/program-state.cc9
1 files changed, 6 insertions, 3 deletions
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index 1c87af0..47e4eca 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -1197,6 +1197,7 @@ program_state::get_representative_tree (const svalue *sval) const
bool
program_state::can_merge_with_p (const program_state &other,
+ const extrinsic_state &ext_state,
const program_point &point,
program_state *out) const
{
@@ -1213,7 +1214,9 @@ program_state::can_merge_with_p (const program_state &other,
/* Attempt to merge the region_models. */
if (!m_region_model->can_merge_with_p (*other.m_region_model,
point,
- out->m_region_model))
+ out->m_region_model,
+ &ext_state,
+ this, &other))
return false;
/* Copy m_checker_states to OUT. */
@@ -1645,7 +1648,7 @@ test_program_state_merging ()
with the given sm-state.
They ought to be mergeable, preserving the sm-state. */
program_state merged (ext_state);
- ASSERT_TRUE (s0.can_merge_with_p (s1, point, &merged));
+ ASSERT_TRUE (s0.can_merge_with_p (s1, ext_state, point, &merged));
merged.validate (ext_state);
/* Verify that the merged state has the sm-state for "p". */
@@ -1703,7 +1706,7 @@ test_program_state_merging_2 ()
/* They ought to not be mergeable. */
program_state merged (ext_state);
- ASSERT_FALSE (s0.can_merge_with_p (s1, point, &merged));
+ ASSERT_FALSE (s0.can_merge_with_p (s1, ext_state, point, &merged));
}
/* Run all of the selftests within this file. */