public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug analyzer/104943] New: Analyzer fails to purge state for local structs
@ 2022-03-15 22:18 dmalcolm at gcc dot gnu.org
2022-03-18 23:20 ` [Bug analyzer/104943] " cvs-commit at gcc dot gnu.org
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2022-03-15 22:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104943
Bug ID: 104943
Summary: Analyzer fails to purge state for local structs
Product: gcc
Version: 12.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: analyzer
Assignee: dmalcolm at gcc dot gnu.org
Reporter: dmalcolm at gcc dot gnu.org
Target Milestone: ---
State purging only happens for SSA names, and locals of struct type aren't SSA
names.
Given e.g.:
struct boxed {
int value;
};
extern struct boxed boxed_add (struct boxed a, struct boxed b);
extern struct boxed boxed_mul (struct boxed a, struct boxed b);
struct boxed
test (struct boxed a, struct boxed b)
{
struct boxed result = boxed_add (boxed_mul (a, a),
boxed_mul (b, b));
return result;
}
without optimization we have this gimple:
struct boxed test (struct boxed a, struct boxed b)
{
struct boxed result;
struct boxed D.1994;
struct boxed D.1993;
struct boxed D.1992;
<bb 2> :
D.1992 = boxed_mul (b, b);
D.1993 = boxed_mul (a, a);
result = boxed_add (D.1993, D.1992);
D.1994 = result;
result ={v} {CLOBBER(eol)};
<bb 3> :
<L1>:
return D.1994;
}
and this final exploded node:
EN 11:
preds: EN: 10
succs:
callstring: []
after SN: 3
rmodel:
stack depth: 1
frame (index 0): frame: ‘test’@1
clusters within frame: ‘test’@1
cluster for: <anonymous>: CONJURED(D.1992 = boxed_mul (b, b);, <anonymous>)
cluster for: <anonymous>: CONJURED(D.1993 = boxed_mul (a, a);, <anonymous>)
cluster for: <anonymous>: CONJURED(result = boxed_add (D.1993, D.1992);,
result)
cluster for: <anonymous>: CONJURED(result = boxed_add (D.1993, D.1992);,
result)
m_called_unknown_fn: TRUE
constraint_manager:
equiv classes:
constraints:
where the various <anonymous> are the values of the local temporaries of struct
type, which ought to be purged; they don't matter anymore.
Seen on linux kernel: drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c which
amongst other things has:
struct bw_fixed {
int64_t value;
};
with numerous calls to manipulate values; the states get bloated with bindings
for temporaries that persist far longer than they are needed.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Bug analyzer/104943] Analyzer fails to purge state for local structs
2022-03-15 22:18 [Bug analyzer/104943] New: Analyzer fails to purge state for local structs dmalcolm at gcc dot gnu.org
@ 2022-03-18 23:20 ` cvs-commit at gcc dot gnu.org
2022-03-18 23:21 ` cvs-commit at gcc dot gnu.org
2022-03-18 23:26 ` dmalcolm at gcc dot gnu.org
2 siblings, 0 replies; 4+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-03-18 23:20 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104943
--- Comment #1 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:
https://gcc.gnu.org/g:1c1daca1cdf7bc0156d57bb2b9083ee70c66b000
commit r12-7717-g1c1daca1cdf7bc0156d57bb2b9083ee70c66b000
Author: David Malcolm <dmalcolm@redhat.com>
Date: Thu Mar 17 18:12:46 2022 -0400
analyzer: add tests of boxed values [PR104943]
This patch adds various regression tests as preparatory work for
purging irrelevant local decls from state (PR analyzer/104943)
gcc/testsuite/ChangeLog:
PR analyzer/104943
* gcc.dg/analyzer/boxed-malloc-1-29.c: New test.
* gcc.dg/analyzer/boxed-malloc-1.c: New test.
* gcc.dg/analyzer/taint-alloc-5.c: New test.
* gcc.dg/analyzer/torture/boxed-int-1.c: New test.
* gcc.dg/analyzer/torture/boxed-ptr-1.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Bug analyzer/104943] Analyzer fails to purge state for local structs
2022-03-15 22:18 [Bug analyzer/104943] New: Analyzer fails to purge state for local structs dmalcolm at gcc dot gnu.org
2022-03-18 23:20 ` [Bug analyzer/104943] " cvs-commit at gcc dot gnu.org
@ 2022-03-18 23:21 ` cvs-commit at gcc dot gnu.org
2022-03-18 23:26 ` dmalcolm at gcc dot gnu.org
2 siblings, 0 replies; 4+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-03-18 23:21 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104943
--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:
https://gcc.gnu.org/g:faacafd2306ad7ece721a79dedbb6e44e0d65bdb
commit r12-7718-gfaacafd2306ad7ece721a79dedbb6e44e0d65bdb
Author: David Malcolm <dmalcolm@redhat.com>
Date: Tue Dec 7 19:22:47 2021 -0500
analyzer: extend state-purging to locals [PR104943]
The existing analyzer code attempts to purge the state of SSA names
where it can in order to minimize the size of program_state instances,
and to increase the chances of being able to reuse exploded_node
instances whilst exploring the user's code.
PR analyzer/104943 identifies that we fail to purge state of local
variables, based on behavior seen in PR analyzer/104954 when attempting
to profile slow performance of -fanalyzer on a particular file in the
Linux kernel, where that testcase has many temporary "boxed" values of
structs containing ints, which are never cleaned up, leading to bloat
of the program_state instances (specifically, of the store objects).
This patch generalizes the state purging from just being on SSA names
to also work on local variables. Doing so requires that we detect where
addresses to a local variable (or within them) are taken; we assume that
once a pointer has been taken, it's not longer safe to purge the value
of that decl at any successor point within the function.
Doing so speeds up the PR analyzer/104954 Linux kernel analyzer testcase
from taking 254 seconds to "just" 186 seconds (and I have a followup
patch in development that seems to further reduce this to 37 seconds).
The patch may also help with scaling up taint-detection so that it can
eventually be turned on by default, but we're not quite there (this
is PR analyzer/103533).
gcc/analyzer/ChangeLog:
PR analyzer/104943
PR analyzer/104954
PR analyzer/103533
* analyzer.h (class state_purge_per_decl): New forward decl.
* engine.cc (impl_run_checkers): Pass region_model_manager to
state_purge_map ctor.
* program-point.cc (function_point::final_stmt_p): New.
(function_point::get_next): New.
* program-point.h (function_point::final_stmt_p): New decl.
(function_point::get_next): New decl.
* program-state.cc (program_state::prune_for_point): Generalize to
purge local decls as well as SSA names.
(program_state::can_purge_base_region_p): New.
* program-state.h (program_state::can_purge_base_region_p): New
decl.
* region-model.cc (struct append_ssa_names_cb_data): Rename to...
(struct append_regions_cb_data): ...this.
(region_model::get_ssa_name_regions_for_current_frame): Rename
to...
(region_model::get_regions_for_current_frame): ...this, updating
for other renamings.
(region_model::append_ssa_names_cb): Rename to...
(region_model::append_regions_cb): ...this, and drop the
requirement
that the subregion be a SSA name.
* region-model.h (struct append_ssa_names_cb_data): Rename decl
to...
(struct append_regions_cb_data): ...this.
(region_model::get_ssa_name_regions_for_current_frame): Rename
decl to...
(region_model::get_regions_for_current_frame): ...this.
(region_model::append_ssa_names_cb): Rename decl to...
(region_model::append_regions_cb): ...this.
* state-purge.cc: Include "tristate.h", "selftest.h",
"analyzer/store.h", "analyzer/region-model.h", and
"gimple-walk.h".
(get_candidate_for_purging): New.
(class gimple_op_visitor): New.
(my_load_cb): New.
(my_store_cb): New.
(my_addr_cb): New.
(state_purge_map::state_purge_map): Add "mgr" param. Update for
renamings. Find uses of local variables.
(state_purge_map::~state_purge_map): Update for renaming of m_map
to m_ssa_map. Clean up m_decl_map.
(state_purge_map::get_or_create_data_for_decl): New.
(state_purge_per_ssa_name::state_purge_per_ssa_name): Update for
inheriting from state_purge_per_tree.
(state_purge_per_ssa_name::add_to_worklist): Likewise.
(state_purge_per_decl::state_purge_per_decl): New.
(state_purge_per_decl::add_needed_at): New.
(state_purge_per_decl::add_pointed_to_at): New.
(state_purge_per_decl::process_worklists): New.
(state_purge_per_decl::add_to_worklist): New.
(same_binding_p): New.
(fully_overwrites_p): New.
(state_purge_per_decl::process_point_backwards): New.
(state_purge_per_decl::process_point_forwards): New.
(state_purge_per_decl::needed_at_point_p): New.
(state_purge_annotator::print_needed): Generalize to print local
decls as well as SSA names.
* state-purge.h (class state_purge_map): Update leading comment.
(state_purge_map::map_t): Rename to...
(state_purge_map::ssa_map_t): ...this.
(state_purge_map::iterator): Rename to...
(state_purge_map::ssa_iterator): ...this.
(state_purge_map::decl_map_t): New typedef.
(state_purge_map::decl_iterator): New typedef.
(state_purge_map::state_purge_map): Add "mgr" param.
(state_purge_map::get_data_for_ssa_name): Update for renaming.
(state_purge_map::get_any_data_for_decl): New.
(state_purge_map::get_or_create_data_for_decl): New decl.
(state_purge_map::begin): Rename to...
(state_purge_map::begin_ssas): ...this.
(state_purge_map::end): Rename to...
(state_purge_map::end_ssa): ...this.
(state_purge_map::begin_decls): New.
(state_purge_map::end_decls): New.
(state_purge_map::m_map): Rename to...
(state_purge_map::m_ssa_map): ...this.
(state_purge_map::m_decl_map): New field.
(class state_purge_per_tree): New class.
(class state_purge_per_ssa_name): Inherit from
state_purge_per_tree.
(state_purge_per_ssa_name::get_function): Move to base class.
(state_purge_per_ssa_name::point_set_t): Likewise.
(state_purge_per_ssa_name::m_fun): Likewise.
(class state_purge_per_decl): New.
gcc/testsuite/ChangeLog:
PR analyzer/104943
PR analyzer/104954
PR analyzer/103533
* gcc.dg/analyzer/torture/boxed-ptr-1.c: Update expected number
of exploded nodes to reflect improvements in state purging.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Bug analyzer/104943] Analyzer fails to purge state for local structs
2022-03-15 22:18 [Bug analyzer/104943] New: Analyzer fails to purge state for local structs dmalcolm at gcc dot gnu.org
2022-03-18 23:20 ` [Bug analyzer/104943] " cvs-commit at gcc dot gnu.org
2022-03-18 23:21 ` cvs-commit at gcc dot gnu.org
@ 2022-03-18 23:26 ` dmalcolm at gcc dot gnu.org
2 siblings, 0 replies; 4+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2022-03-18 23:26 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104943
David Malcolm <dmalcolm at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |FIXED
--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed by the above commit.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-18 23:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-15 22:18 [Bug analyzer/104943] New: Analyzer fails to purge state for local structs dmalcolm at gcc dot gnu.org
2022-03-18 23:20 ` [Bug analyzer/104943] " cvs-commit at gcc dot gnu.org
2022-03-18 23:21 ` cvs-commit at gcc dot gnu.org
2022-03-18 23:26 ` dmalcolm at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).