| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
gcc/analyzer/ChangeLog:
PR analyzer/108935
* infinite-recursion.cc (contains_unknown_p): New.
(sufficiently_different_region_binding_p): New function, splitting
out inner loop from...
(sufficiently_different_p): ...here. Extend detection of unknown
svalues to also include svalues that contain unknown. Treat
changes in frames below the entry to the recursion as being
sufficiently different to reject being an infinite recursion.
gcc/testsuite/ChangeLog:
PR analyzer/108935
* gcc.dg/analyzer/infinite-recursion-pr108935-1.c: New test.
* gcc.dg/analyzer/infinite-recursion-pr108935-1a.c: New test.
* gcc.dg/analyzer/infinite-recursion-pr108935-2.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
This patch updates poisoned_value_diagnostic so that, where possible,
it checks to see if the value is still poisoned along the execution
path seen during feasibility analysis, rather than just that seen
in the exploded graph.
Integration testing shows this reduction in the number of
false positives:
-Wanalyzer-use-of-uninitialized-value: 191 -> 153 (-38)
where the changes happen in:
coreutils-9.1: 34 -> 20 (-14)
qemu-7.2.0: 78 -> 54 (-24)
gcc/analyzer/ChangeLog:
PR analyzer/108664
PR analyzer/108666
PR analyzer/108725
* diagnostic-manager.cc (epath_finder::get_best_epath): Add
"target_stmt" param.
(epath_finder::explore_feasible_paths): Likewise.
(epath_finder::process_worklist_item): Likewise.
(saved_diagnostic::calc_best_epath): Pass m_stmt to
epath_finder::get_best_epath.
* engine.cc (feasibility_state::maybe_update_for_edge): Move
per-stmt logic to...
(feasibility_state::update_for_stmt): ...this new function.
* exploded-graph.h (feasibility_state::update_for_stmt): New decl.
* feasible-graph.cc (feasible_node::get_state_at_stmt): New.
* feasible-graph.h: Include "analyzer/exploded-graph.h".
(feasible_node::get_state_at_stmt): New decl.
* infinite-recursion.cc
(infinite_recursion_diagnostic::check_valid_fpath_p): Update for
vfunc signature change.
* pending-diagnostic.h (pending_diagnostic::check_valid_fpath_p):
Convert first param to a reference. Add stmt param.
* region-model.cc: Include "analyzer/feasible-graph.h".
(poisoned_value_diagnostic::poisoned_value_diagnostic): Add
"check_expr" param.
(poisoned_value_diagnostic::check_valid_fpath_p): New.
(poisoned_value_diagnostic::m_check_expr): New field.
(region_model::check_for_poison): Attempt to supply a check_expr
to the diagnostic
(region_model::deref_rvalue): Add NULL for new check_expr param
of poisoned_value_diagnostic.
(region_model::get_or_create_region_for_heap_alloc): Don't reuse
regions that are marked as TOUCHED.
gcc/testsuite/ChangeLog:
PR analyzer/108664
PR analyzer/108666
PR analyzer/108725
* gcc.dg/analyzer/coreutils-cksum-pr108664.c: New test.
* gcc.dg/analyzer/coreutils-sum-pr108666.c: New test.
* gcc.dg/analyzer/torture/uninit-pr108725.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
Reject -Wanalyzer-infinite-recursion diagnostics in which control flow
has been affected by conjured_svalues between the initial call to a
function and the subsequent entry to that function. This prevents false
positives such as in qemu's recursive JSON parser where function calls are
changing state in the rest of the program (e.g. consuming tokens), despite
the modelled state being effectively identical at both nested entrypoints.
gcc/analyzer/ChangeLog:
PR analyzer/108524
* analyzer.h (class feasible_node): New forward decl.
* diagnostic-manager.cc (epath_finder::get_best_epath): Add "pd"
param.
(epath_finder::explore_feasible_paths): Likewise.
(epath_finder::process_worklist_item): Likewise. Use it to call
pending_diagnostic::check_valid_fpath_p on the final fpath to
give pending_diagnostic a way to add additional restrictions on
feasibility.
(saved_diagnostic::calc_best_epath): Pass pending_diagnostic to
epath_finder::get_best_epath.
* infinite-recursion.cc: Include "analyzer/feasible-graph.h".
(infinite_recursion_diagnostic::check_valid_fpath_p): New.
(infinite_recursion_diagnostic::fedge_uses_conjured_svalue_p): New.
(infinite_recursion_diagnostic::expr_uses_conjured_svalue_p): New.
* pending-diagnostic.h (pending_diagnostic::check_valid_fpath_p):
New vfunc.
gcc/testsuite/ChangeLog:
PR analyzer/108524
* gcc.dg/analyzer/infinite-recursion-pr108524-1.c: New test.
* gcc.dg/analyzer/infinite-recursion-pr108524-2.c: New test.
* gcc.dg/analyzer/infinite-recursion-pr108524-qobject-json-parser.c:
New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
My integration testing [1] of -fanalyzer in GCC 13 is showing a lot of
diagnostics from the new -Wanalyzer-deref-before-check warning on
real-world C projects, and most of these seem to be false positives.
This patch updates the warning to make it much less likely to fire:
- only intraprocedural cases are now reported
- reject cases in which there are control flow paths to the check
that didn't come through the dereference, by looking at BB dominator
information. This fixes a false positive seen in git-2.39.0's
pack-revindex.c: load_revindex_from_disk (PR analyzer/108455), in
which a shared "cleanup:" section checks "data" for NULL, and
depending on how much of the function is executed "data" might or
might not have already been dereferenced.
The counts of -Wanalyzer-deref-before-check diagnostics in [1]
before/after this patch show this improvement:
Known false positives: 6 -> 0 (-6)
Known true positives: 1 -> 1
Unclassified positives: 123 -> 63 (-60)
[1] https://github.com/davidmalcolm/gcc-analyzer-integration-tests
gcc/analyzer/ChangeLog:
PR analyzer/108455
* analyzer.h (class checker_event): New forward decl.
(class state_change_event): Indent.
(class warning_event): New forward decl.
* checker-event.cc (state_change_event::state_change_event): Add
"enode" param.
(warning_event::get_desc): Update for new param of
evdesc::final_event ctor.
* checker-event.h (state_change_event::state_change_event): Add
"enode" param.
(state_change_event::get_exploded_node): New accessor.
(state_change_event::m_enode): New field.
(warning_event::warning_event): New "enode" param.
(warning_event::get_exploded_node): New accessor.
(warning_event::m_enode): New field.
* diagnostic-manager.cc
(state_change_event_creator::on_global_state_change): Pass
src_node to state_change_event ctor.
(state_change_event_creator::on_state_change): Likewise.
(null_assignment_sm_context::set_next_state): Pass NULL for
new param of state_change_event ctor.
* infinite-recursion.cc
(infinite_recursion_diagnostic::add_final_event): Update for new
param of warning_event ctor.
* pending-diagnostic.cc (pending_diagnostic::add_final_event):
Pass enode to warning_event ctor.
* pending-diagnostic.h (evdesc::final_event): Add reference to
warning_event.
* sm-malloc.cc: Include "analyzer/checker-event.h" and
"analyzer/exploded-graph.h".
(deref_before_check::deref_before_check): Initialize new fields.
(deref_before_check::emit): Reject warnings in which we were
unable to determine the enodes of the dereference and the check.
Reject warnings interprocedural warnings. Reject warnings in which
the dereference doesn't dominate the check.
(deref_before_check::describe_state_change): Set m_deref_enode.
(deref_before_check::describe_final_event): Set m_check_enode.
(deref_before_check::m_deref_enode): New field.
(deref_before_check::m_check_enode): New field.
gcc/testsuite/ChangeLog:
PR analyzer/108455
* gcc.dg/analyzer/deref-before-check-1.c: Add test coverage
involving dominance.
* gcc.dg/analyzer/deref-before-check-pr108455-1.c: New test.
* gcc.dg/analyzer/deref-before-check-pr108455-git-pack-revindex.c:
New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
* analyzer.h (struct event_loc_info): New forward decl.
* bounds-checking.cc: Use event_loc_info throughout to bundle the
loc, fndecl, depth triples.
* call-info.cc: Likewise.
* checker-event.cc: Likewise.
* checker-event.h (struct event_loc_info): New decl. Use it
throughout to bundle the loc, fndecl, depth triples.
* checker-path.cc: Likewise.
* checker-path.h: Likewise.
* diagnostic-manager.cc: Likewise.
* engine.cc: Likewise.
* infinite-recursion.cc: Likewise.
* pending-diagnostic.cc: Likewise.
* pending-diagnostic.h: Likewise.
* region-model.cc: Likewise.
* sm-signal.cc: Likewise.
* varargs.cc: Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
This patch adds a new -Wanalyzer-infinite-recursion warning to
-fanalyzer, which complains about certain cases of infinite recursion.
Specifically, when it detects recursion during its symbolic execution
of the user's code, it compares the state of memory to that at the
previous level of recursion, and if nothing appears to have effectively
changed, it issues a warning.
Unlike the middle-end warning -Winfinite-recursion (added by Martin
Sebor in GCC 12; r12-5483-g30ba058f77eedf), the analyzer warning
complains if there exists an interprocedural path in which recursion
occurs in which memory has not changed, whereas -Winfinite-recursion
complains if *every* intraprocedural path through the function leads to
a self-call.
Hence the warnings complement each other: there's some overlap, but each
also catches issues that the other misses.
For example, the new warning complains about a guarded recursion in
which the guard is passed unchanged:
void test_guarded (int flag)
{
if (flag)
test_guarded (flag);
}
t.c: In function 'test_guarded':
t.c:4:5: warning: infinite recursion [CWE-674] [-Wanalyzer-infinite-recursion]
4 | test_guarded (flag);
| ^~~~~~~~~~~~~~~~~~~
'test_guarded': events 1-4
|
| 1 | void test_guarded (int flag)
| | ^~~~~~~~~~~~
| | |
| | (1) initial entry to 'test_guarded'
| 2 | {
| 3 | if (flag)
| | ~
| | |
| | (2) following 'true' branch (when 'flag != 0')...
| 4 | test_guarded (flag);
| | ~~~~~~~~~~~~~~~~~~~
| | |
| | (3) ...to here
| | (4) calling 'test_guarded' from 'test_guarded'
|
+--> 'test_guarded': events 5-6
|
| 1 | void test_guarded (int flag)
| | ^~~~~~~~~~~~
| | |
| | (5) recursive entry to 'test_guarded'; previously entered at (1)
| | (6) apparently infinite recursion
|
whereas the existing warning doesn't complain, since when "flag" is
false the function doesn't recurse.
The new warning doesn't trigger for e.g.:
void test_param_variant (int depth)
{
if (depth > 0)
test_param_variant (depth - 1);
}
on the grounds that "depth" is changing, and appears to be a variant
that enforces termination of the recursion.
gcc/ChangeLog:
PR analyzer/106147
* Makefile.in (ANALYZER_OBJS): Add analyzer/infinite-recursion.o.
gcc/analyzer/ChangeLog:
PR analyzer/106147
* analyzer.opt (Wanalyzer-infinite-recursion): New.
* call-string.cc (call_string::count_occurrences_of_function):
New.
* call-string.h (call_string::count_occurrences_of_function): New
decl.
* checker-path.cc (function_entry_event::function_entry_event):
New ctor.
(checker_path::add_final_event): Delete.
* checker-path.h (function_entry_event::function_entry_event): New
ctor.
(function_entry_event::get_desc): Drop "final".
(checker_path::add_final_event): Delete.
* diagnostic-manager.cc
(diagnostic_manager::emit_saved_diagnostic): Create the final
event via a new pending_diagnostic::add_final_event vfunc, rather
than checker_path::add_final_event.
(diagnostic_manager::add_events_for_eedge): Create function entry
events via a new pending_diagnostic::add_function_entry_event
vfunc.
* engine.cc (exploded_graph::process_node): When creating a new
PK_BEFORE_SUPERNODE node, call
exploded_graph::detect_infinite_recursion on it after adding the
in-edge.
* exploded-graph.h (exploded_graph::detect_infinite_recursion):
New decl.
(exploded_graph::find_previous_entry_to): New decl.
* infinite-recursion.cc: New file.
* pending-diagnostic.cc
(pending_diagnostic::add_function_entry_event): New.
(pending_diagnostic::add_final_event): New.
* pending-diagnostic.h
(pending_diagnostic::add_function_entry_event): New vfunc.
(pending_diagnostic::add_final_event): New vfunc.
gcc/ChangeLog:
PR analyzer/106147
* doc/gcc/gcc-command-options/options-that-control-static-analysis.rst:
Add -Wanalyzer-infinite-recursion.
* doc/gcc/gcc-command-options/options-to-request-or-suppress-warnings.rst
(-Winfinite-recursion): Mention -Wanalyzer-infinite-recursion.
gcc/testsuite/ChangeLog:
PR analyzer/106147
* g++.dg/analyzer/infinite-recursion-1.C: New test.
* g++.dg/analyzer/infinite-recursion-2.C: New test, copied from
g++.dg/warn/Winfinite-recursion-2.C.
* g++.dg/analyzer/infinite-recursion-3.C: New test, adapted from
g++.dg/warn/Winfinite-recursion-3.C.
* gcc.dg/analyzer/infinite-recursion-2.c: New test.
* gcc.dg/analyzer/infinite-recursion-3.c: New test.
* gcc.dg/analyzer/infinite-recursion-4-limited-buggy.c: New test.
* gcc.dg/analyzer/infinite-recursion-4-limited.c: New test.
* gcc.dg/analyzer/infinite-recursion-4-unlimited-buggy.c: New test.
* gcc.dg/analyzer/infinite-recursion-4-unlimited.c: New test.
* gcc.dg/analyzer/infinite-recursion-5.c: New test, adapted from
gcc.dg/Winfinite-recursion.c.
* gcc.dg/analyzer/infinite-recursion-alloca.c: New test.
* gcc.dg/analyzer/infinite-recursion-inlining.c: New test.
* gcc.dg/analyzer/infinite-recursion-multiline-1.c: New test.
* gcc.dg/analyzer/infinite-recursion-multiline-2.c: New test.
* gcc.dg/analyzer/infinite-recursion-variadic.c: New test.
* gcc.dg/analyzer/infinite-recursion.c: Add dg-warning directives
where infinite recursions occur.
* gcc.dg/analyzer/malloc-ipa-12.c: Likewise.
* gcc.dg/analyzer/pr105365.c: Likewise.
* gcc.dg/analyzer/pr105366.c: Likewise.
* gcc.dg/analyzer/pr97029.c: Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
