| Message ID | CAKiQ0GFBmnmg1xSa0FEU-nENjdCZVCS28-3rVn2r1qCXxyyeag@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [WIP,RFC] analyzer: Add optional trim of the analyzer diagnostics going too deep [PR110543] | expand |
On Fri, 2023-07-21 at 17:35 +0200, Benjamin Priour wrote: > Hi, > > Upon David's request I've joined the in progress patch to the below > email. > I hope it makes more sense now. > > Best, > Benjamin. Thanks for posting the work-in-progress patch; it makes the idea clearer. Some thoughts about this: - I like the idea of defaulting to *not* showing events within system headers, which the patch achieves - I don't like the combination of never/system with maxdepth, in that it seems complicated and I don't think a user is likely to experiment with different depths. - Hence I think it would work better as a simple boolean, perhaps "-fanalyzer-show-events-in-system-headers" or somesuch? It seems like the sort of thing that we want to provide a sensible default for, but have the option of turning off for debugging the analyzer itself, but I don't expect an end-user to touch that option. FWIW the patch seems to have been mangled somewhat via email, so I don't have a sense of what the actual output from patched analyzer looks like. What should we output to the user with -fanalyzer and no other options for the case in PR 110543? Currently, for https://godbolt.org/z/sb9dM9Gqa trunk emits 12 events, of which probably only this last one is useful: (12) dereference of NULL 'a.std::__shared_ptr_access<A, __gnu_cxx::_S_atomic, false, false>::operator->()' What does the output look like with your patch? Thanks Dave > > ---------- Forwarded message --------- > From: Benjamin Priour <priour.be@gmail.com> > Date: Tue, Jul 18, 2023 at 3:30 PM > Subject: [RFC] analyzer: Add optional trim of the analyzer > diagnostics > going too deep [PR110543] > To: <gcc-patches@gcc.gnu.org>, David Malcolm <dmalcolm@redhat.com> > > > Hi, > > I'd like to request comments on a patch I am writing for PR110543. > The goal of this patch is to reduce the noise of the analyzer emitted > diagnostics when dealing with > system headers, or simply diagnostic paths that are too long. The new > option only affects the display > of the diagnostics, but doesn't hinder the actual analysis. > > I've defaulted the new option to "system", thus preventing the > diagnostic > paths from showing system headers. > "never" corresponds to the pre-patch behavior, whereas you can also > specify > an unsigned value <maxdepth> > that prevents paths to go deeper than <maxdepth> frames. > > fanalyzer-trim-diagnostics= > > Common Joined RejectNegative ToLower > > Var(flag_analyzer_trim_diagnostics) > > Init("system") > > -fanalyzer-trim-diagnostics=[never|system|<maxdepth>] Trim > > diagnostics > > path that are too long before emission. > > > > Does it sounds reasonable and user-friendly ? > > Regstrapping was a success against trunk, although one of the newly > added > test case fails for c++14. > Note that the test case below was done with "never", thus behaves > exactly > as the pre-patch analyzer > on x86_64-linux-gnu. > > /* { dg-additional-options "-fdiagnostics-plain-output > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > diagnostics=never" > > } */ > > /* { dg-skip-if "" { c++98_only } } */ > > > > #include <memory> > > struct A {int x; int y;}; > > > > int main () { > > std::shared_ptr<A> a; > > a->x = 4; /* { dg-line deref_a } */ > > /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > > } */ > > > > return 0; > > } > > > > /* { dg-begin-multiline-output "" } > > 'int main()': events 1-2 > > | > > | > > +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::operator->() const [with _Tp = A; > > __gnu_cxx::_Lock_policy > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > <anonymous> = > > false]': events 3-4 > > | > > | > > +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool > > <anonymous> = > > false; bool <anonymous> = false]': events 5-6 > > | > > | > > +--> 'std::__shared_ptr<_Tp, _Lp>::element_type* > > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 > > | > > | > > <------+ > > | > > 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool > > <anonymous> = > > false; bool <anonymous> = false]': event 9 > > | > > | > > <------+ > > | > > 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::operator->() const [with _Tp = A; > > __gnu_cxx::_Lock_policy > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > <anonymous> = > > false]': event 10 > > | > > | > > <------+ > > | > > 'int main()': events 11-12 > > | > > | > > { dg-end-multiline-output "" } */ > > > > > The first events "'int main()': events 1-2" vary in c++14 (get events > 1-3). > > > > > // c++14 with fully detailed output > > ‘int main()’: events 1-3 > > | > > | 8 | int main () { > > | | ^~~~ > > | | | > > | | (1) entry to ‘main’ > > | 9 | std::shared_ptr<A> a; > > | | ~ > > | | | > > | | (2) > > ‘a.std::shared_ptr<A>::<anonymous>.std::__shared_ptr<A, > > __gnu_cxx::_S_atomic>::_M_ptr’ is NULL > > | 10 | a->x = 4; /* { dg-line deref_a } */ > > | | ~~ > > | | | > > | | (3) calling ‘std::__shared_ptr_access<A, > > __gnu_cxx::_S_atomic, false, false>::operator->’ from ‘main’ > > > > whereas c++17 and posterior give > > > // c++17 with fully detailed output > > > // ./xg++ -fanalyzer > > ../../gcc/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > > diagnostics-never.C > > -B. -shared-libgcc -fanalyzer-trim-diagnostics=never -std=c++17 > > > ‘int main()’: events 1-2 > > | > > | 8 | int main () { > > | | ^~~~ > > | | | > > | | (1) entry to ‘main’ > > | 9 | std::shared_ptr<A> a; > > | 10 | a->x = 4; /* { dg-line deref_a } */ > > | | ~~ > > | | | > > | | (2) calling ‘std::__shared_ptr_access<A, > > __gnu_cxx::_S_atomic, false, false>::operator->’ from ‘main’ > > > > Is there a way to make dg-multiline-output check for a regex ? Or > would > checking the multiline-output only for c++17 and c++20 be acceptable > ? > This divergence results from two slightly different IPA: > > // c++14 -fdump-ipa-analyzer > > // std::shared_ptr<A> a; becomes > > a.D.29392._M_ptr = 0B; > > a.D.29392._M_refcount._M_pi = 0B; > > > > whereas in c++17 > > > // c++17 -fdump-ipa-analyzer > > // std::shared_ptr<A> a; becomes > > a = {}; > > > > I know shared_ptr limited support is a coincidence more than a > feature, but > maybe there is still a way to make the above test case work ? > Otherwise, I'd fallback to a supported system function. If you have > any > that you fancy, do tell me. > > Thanks, > Benjamin. > > --- > gcc/analyzer/analyzer.cc | 2 +- > gcc/analyzer/analyzer.h | 1 + > gcc/analyzer/analyzer.opt | 4 + > gcc/analyzer/diagnostic-manager.cc | 132 > ++++++++++++++++++ > gcc/analyzer/diagnostic-manager.h | 7 + > .../analyzer/fanalyzer-trim-diagnostics-0.C | 19 +++ > .../analyzer/fanalyzer-trim-diagnostics-1.C | 28 ++++ > .../analyzer/fanalyzer-trim-diagnostics-2.C | 44 ++++++ > .../fanalyzer-trim-diagnostics-default.C | 21 +++ > .../fanalyzer-trim-diagnostics-never.C | 44 ++++++ > .../fanalyzer-trim-diagnostics-system.C | 19 +++ > 11 files changed, 320 insertions(+), 1 deletion(-) > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C > > diff --git a/gcc/analyzer/analyzer.cc b/gcc/analyzer/analyzer.cc > index 5091fb7a583..b27d8e359db 100644 > --- a/gcc/analyzer/analyzer.cc > +++ b/gcc/analyzer/analyzer.cc > @@ -274,7 +274,7 @@ is_named_call_p (const_tree fndecl, const char > *funcname) > Compare with cp/typeck.cc: decl_in_std_namespace_p, but this > doesn't > rely on being the C++ FE (or handle inline namespaces inside of > std). > */ > > -static inline bool > +bool > is_std_function_p (const_tree fndecl) > { > tree name_decl = DECL_NAME (fndecl); > diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h > index 579517c23e6..31597079153 100644 > --- a/gcc/analyzer/analyzer.h > +++ b/gcc/analyzer/analyzer.h > @@ -386,6 +386,7 @@ extern bool is_special_named_call_p (const gcall > *call, > const char *funcname, > extern bool is_named_call_p (const_tree fndecl, const char > *funcname); > extern bool is_named_call_p (const_tree fndecl, const char > *funcname, > const gcall *call, unsigned int num_args); > +extern bool is_std_function_p (const_tree fndecl); > extern bool is_std_named_call_p (const_tree fndecl, const char > *funcname); > extern bool is_std_named_call_p (const_tree fndecl, const char > *funcname, > const gcall *call, unsigned int num_args); > diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt > index 2760aaa8151..78ff2c07483 100644 > --- a/gcc/analyzer/analyzer.opt > +++ b/gcc/analyzer/analyzer.opt > @@ -362,4 +362,8 @@ fdump-analyzer-untracked > Common RejectNegative Var(flag_dump_analyzer_untracked) > Emit custom warnings with internal details intended for analyzer > developers. > > +fanalyzer-trim-diagnostics= > +Common Joined RejectNegative ToLower > Var(flag_analyzer_trim_diagnostics) > Init("system") > +-fanalyzer-trim-diagnostics=[never|system|<maxdepth>] Trim > diagnostics > path that are too long before emission. > + > ; This comment is to ensure we retain the blank line above. > diff --git a/gcc/analyzer/diagnostic-manager.cc > b/gcc/analyzer/diagnostic-manager.cc > index cfca305d552..d946ad5bac7 100644 > --- a/gcc/analyzer/diagnostic-manager.cc > +++ b/gcc/analyzer/diagnostic-manager.cc > @@ -20,9 +20,11 @@ along with GCC; see the file COPYING3. If not see > > #include "config.h" > #define INCLUDE_MEMORY > +#define INCLUDE_VECTOR > #include "system.h" > #include "coretypes.h" > #include "tree.h" > +#include "input.h" > #include "pretty-print.h" > #include "gcc-rich-location.h" > #include "gimple-pretty-print.h" > @@ -60,6 +62,37 @@ along with GCC; see the file COPYING3. If not see > > #if ENABLE_ANALYZER > > +/* Return positive value of fanalyzer-trim-diagnostics if a depth > was given > + or a negative value otherwise. */ > + > +long trim_diagnostics_depth () > +{ > + if (trim_diagnostics_system_p () || trim_diagnostics_never_p ()) > + return -1; > + > + char *end = NULL; > + long depth = strtol (flag_analyzer_trim_diagnostics, &end, 10); > + gcc_assert (end == NULL || *end == '\0'); > + gcc_assert (!(end == flag_analyzer_trim_diagnostics && depth == > 0)); > + return depth; > +} > + > +/* Return true if fanalyzer-trim-diagnostics is set to "system". */ > + > +bool trim_diagnostics_system_p () > +{ > + int len = strlen (flag_analyzer_trim_diagnostics); > + return len == 6 && strcmp (flag_analyzer_trim_diagnostics, > "system") == > 0; > +} > + > +/* Return true if fanalyzer-trim-diagnostics is set to "never". */ > + > +bool trim_diagnostics_never_p () > +{ > + int len = strlen (flag_analyzer_trim_diagnostics); > + return len == 5 && strcmp (flag_analyzer_trim_diagnostics, > "never") == 0; > +} > + > namespace ana { > > class feasible_worklist; > @@ -2281,6 +2314,8 @@ diagnostic_manager::prune_path (checker_path > *path, > path->maybe_log (get_logger (), "path"); > prune_for_sm_diagnostic (path, sm, sval, state); > prune_interproc_events (path); > + if (! trim_diagnostics_never_p ()) > + trim_diagnostic_path (path); > consolidate_conditions (path); > finish_pruning (path); > path->maybe_log (get_logger (), "pruned"); > @@ -2667,6 +2702,103 @@ diagnostic_manager::prune_interproc_events > (checker_path *path) const > while (changed); > } > > +void > +diagnostic_manager::trim_diagnostic_path (checker_path *path) const > +{ > + if (trim_diagnostics_system_p ()) > + prune_system_headers (path); > + else > + prune_events_too_deep (path); > +} > + > +/* Remove everything within ]callsite, IDX]. */ > +static void > +prune_within_frame (checker_path *path, int &idx) > +{ > + int nesting = 1; > + while (idx >= 0 && nesting != 0) > + { > + if (path->get_checker_event (idx)->is_call_p ()) > + nesting--; > + else if (path->get_checker_event (idx)->is_return_p ()) > + nesting++; > + > + path->delete_event (idx--); > + } > +} > + > +void > +diagnostic_manager::prune_system_headers (checker_path *path) const > +{ > + int idx = (signed)path->num_events () - 1; > + while (idx >= 0) > + { > + const checker_event *event = path->get_checker_event (idx); > + /* Prune everything between [..., system call, (...), system > return, > ...]. */ > + if (event->is_return_p () > + && in_system_header_at (event->get_location ())) > + { > + int ret_idx = idx; > + prune_within_frame (path, idx); > + > + if (get_logger ()) > + { > + label_text desc > + (path->get_checker_event (idx)->get_desc (false)); > + log ("filtering event %i-%i:" > + " system header event: %s", > + idx, ret_idx, desc.get ()); > + } > + // delete callsite > + if (idx >= 0) > + path->delete_event (idx); > + } > + > + idx--; > + } > +} > + > +void > +diagnostic_manager::prune_events_too_deep (checker_path *path) const > +{ > + long depth = 0; > + long maxdepth = trim_diagnostics_depth (); > + gcc_assert (maxdepth >= 0); > + int idx = (signed)path->num_events () - 1; > + if (idx >= 0) > + { > + depth = path->get_checker_event (0)->get_stack_depth (); > + maxdepth += depth; > + } > + > + while (idx >= 0) > + { > + const checker_event *event = path->get_checker_event (idx); > + /* Prune everything between [..., call too deep, (...), return > too > deep, ...]. */ > + if (event->is_return_p () > + && event->get_stack_depth () > maxdepth) > + { > + int ret_idx = idx; > + prune_within_frame (path, idx); > + > + if (get_logger ()) > + { > + label_text desc > + (path->get_checker_event (idx)->get_desc (false)); > + log ("filtering event %i-%i:" > + " event too deep: %s", > + idx, ret_idx, desc.get ()); > + } > + // delete callsite > + if (idx >= 0) > + path->delete_event (idx); > + } > + > + idx--; > + } > + > +} > + > /* Return true iff event IDX within PATH is on the same line as > REF_EXP_LOC. */ > > static bool > diff --git a/gcc/analyzer/diagnostic-manager.h > b/gcc/analyzer/diagnostic-manager.h > index 9b3e903fc5d..9afef0975be 100644 > --- a/gcc/analyzer/diagnostic-manager.h > +++ b/gcc/analyzer/diagnostic-manager.h > @@ -21,6 +21,10 @@ along with GCC; see the file COPYING3. If not see > #ifndef GCC_ANALYZER_DIAGNOSTIC_MANAGER_H > #define GCC_ANALYZER_DIAGNOSTIC_MANAGER_H > > +long trim_diagnostics_depth (); > +bool trim_diagnostics_system_p (); > +bool trim_diagnostics_never_p (); > + > namespace ana { > > class epath_finder; > @@ -180,6 +184,9 @@ private: > state_machine::state_t state) const; > void update_for_unsuitable_sm_exprs (tree *expr) const; > void prune_interproc_events (checker_path *path) const; > + void trim_diagnostic_path (checker_path *path) const; > + void prune_system_headers (checker_path *path) const; > + void prune_events_too_deep (checker_path *path) const; > void consolidate_conditions (checker_path *path) const; > void finish_pruning (checker_path *path) const; > > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > diagnostics-0.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > new file mode 100644 > index 00000000000..ddf39ed1690 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > @@ -0,0 +1,19 @@ > +/* { dg-additional-options "-fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > diagnostics=0" } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > } */ > + > + return 0; > +} > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > diagnostics-1.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > new file mode 100644 > index 00000000000..83a132078a1 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > @@ -0,0 +1,28 @@ > +/* { dg-additional-options "-fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > diagnostics=1" } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > } */ > + > + return 0; > +} > + > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::operator->() const [with _Tp = A; > __gnu_cxx::_Lock_policy > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > <anonymous> = > false]': event 3 > + | > + | > + <------+ > + | > + 'int main()': events 4-5 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > diagnostics-2.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > new file mode 100644 > index 00000000000..cf065929ec9 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > @@ -0,0 +1,44 @@ > +/* { dg-additional-options "-fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > diagnostics=2" } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > } */ > + > + return 0; > +} > + > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::operator->() const [with _Tp = A; > __gnu_cxx::_Lock_policy > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > <anonymous> = > false]': events 3-4 > + | > + | > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> > = > false; bool <anonymous> = false]': events 5-6 > + | > + | > + +--> 'std::__shared_ptr<_Tp, _Lp>::element_type* > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 > + | > + | > + <------+ > + | > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> > = > false; bool <anonymous> = false]': event 9 > + | > + | > + <------+ > + | > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::operator->() const [with _Tp = A; > __gnu_cxx::_Lock_policy > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > <anonymous> = > false]': event 10 > + | > + | > + <------+ > + | > + 'int main()': events 11-12 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file > diff --git > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C > new file mode 100644 > index 00000000000..fc271309d4e > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > default.C > @@ -0,0 +1,21 @@ > +/* { dg-additional-options "-O0 -fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events" } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +// Must behave like -fanalyzer-trim-diagnostics=system > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > } */ > + > + return 0; > +} > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file > diff --git > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > new file mode 100644 > index 00000000000..a69416608da > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > never.C > @@ -0,0 +1,44 @@ > +/* { dg-additional-options "-fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > diagnostics=never" > } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > } */ > + > + return 0; > +} > + > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::operator->() const [with _Tp = A; > __gnu_cxx::_Lock_policy > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > <anonymous> = > false]': events 3-4 > + | > + | > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> > = > false; bool <anonymous> = false]': events 5-6 > + | > + | > + +--> 'std::__shared_ptr<_Tp, _Lp>::element_type* > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 > + | > + | > + <------+ > + | > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> > = > false; bool <anonymous> = false]': event 9 > + | > + | > + <------+ > + | > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::operator->() const [with _Tp = A; > __gnu_cxx::_Lock_policy > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > <anonymous> = > false]': event 10 > + | > + | > + <------+ > + | > + 'int main()': events 11-12 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file > diff --git > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C > new file mode 100644 > index 00000000000..22fadc84e75 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > system.C > @@ -0,0 +1,19 @@ > +/* { dg-additional-options "-fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > diagnostics=system" > } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > } */ > + > + return 0; > +} > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file
On Fri, 21 Jul 2023 at 21:05, Benjamin Priour via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Hi, > > Upon David's request I've joined the in progress patch to the below email. > I hope it makes more sense now. > > Best, > Benjamin. > > ---------- Forwarded message --------- > From: Benjamin Priour <priour.be@gmail.com> > Date: Tue, Jul 18, 2023 at 3:30 PM > Subject: [RFC] analyzer: Add optional trim of the analyzer diagnostics > going too deep [PR110543] > To: <gcc-patches@gcc.gnu.org>, David Malcolm <dmalcolm@redhat.com> > > > Hi, > > I'd like to request comments on a patch I am writing for PR110543. > The goal of this patch is to reduce the noise of the analyzer emitted > diagnostics when dealing with > system headers, or simply diagnostic paths that are too long. The new > option only affects the display > of the diagnostics, but doesn't hinder the actual analysis. > > I've defaulted the new option to "system", thus preventing the diagnostic > paths from showing system headers. > "never" corresponds to the pre-patch behavior, whereas you can also specify > an unsigned value <maxdepth> > that prevents paths to go deeper than <maxdepth> frames. > > fanalyzer-trim-diagnostics= > > Common Joined RejectNegative ToLower Var(flag_analyzer_trim_diagnostics) > > Init("system") > > -fanalyzer-trim-diagnostics=[never|system|<maxdepth>] Trim diagnostics > > path that are too long before emission. > > > > Does it sounds reasonable and user-friendly ? > > Regstrapping was a success against trunk, although one of the newly added > test case fails for c++14. > Note that the test case below was done with "never", thus behaves exactly > as the pre-patch analyzer > on x86_64-linux-gnu. > > /* { dg-additional-options "-fdiagnostics-plain-output > > -fdiagnostics-path-format=inline-events -fanalyzer-trim-diagnostics=never" > > } */ > > /* { dg-skip-if "" { c++98_only } } */ > > > > #include <memory> > > struct A {int x; int y;}; > > > > int main () { > > std::shared_ptr<A> a; > > a->x = 4; /* { dg-line deref_a } */ > > /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ > > > > return 0; > > } > > > > /* { dg-begin-multiline-output "" } > > 'int main()': events 1-2 > > | > > | > > +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> > > >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = > > false]': events 3-4 > > | > > | > > +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = > > false; bool <anonymous> = false]': events 5-6 > > | > > | > > +--> 'std::__shared_ptr<_Tp, _Lp>::element_type* > > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 > > | > > | > > <------+ > > | > > 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = > > false; bool <anonymous> = false]': event 9 > > | > > | > > <------+ > > | > > 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> > > >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = > > false]': event 10 > > | > > | > > <------+ > > | > > 'int main()': events 11-12 > > | > > | > > { dg-end-multiline-output "" } */ > > > > > The first events "'int main()': events 1-2" vary in c++14 (get events 1-3). > > > > > // c++14 with fully detailed output > > ‘int main()’: events 1-3 > > | > > | 8 | int main () { > > | | ^~~~ > > | | | > > | | (1) entry to ‘main’ > > | 9 | std::shared_ptr<A> a; > > | | ~ > > | | | > > | | (2) > > ‘a.std::shared_ptr<A>::<anonymous>.std::__shared_ptr<A, > > __gnu_cxx::_S_atomic>::_M_ptr’ is NULL > > | 10 | a->x = 4; /* { dg-line deref_a } */ > > | | ~~ > > | | | > > | | (3) calling ‘std::__shared_ptr_access<A, > > __gnu_cxx::_S_atomic, false, false>::operator->’ from ‘main’ > > > > whereas c++17 and posterior give > > > // c++17 with fully detailed output > > > // ./xg++ -fanalyzer > > ../../gcc/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > > -B. -shared-libgcc -fanalyzer-trim-diagnostics=never -std=c++17 > > > ‘int main()’: events 1-2 > > | > > | 8 | int main () { > > | | ^~~~ > > | | | > > | | (1) entry to ‘main’ > > | 9 | std::shared_ptr<A> a; > > | 10 | a->x = 4; /* { dg-line deref_a } */ > > | | ~~ > > | | | > > | | (2) calling ‘std::__shared_ptr_access<A, > > __gnu_cxx::_S_atomic, false, false>::operator->’ from ‘main’ > > > > Is there a way to make dg-multiline-output check for a regex ? Or would > checking the multiline-output only for c++17 and c++20 be acceptable ? > This divergence results from two slightly different IPA: > > // c++14 -fdump-ipa-analyzer > > // std::shared_ptr<A> a; becomes > > a.D.29392._M_ptr = 0B; > > a.D.29392._M_refcount._M_pi = 0B; > > > > whereas in c++17 > > > // c++17 -fdump-ipa-analyzer > > // std::shared_ptr<A> a; becomes > > a = {}; > > > > I know shared_ptr limited support is a coincidence more than a feature, but > maybe there is still a way to make the above test case work ? > Otherwise, I'd fallback to a supported system function. If you have any > that you fancy, do tell me. > > Thanks, > Benjamin. > > --- > gcc/analyzer/analyzer.cc | 2 +- > gcc/analyzer/analyzer.h | 1 + > gcc/analyzer/analyzer.opt | 4 + > gcc/analyzer/diagnostic-manager.cc | 132 ++++++++++++++++++ > gcc/analyzer/diagnostic-manager.h | 7 + > .../analyzer/fanalyzer-trim-diagnostics-0.C | 19 +++ > .../analyzer/fanalyzer-trim-diagnostics-1.C | 28 ++++ > .../analyzer/fanalyzer-trim-diagnostics-2.C | 44 ++++++ > .../fanalyzer-trim-diagnostics-default.C | 21 +++ > .../fanalyzer-trim-diagnostics-never.C | 44 ++++++ > .../fanalyzer-trim-diagnostics-system.C | 19 +++ > 11 files changed, 320 insertions(+), 1 deletion(-) > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > create mode 100644 > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C > > diff --git a/gcc/analyzer/analyzer.cc b/gcc/analyzer/analyzer.cc > index 5091fb7a583..b27d8e359db 100644 > --- a/gcc/analyzer/analyzer.cc > +++ b/gcc/analyzer/analyzer.cc > @@ -274,7 +274,7 @@ is_named_call_p (const_tree fndecl, const char > *funcname) > Compare with cp/typeck.cc: decl_in_std_namespace_p, but this doesn't > rely on being the C++ FE (or handle inline namespaces inside of std). > */ > > -static inline bool > +bool > is_std_function_p (const_tree fndecl) > { > tree name_decl = DECL_NAME (fndecl); > diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h > index 579517c23e6..31597079153 100644 > --- a/gcc/analyzer/analyzer.h > +++ b/gcc/analyzer/analyzer.h > @@ -386,6 +386,7 @@ extern bool is_special_named_call_p (const gcall *call, > const char *funcname, > extern bool is_named_call_p (const_tree fndecl, const char *funcname); > extern bool is_named_call_p (const_tree fndecl, const char *funcname, > const gcall *call, unsigned int num_args); > +extern bool is_std_function_p (const_tree fndecl); > extern bool is_std_named_call_p (const_tree fndecl, const char *funcname); > extern bool is_std_named_call_p (const_tree fndecl, const char *funcname, > const gcall *call, unsigned int num_args); > diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt > index 2760aaa8151..78ff2c07483 100644 > --- a/gcc/analyzer/analyzer.opt > +++ b/gcc/analyzer/analyzer.opt > @@ -362,4 +362,8 @@ fdump-analyzer-untracked > Common RejectNegative Var(flag_dump_analyzer_untracked) > Emit custom warnings with internal details intended for analyzer > developers. > > +fanalyzer-trim-diagnostics= > +Common Joined RejectNegative ToLower Var(flag_analyzer_trim_diagnostics) > Init("system") > +-fanalyzer-trim-diagnostics=[never|system|<maxdepth>] Trim diagnostics > path that are too long before emission. > + > ; This comment is to ensure we retain the blank line above. > diff --git a/gcc/analyzer/diagnostic-manager.cc > b/gcc/analyzer/diagnostic-manager.cc > index cfca305d552..d946ad5bac7 100644 > --- a/gcc/analyzer/diagnostic-manager.cc > +++ b/gcc/analyzer/diagnostic-manager.cc > @@ -20,9 +20,11 @@ along with GCC; see the file COPYING3. If not see > > #include "config.h" > #define INCLUDE_MEMORY > +#define INCLUDE_VECTOR > #include "system.h" > #include "coretypes.h" > #include "tree.h" > +#include "input.h" > #include "pretty-print.h" > #include "gcc-rich-location.h" > #include "gimple-pretty-print.h" > @@ -60,6 +62,37 @@ along with GCC; see the file COPYING3. If not see > > #if ENABLE_ANALYZER > > +/* Return positive value of fanalyzer-trim-diagnostics if a depth was given > + or a negative value otherwise. */ > + > +long trim_diagnostics_depth () > +{ > + if (trim_diagnostics_system_p () || trim_diagnostics_never_p ()) > + return -1; > + > + char *end = NULL; > + long depth = strtol (flag_analyzer_trim_diagnostics, &end, 10); > + gcc_assert (end == NULL || *end == '\0'); > + gcc_assert (!(end == flag_analyzer_trim_diagnostics && depth == 0)); > + return depth; > +} > + > +/* Return true if fanalyzer-trim-diagnostics is set to "system". */ > + > +bool trim_diagnostics_system_p () > +{ > + int len = strlen (flag_analyzer_trim_diagnostics); > + return len == 6 && strcmp (flag_analyzer_trim_diagnostics, "system") == > 0; > +} > + > +/* Return true if fanalyzer-trim-diagnostics is set to "never". */ > + > +bool trim_diagnostics_never_p () > +{ > + int len = strlen (flag_analyzer_trim_diagnostics); > + return len == 5 && strcmp (flag_analyzer_trim_diagnostics, "never") == 0; > +} Hi Benjamin, Just wondering if the len check is necessary here ? IIUC, strcmp should handle the case correctly when the strings are of different lengths ? Also, I suppose it'd be better to merge above two functions, taking the string to match against ("system" / "never") as argument. Thanks, Prathamesh > + > namespace ana { > > class feasible_worklist; > @@ -2281,6 +2314,8 @@ diagnostic_manager::prune_path (checker_path *path, > path->maybe_log (get_logger (), "path"); > prune_for_sm_diagnostic (path, sm, sval, state); > prune_interproc_events (path); > + if (! trim_diagnostics_never_p ()) > + trim_diagnostic_path (path); > consolidate_conditions (path); > finish_pruning (path); > path->maybe_log (get_logger (), "pruned"); > @@ -2667,6 +2702,103 @@ diagnostic_manager::prune_interproc_events > (checker_path *path) const > while (changed); > } > > +void > +diagnostic_manager::trim_diagnostic_path (checker_path *path) const > +{ > + if (trim_diagnostics_system_p ()) > + prune_system_headers (path); > + else > + prune_events_too_deep (path); > +} > + > +/* Remove everything within ]callsite, IDX]. */ > +static void > +prune_within_frame (checker_path *path, int &idx) > +{ > + int nesting = 1; > + while (idx >= 0 && nesting != 0) > + { > + if (path->get_checker_event (idx)->is_call_p ()) > + nesting--; > + else if (path->get_checker_event (idx)->is_return_p ()) > + nesting++; > + > + path->delete_event (idx--); > + } > +} > + > +void > +diagnostic_manager::prune_system_headers (checker_path *path) const > +{ > + int idx = (signed)path->num_events () - 1; > + while (idx >= 0) > + { > + const checker_event *event = path->get_checker_event (idx); > + /* Prune everything between [..., system call, (...), system return, > ...]. */ > + if (event->is_return_p () > + && in_system_header_at (event->get_location ())) > + { > + int ret_idx = idx; > + prune_within_frame (path, idx); > + > + if (get_logger ()) > + { > + label_text desc > + (path->get_checker_event (idx)->get_desc (false)); > + log ("filtering event %i-%i:" > + " system header event: %s", > + idx, ret_idx, desc.get ()); > + } > + // delete callsite > + if (idx >= 0) > + path->delete_event (idx); > + } > + > + idx--; > + } > +} > + > +void > +diagnostic_manager::prune_events_too_deep (checker_path *path) const > +{ > + long depth = 0; > + long maxdepth = trim_diagnostics_depth (); > + gcc_assert (maxdepth >= 0); > + int idx = (signed)path->num_events () - 1; > + if (idx >= 0) > + { > + depth = path->get_checker_event (0)->get_stack_depth (); > + maxdepth += depth; > + } > + > + while (idx >= 0) > + { > + const checker_event *event = path->get_checker_event (idx); > + /* Prune everything between [..., call too deep, (...), return too > deep, ...]. */ > + if (event->is_return_p () > + && event->get_stack_depth () > maxdepth) > + { > + int ret_idx = idx; > + prune_within_frame (path, idx); > + > + if (get_logger ()) > + { > + label_text desc > + (path->get_checker_event (idx)->get_desc (false)); > + log ("filtering event %i-%i:" > + " event too deep: %s", > + idx, ret_idx, desc.get ()); > + } > + // delete callsite > + if (idx >= 0) > + path->delete_event (idx); > + } > + > + idx--; > + } > + > +} > + > /* Return true iff event IDX within PATH is on the same line as > REF_EXP_LOC. */ > > static bool > diff --git a/gcc/analyzer/diagnostic-manager.h > b/gcc/analyzer/diagnostic-manager.h > index 9b3e903fc5d..9afef0975be 100644 > --- a/gcc/analyzer/diagnostic-manager.h > +++ b/gcc/analyzer/diagnostic-manager.h > @@ -21,6 +21,10 @@ along with GCC; see the file COPYING3. If not see > #ifndef GCC_ANALYZER_DIAGNOSTIC_MANAGER_H > #define GCC_ANALYZER_DIAGNOSTIC_MANAGER_H > > +long trim_diagnostics_depth (); > +bool trim_diagnostics_system_p (); > +bool trim_diagnostics_never_p (); > + > namespace ana { > > class epath_finder; > @@ -180,6 +184,9 @@ private: > state_machine::state_t state) const; > void update_for_unsuitable_sm_exprs (tree *expr) const; > void prune_interproc_events (checker_path *path) const; > + void trim_diagnostic_path (checker_path *path) const; > + void prune_system_headers (checker_path *path) const; > + void prune_events_too_deep (checker_path *path) const; > void consolidate_conditions (checker_path *path) const; > void finish_pruning (checker_path *path) const; > > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > new file mode 100644 > index 00000000000..ddf39ed1690 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > @@ -0,0 +1,19 @@ > +/* { dg-additional-options "-fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events -fanalyzer-trim-diagnostics=0" } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ > + > + return 0; > +} > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > new file mode 100644 > index 00000000000..83a132078a1 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > @@ -0,0 +1,28 @@ > +/* { dg-additional-options "-fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events -fanalyzer-trim-diagnostics=1" } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ > + > + return 0; > +} > + > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> > >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = > false]': event 3 > + | > + | > + <------+ > + | > + 'int main()': events 4-5 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > new file mode 100644 > index 00000000000..cf065929ec9 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > @@ -0,0 +1,44 @@ > +/* { dg-additional-options "-fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events -fanalyzer-trim-diagnostics=2" } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ > + > + return 0; > +} > + > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> > >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = > false]': events 3-4 > + | > + | > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = > false; bool <anonymous> = false]': events 5-6 > + | > + | > + +--> 'std::__shared_ptr<_Tp, _Lp>::element_type* > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 > + | > + | > + <------+ > + | > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = > false; bool <anonymous> = false]': event 9 > + | > + | > + <------+ > + | > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> > >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = > false]': event 10 > + | > + | > + <------+ > + | > + 'int main()': events 11-12 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file > diff --git > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C > new file mode 100644 > index 00000000000..fc271309d4e > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C > @@ -0,0 +1,21 @@ > +/* { dg-additional-options "-O0 -fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events" } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +// Must behave like -fanalyzer-trim-diagnostics=system > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ > + > + return 0; > +} > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file > diff --git > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > new file mode 100644 > index 00000000000..a69416608da > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > @@ -0,0 +1,44 @@ > +/* { dg-additional-options "-fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events -fanalyzer-trim-diagnostics=never" > } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ > + > + return 0; > +} > + > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> > >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = > false]': events 3-4 > + | > + | > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = > false; bool <anonymous> = false]': events 5-6 > + | > + | > + +--> 'std::__shared_ptr<_Tp, _Lp>::element_type* > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 > + | > + | > + <------+ > + | > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = > false; bool <anonymous> = false]': event 9 > + | > + | > + <------+ > + | > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> > >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = > false]': event 10 > + | > + | > + <------+ > + | > + 'int main()': events 11-12 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file > diff --git > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C > new file mode 100644 > index 00000000000..22fadc84e75 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C > @@ -0,0 +1,19 @@ > +/* { dg-additional-options "-fdiagnostics-plain-output > -fdiagnostics-path-format=inline-events -fanalyzer-trim-diagnostics=system" > } */ > +/* { dg-skip-if "" { c++98_only } } */ > + > +#include <memory> > + > +struct A {int x; int y;}; > + > +int main () { > + std::shared_ptr<A> a; > + a->x = 4; /* { dg-line deref_a } */ > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ > + > + return 0; > +} > +/* { dg-begin-multiline-output "" } > + 'int main()': events 1-2 > + | > + | > + { dg-end-multiline-output "" } */ > \ No newline at end of file > -- > 2.34.1
On Sat, Jul 22, 2023 at 12:04 AM David Malcolm <dmalcolm@redhat.com> wrote: > On Fri, 2023-07-21 at 17:35 +0200, Benjamin Priour wrote: > > Hi, > > > > Upon David's request I've joined the in progress patch to the below > > email. > > I hope it makes more sense now. > > > > Best, > > Benjamin. > > Thanks for posting the work-in-progress patch; it makes the idea > clearer. > > Some thoughts about this: > > - I like the idea of defaulting to *not* showing events within system > headers, which the patch achieves > - I don't like the combination of never/system with maxdepth, in that > it seems complicated and I don't think a user is likely to experiment > with different depths. > - Hence I think it would work better as a simple boolean, perhaps > "-fanalyzer-show-events-in-system-headers" > or somesuch? It seems like the sort of thing that we want to provide > a sensible default for, but have the option of turning off for > debugging the analyzer itself, but I don't expect an end-user to touch > that option. > A boolean sounds good, I will trust your experience with the end-user here, especially since <maxdepth> and "never" had some overlap, it could have been confusing. > FWIW the patch seems to have been mangled somewhat via email, so I > don't have a sense of what the actual output from patched analyzer > looks like. What should we output to the user with -fanalyzer and no > other options for the case in PR 110543? Currently, for > https://godbolt.org/z/sb9dM9Gqa trunk emits 12 events, of which > probably only this last one is useful: > > (12) dereference of NULL 'a.std::__shared_ptr_access<A, > __gnu_cxx::_S_atomic, false, false>::operator->()' > > What does the output look like with your patch? > The plan with this patch was to get events : (1) entry to 'main' (2) calling 'std::__shared_ptr_access<A, __gnu_cxx::_S_atomic, false, false>::operator->' from 'main' (12) dereference of NULL 'a.std::__shared_ptr_access<A, __gnu_cxx::_S_atomic, false, false>::operator->()' (11) returning to 'main' from 'std::__shared_ptr_access<A, __gnu_cxx::_S_atomic, false, false>::operator->' This way, we get the entry and exit point to the system headers ( (2) and (11) ), and the actual injurious event ( (12) ). We could however go as you suggest, with an even more succint path and only keep (1) and (12). Thanks, Benjamin > Thanks > Dave > > > > > > ---------- Forwarded message --------- > > From: Benjamin Priour <priour.be@gmail.com> > > Date: Tue, Jul 18, 2023 at 3:30 PM > > Subject: [RFC] analyzer: Add optional trim of the analyzer > > diagnostics > > going too deep [PR110543] > > To: <gcc-patches@gcc.gnu.org>, David Malcolm <dmalcolm@redhat.com> > > > > > > Hi, > > > > I'd like to request comments on a patch I am writing for PR110543. > > The goal of this patch is to reduce the noise of the analyzer emitted > > diagnostics when dealing with > > system headers, or simply diagnostic paths that are too long. The new > > option only affects the display > > of the diagnostics, but doesn't hinder the actual analysis. > > > > I've defaulted the new option to "system", thus preventing the > > diagnostic > > paths from showing system headers. > > "never" corresponds to the pre-patch behavior, whereas you can also > > specify > > an unsigned value <maxdepth> > > that prevents paths to go deeper than <maxdepth> frames. > > > > fanalyzer-trim-diagnostics= > > > Common Joined RejectNegative ToLower > > > Var(flag_analyzer_trim_diagnostics) > > > Init("system") > > > -fanalyzer-trim-diagnostics=[never|system|<maxdepth>] Trim > > > diagnostics > > > path that are too long before emission. > > > > > > > Does it sounds reasonable and user-friendly ? > > > > Regstrapping was a success against trunk, although one of the newly > > added > > test case fails for c++14. > > Note that the test case below was done with "never", thus behaves > > exactly > > as the pre-patch analyzer > > on x86_64-linux-gnu. > > > > /* { dg-additional-options "-fdiagnostics-plain-output > > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > > diagnostics=never" > > > } */ > > > /* { dg-skip-if "" { c++98_only } } */ > > > > > > #include <memory> > > > struct A {int x; int y;}; > > > > > > int main () { > > > std::shared_ptr<A> a; > > > a->x = 4; /* { dg-line deref_a } */ > > > /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > > > } */ > > > > > > return 0; > > > } > > > > > > /* { dg-begin-multiline-output "" } > > > 'int main()': events 1-2 > > > | > > > | > > > +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> > > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::operator->() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy > > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > > <anonymous> = > > > false]': events 3-4 > > > | > > > | > > > +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool > > > <anonymous> = > > > false; bool <anonymous> = false]': events 5-6 > > > | > > > | > > > +--> 'std::__shared_ptr<_Tp, _Lp>::element_type* > > > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 > > > | > > > | > > > <------+ > > > | > > > 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool > > > <anonymous> = > > > false; bool <anonymous> = false]': event 9 > > > | > > > | > > > <------+ > > > | > > > 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> > > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::operator->() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy > > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > > <anonymous> = > > > false]': event 10 > > > | > > > | > > > <------+ > > > | > > > 'int main()': events 11-12 > > > | > > > | > > > { dg-end-multiline-output "" } */ > > > > > > > > > The first events "'int main()': events 1-2" vary in c++14 (get events > > 1-3). > > > > > > > > // c++14 with fully detailed output > > > ‘int main()’: events 1-3 > > > | > > > | 8 | int main () { > > > | | ^~~~ > > > | | | > > > | | (1) entry to ‘main’ > > > | 9 | std::shared_ptr<A> a; > > > | | ~ > > > | | | > > > | | (2) > > > ‘a.std::shared_ptr<A>::<anonymous>.std::__shared_ptr<A, > > > __gnu_cxx::_S_atomic>::_M_ptr’ is NULL > > > | 10 | a->x = 4; /* { dg-line deref_a } */ > > > | | ~~ > > > | | | > > > | | (3) calling ‘std::__shared_ptr_access<A, > > > __gnu_cxx::_S_atomic, false, false>::operator->’ from ‘main’ > > > > > > > whereas c++17 and posterior give > > > > > // c++17 with fully detailed output > > > > > // ./xg++ -fanalyzer > > > ../../gcc/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > > > diagnostics-never.C > > > -B. -shared-libgcc -fanalyzer-trim-diagnostics=never -std=c++17 > > > > > ‘int main()’: events 1-2 > > > | > > > | 8 | int main () { > > > | | ^~~~ > > > | | | > > > | | (1) entry to ‘main’ > > > | 9 | std::shared_ptr<A> a; > > > | 10 | a->x = 4; /* { dg-line deref_a } */ > > > | | ~~ > > > | | | > > > | | (2) calling ‘std::__shared_ptr_access<A, > > > __gnu_cxx::_S_atomic, false, false>::operator->’ from ‘main’ > > > > > > > Is there a way to make dg-multiline-output check for a regex ? Or > > would > > checking the multiline-output only for c++17 and c++20 be acceptable > > ? > > This divergence results from two slightly different IPA: > > > > // c++14 -fdump-ipa-analyzer > > > // std::shared_ptr<A> a; becomes > > > a.D.29392._M_ptr = 0B; > > > a.D.29392._M_refcount._M_pi = 0B; > > > > > > > whereas in c++17 > > > > > // c++17 -fdump-ipa-analyzer > > > // std::shared_ptr<A> a; becomes > > > a = {}; > > > > > > > I know shared_ptr limited support is a coincidence more than a > > feature, but > > maybe there is still a way to make the above test case work ? > > Otherwise, I'd fallback to a supported system function. If you have > > any > > that you fancy, do tell me. > > > > Thanks, > > Benjamin. > > > > --- > > gcc/analyzer/analyzer.cc | 2 +- > > gcc/analyzer/analyzer.h | 1 + > > gcc/analyzer/analyzer.opt | 4 + > > gcc/analyzer/diagnostic-manager.cc | 132 > > ++++++++++++++++++ > > gcc/analyzer/diagnostic-manager.h | 7 + > > .../analyzer/fanalyzer-trim-diagnostics-0.C | 19 +++ > > .../analyzer/fanalyzer-trim-diagnostics-1.C | 28 ++++ > > .../analyzer/fanalyzer-trim-diagnostics-2.C | 44 ++++++ > > .../fanalyzer-trim-diagnostics-default.C | 21 +++ > > .../fanalyzer-trim-diagnostics-never.C | 44 ++++++ > > .../fanalyzer-trim-diagnostics-system.C | 19 +++ > > 11 files changed, 320 insertions(+), 1 deletion(-) > > create mode 100644 > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > > create mode 100644 > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > > create mode 100644 > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > > create mode 100644 > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C > > create mode 100644 > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > > create mode 100644 > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C > > > > diff --git a/gcc/analyzer/analyzer.cc b/gcc/analyzer/analyzer.cc > > index 5091fb7a583..b27d8e359db 100644 > > --- a/gcc/analyzer/analyzer.cc > > +++ b/gcc/analyzer/analyzer.cc > > @@ -274,7 +274,7 @@ is_named_call_p (const_tree fndecl, const char > > *funcname) > > Compare with cp/typeck.cc: decl_in_std_namespace_p, but this > > doesn't > > rely on being the C++ FE (or handle inline namespaces inside of > > std). > > */ > > > > -static inline bool > > +bool > > is_std_function_p (const_tree fndecl) > > { > > tree name_decl = DECL_NAME (fndecl); > > diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h > > index 579517c23e6..31597079153 100644 > > --- a/gcc/analyzer/analyzer.h > > +++ b/gcc/analyzer/analyzer.h > > @@ -386,6 +386,7 @@ extern bool is_special_named_call_p (const gcall > > *call, > > const char *funcname, > > extern bool is_named_call_p (const_tree fndecl, const char > > *funcname); > > extern bool is_named_call_p (const_tree fndecl, const char > > *funcname, > > const gcall *call, unsigned int num_args); > > +extern bool is_std_function_p (const_tree fndecl); > > extern bool is_std_named_call_p (const_tree fndecl, const char > > *funcname); > > extern bool is_std_named_call_p (const_tree fndecl, const char > > *funcname, > > const gcall *call, unsigned int num_args); > > diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt > > index 2760aaa8151..78ff2c07483 100644 > > --- a/gcc/analyzer/analyzer.opt > > +++ b/gcc/analyzer/analyzer.opt > > @@ -362,4 +362,8 @@ fdump-analyzer-untracked > > Common RejectNegative Var(flag_dump_analyzer_untracked) > > Emit custom warnings with internal details intended for analyzer > > developers. > > > > +fanalyzer-trim-diagnostics= > > +Common Joined RejectNegative ToLower > > Var(flag_analyzer_trim_diagnostics) > > Init("system") > > +-fanalyzer-trim-diagnostics=[never|system|<maxdepth>] Trim > > diagnostics > > path that are too long before emission. > > + > > ; This comment is to ensure we retain the blank line above. > > diff --git a/gcc/analyzer/diagnostic-manager.cc > > b/gcc/analyzer/diagnostic-manager.cc > > index cfca305d552..d946ad5bac7 100644 > > --- a/gcc/analyzer/diagnostic-manager.cc > > +++ b/gcc/analyzer/diagnostic-manager.cc > > @@ -20,9 +20,11 @@ along with GCC; see the file COPYING3. If not see > > > > #include "config.h" > > #define INCLUDE_MEMORY > > +#define INCLUDE_VECTOR > > #include "system.h" > > #include "coretypes.h" > > #include "tree.h" > > +#include "input.h" > > #include "pretty-print.h" > > #include "gcc-rich-location.h" > > #include "gimple-pretty-print.h" > > @@ -60,6 +62,37 @@ along with GCC; see the file COPYING3. If not see > > > > #if ENABLE_ANALYZER > > > > +/* Return positive value of fanalyzer-trim-diagnostics if a depth > > was given > > + or a negative value otherwise. */ > > + > > +long trim_diagnostics_depth () > > +{ > > + if (trim_diagnostics_system_p () || trim_diagnostics_never_p ()) > > + return -1; > > + > > + char *end = NULL; > > + long depth = strtol (flag_analyzer_trim_diagnostics, &end, 10); > > + gcc_assert (end == NULL || *end == '\0'); > > + gcc_assert (!(end == flag_analyzer_trim_diagnostics && depth == > > 0)); > > + return depth; > > +} > > + > > +/* Return true if fanalyzer-trim-diagnostics is set to "system". */ > > + > > +bool trim_diagnostics_system_p () > > +{ > > + int len = strlen (flag_analyzer_trim_diagnostics); > > + return len == 6 && strcmp (flag_analyzer_trim_diagnostics, > > "system") == > > 0; > > +} > > + > > +/* Return true if fanalyzer-trim-diagnostics is set to "never". */ > > + > > +bool trim_diagnostics_never_p () > > +{ > > + int len = strlen (flag_analyzer_trim_diagnostics); > > + return len == 5 && strcmp (flag_analyzer_trim_diagnostics, > > "never") == 0; > > +} > > + > > namespace ana { > > > > class feasible_worklist; > > @@ -2281,6 +2314,8 @@ diagnostic_manager::prune_path (checker_path > > *path, > > path->maybe_log (get_logger (), "path"); > > prune_for_sm_diagnostic (path, sm, sval, state); > > prune_interproc_events (path); > > + if (! trim_diagnostics_never_p ()) > > + trim_diagnostic_path (path); > > consolidate_conditions (path); > > finish_pruning (path); > > path->maybe_log (get_logger (), "pruned"); > > @@ -2667,6 +2702,103 @@ diagnostic_manager::prune_interproc_events > > (checker_path *path) const > > while (changed); > > } > > > > +void > > +diagnostic_manager::trim_diagnostic_path (checker_path *path) const > > +{ > > + if (trim_diagnostics_system_p ()) > > + prune_system_headers (path); > > + else > > + prune_events_too_deep (path); > > +} > > + > > +/* Remove everything within ]callsite, IDX]. */ > > +static void > > +prune_within_frame (checker_path *path, int &idx) > > +{ > > + int nesting = 1; > > + while (idx >= 0 && nesting != 0) > > + { > > + if (path->get_checker_event (idx)->is_call_p ()) > > + nesting--; > > + else if (path->get_checker_event (idx)->is_return_p ()) > > + nesting++; > > + > > + path->delete_event (idx--); > > + } > > +} > > + > > +void > > +diagnostic_manager::prune_system_headers (checker_path *path) const > > +{ > > + int idx = (signed)path->num_events () - 1; > > + while (idx >= 0) > > + { > > + const checker_event *event = path->get_checker_event (idx); > > + /* Prune everything between [..., system call, (...), system > > return, > > ...]. */ > > + if (event->is_return_p () > > + && in_system_header_at (event->get_location ())) > > + { > > + int ret_idx = idx; > > + prune_within_frame (path, idx); > > + > > + if (get_logger ()) > > + { > > + label_text desc > > + (path->get_checker_event (idx)->get_desc (false)); > > + log ("filtering event %i-%i:" > > + " system header event: %s", > > + idx, ret_idx, desc.get ()); > > + } > > + // delete callsite > > + if (idx >= 0) > > + path->delete_event (idx); > > + } > > + > > + idx--; > > + } > > +} > > + > > +void > > +diagnostic_manager::prune_events_too_deep (checker_path *path) const > > +{ > > + long depth = 0; > > + long maxdepth = trim_diagnostics_depth (); > > + gcc_assert (maxdepth >= 0); > > + int idx = (signed)path->num_events () - 1; > > + if (idx >= 0) > > + { > > + depth = path->get_checker_event (0)->get_stack_depth (); > > + maxdepth += depth; > > + } > > + > > + while (idx >= 0) > > + { > > + const checker_event *event = path->get_checker_event (idx); > > + /* Prune everything between [..., call too deep, (...), return > > too > > deep, ...]. */ > > + if (event->is_return_p () > > + && event->get_stack_depth () > maxdepth) > > + { > > + int ret_idx = idx; > > + prune_within_frame (path, idx); > > + > > + if (get_logger ()) > > + { > > + label_text desc > > + (path->get_checker_event (idx)->get_desc (false)); > > + log ("filtering event %i-%i:" > > + " event too deep: %s", > > + idx, ret_idx, desc.get ()); > > + } > > + // delete callsite > > + if (idx >= 0) > > + path->delete_event (idx); > > + } > > + > > + idx--; > > + } > > + > > +} > > + > > /* Return true iff event IDX within PATH is on the same line as > > REF_EXP_LOC. */ > > > > static bool > > diff --git a/gcc/analyzer/diagnostic-manager.h > > b/gcc/analyzer/diagnostic-manager.h > > index 9b3e903fc5d..9afef0975be 100644 > > --- a/gcc/analyzer/diagnostic-manager.h > > +++ b/gcc/analyzer/diagnostic-manager.h > > @@ -21,6 +21,10 @@ along with GCC; see the file COPYING3. If not see > > #ifndef GCC_ANALYZER_DIAGNOSTIC_MANAGER_H > > #define GCC_ANALYZER_DIAGNOSTIC_MANAGER_H > > > > +long trim_diagnostics_depth (); > > +bool trim_diagnostics_system_p (); > > +bool trim_diagnostics_never_p (); > > + > > namespace ana { > > > > class epath_finder; > > @@ -180,6 +184,9 @@ private: > > state_machine::state_t state) const; > > void update_for_unsuitable_sm_exprs (tree *expr) const; > > void prune_interproc_events (checker_path *path) const; > > + void trim_diagnostic_path (checker_path *path) const; > > + void prune_system_headers (checker_path *path) const; > > + void prune_events_too_deep (checker_path *path) const; > > void consolidate_conditions (checker_path *path) const; > > void finish_pruning (checker_path *path) const; > > > > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > > diagnostics-0.C > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > > new file mode 100644 > > index 00000000000..ddf39ed1690 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > > @@ -0,0 +1,19 @@ > > +/* { dg-additional-options "-fdiagnostics-plain-output > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > diagnostics=0" } */ > > +/* { dg-skip-if "" { c++98_only } } */ > > + > > +#include <memory> > > + > > +struct A {int x; int y;}; > > + > > +int main () { > > + std::shared_ptr<A> a; > > + a->x = 4; /* { dg-line deref_a } */ > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > > } */ > > + > > + return 0; > > +} > > +/* { dg-begin-multiline-output "" } > > + 'int main()': events 1-2 > > + | > > + | > > + { dg-end-multiline-output "" } */ > > \ No newline at end of file > > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > > diagnostics-1.C > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > > new file mode 100644 > > index 00000000000..83a132078a1 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > > @@ -0,0 +1,28 @@ > > +/* { dg-additional-options "-fdiagnostics-plain-output > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > diagnostics=1" } */ > > +/* { dg-skip-if "" { c++98_only } } */ > > + > > +#include <memory> > > + > > +struct A {int x; int y;}; > > + > > +int main () { > > + std::shared_ptr<A> a; > > + a->x = 4; /* { dg-line deref_a } */ > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > > } */ > > + > > + return 0; > > +} > > + > > +/* { dg-begin-multiline-output "" } > > + 'int main()': events 1-2 > > + | > > + | > > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::operator->() const [with _Tp = A; > > __gnu_cxx::_Lock_policy > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > <anonymous> = > > false]': event 3 > > + | > > + | > > + <------+ > > + | > > + 'int main()': events 4-5 > > + | > > + | > > + { dg-end-multiline-output "" } */ > > \ No newline at end of file > > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > > diagnostics-2.C > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > > new file mode 100644 > > index 00000000000..cf065929ec9 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > > @@ -0,0 +1,44 @@ > > +/* { dg-additional-options "-fdiagnostics-plain-output > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > diagnostics=2" } */ > > +/* { dg-skip-if "" { c++98_only } } */ > > + > > +#include <memory> > > + > > +struct A {int x; int y;}; > > + > > +int main () { > > + std::shared_ptr<A> a; > > + a->x = 4; /* { dg-line deref_a } */ > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > > } */ > > + > > + return 0; > > +} > > + > > +/* { dg-begin-multiline-output "" } > > + 'int main()': events 1-2 > > + | > > + | > > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::operator->() const [with _Tp = A; > > __gnu_cxx::_Lock_policy > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > <anonymous> = > > false]': events 3-4 > > + | > > + | > > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> > > = > > false; bool <anonymous> = false]': events 5-6 > > + | > > + | > > + +--> 'std::__shared_ptr<_Tp, _Lp>::element_type* > > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 > > + | > > + | > > + <------+ > > + | > > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> > > = > > false; bool <anonymous> = false]': event 9 > > + | > > + | > > + <------+ > > + | > > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::operator->() const [with _Tp = A; > > __gnu_cxx::_Lock_policy > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > <anonymous> = > > false]': event 10 > > + | > > + | > > + <------+ > > + | > > + 'int main()': events 11-12 > > + | > > + | > > + { dg-end-multiline-output "" } */ > > \ No newline at end of file > > diff --git > > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C > > new file mode 100644 > > index 00000000000..fc271309d4e > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > default.C > > @@ -0,0 +1,21 @@ > > +/* { dg-additional-options "-O0 -fdiagnostics-plain-output > > -fdiagnostics-path-format=inline-events" } */ > > +/* { dg-skip-if "" { c++98_only } } */ > > + > > +// Must behave like -fanalyzer-trim-diagnostics=system > > + > > +#include <memory> > > + > > +struct A {int x; int y;}; > > + > > +int main () { > > + std::shared_ptr<A> a; > > + a->x = 4; /* { dg-line deref_a } */ > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > > } */ > > + > > + return 0; > > +} > > +/* { dg-begin-multiline-output "" } > > + 'int main()': events 1-2 > > + | > > + | > > + { dg-end-multiline-output "" } */ > > \ No newline at end of file > > diff --git > > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > > new file mode 100644 > > index 00000000000..a69416608da > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > never.C > > @@ -0,0 +1,44 @@ > > +/* { dg-additional-options "-fdiagnostics-plain-output > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > diagnostics=never" > > } */ > > +/* { dg-skip-if "" { c++98_only } } */ > > + > > +#include <memory> > > + > > +struct A {int x; int y;}; > > + > > +int main () { > > + std::shared_ptr<A> a; > > + a->x = 4; /* { dg-line deref_a } */ > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > > } */ > > + > > + return 0; > > +} > > + > > +/* { dg-begin-multiline-output "" } > > + 'int main()': events 1-2 > > + | > > + | > > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::operator->() const [with _Tp = A; > > __gnu_cxx::_Lock_policy > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > <anonymous> = > > false]': events 3-4 > > + | > > + | > > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> > > = > > false; bool <anonymous> = false]': events 5-6 > > + | > > + | > > + +--> 'std::__shared_ptr<_Tp, _Lp>::element_type* > > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 > > + | > > + | > > + <------+ > > + | > > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> > > = > > false; bool <anonymous> = false]': event 9 > > + | > > + | > > + <------+ > > + | > > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > <anonymous> >::operator->() const [with _Tp = A; > > __gnu_cxx::_Lock_policy > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > <anonymous> = > > false]': event 10 > > + | > > + | > > + <------+ > > + | > > + 'int main()': events 11-12 > > + | > > + | > > + { dg-end-multiline-output "" } */ > > \ No newline at end of file > > diff --git > > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C > > new file mode 100644 > > index 00000000000..22fadc84e75 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > system.C > > @@ -0,0 +1,19 @@ > > +/* { dg-additional-options "-fdiagnostics-plain-output > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > diagnostics=system" > > } */ > > +/* { dg-skip-if "" { c++98_only } } */ > > + > > +#include <memory> > > + > > +struct A {int x; int y;}; > > + > > +int main () { > > + std::shared_ptr<A> a; > > + a->x = 4; /* { dg-line deref_a } */ > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a > > } */ > > + > > + return 0; > > +} > > +/* { dg-begin-multiline-output "" } > > + 'int main()': events 1-2 > > + | > > + | > > + { dg-end-multiline-output "" } */ > > \ No newline at end of file > >
On Wed, 2023-07-26 at 11:27 +0200, Benjamin Priour wrote: > On Sat, Jul 22, 2023 at 12:04 AM David Malcolm <dmalcolm@redhat.com> > wrote: > > > On Fri, 2023-07-21 at 17:35 +0200, Benjamin Priour wrote: > > > Hi, > > > > > > Upon David's request I've joined the in progress patch to the > > > below > > > email. > > > I hope it makes more sense now. > > > > > > Best, > > > Benjamin. > > > > Thanks for posting the work-in-progress patch; it makes the idea > > clearer. > > > > Some thoughts about this: > > > > - I like the idea of defaulting to *not* showing events within > > system > > headers, which the patch achieves > > - I don't like the combination of never/system with maxdepth, in > > that > > it seems complicated and I don't think a user is likely to > > experiment > > with different depths. > > - Hence I think it would work better as a simple boolean, perhaps > > "-fanalyzer-show-events-in-system-headers" > > or somesuch? It seems like the sort of thing that we want to > > provide > > a sensible default for, but have the option of turning off for > > debugging the analyzer itself, but I don't expect an end-user to > > touch > > that option. > > > > A boolean sounds good, I will trust your experience with the end-user > here, > especially since <maxdepth> and "never" had some overlap, it could > have > been confusing. > > > > FWIW the patch seems to have been mangled somewhat via email, so I > > don't have a sense of what the actual output from patched analyzer > > looks like. What should we output to the user with -fanalyzer and > > no > > other options for the case in PR 110543? Currently, for > > https://godbolt.org/z/sb9dM9Gqa trunk emits 12 events, of which > > probably only this last one is useful: > > > > (12) dereference of NULL 'a.std::__shared_ptr_access<A, > > __gnu_cxx::_S_atomic, false, false>::operator->()' > > > > What does the output look like with your patch? > > > > The plan with this patch was to get events : > (1) entry to 'main' > (2) calling 'std::__shared_ptr_access<A, __gnu_cxx::_S_atomic, false, > false>::operator->' from 'main' > (12) dereference of NULL 'a.std::__shared_ptr_access<A, > __gnu_cxx::_S_atomic, false, false>::operator->()' > (11) returning to 'main' from 'std::__shared_ptr_access<A, > __gnu_cxx::_S_atomic, false, false>::operator->' > > This way, we get the entry and exit point to the system headers ( (2) > and > (11) ), and the actual injurious event ( (12) ). > We could however go as you suggest, with an even more succint path > and only > keep (1) and (12). I think we could still have events (11) and (12): what if for call and return events we consider the location of both the call site and the called function: we suppress if both are in a system header, but don't suppress if only one is a system header. That way by default we'd show the call into a system header and show the return from the system header, but we'd suppress all the implementation details within the system header. Does that seem like it could work? Thanks Dave > > Thanks, > Benjamin > > > > Thanks > > Dave > > > > > > > > > > > ---------- Forwarded message --------- > > > From: Benjamin Priour <priour.be@gmail.com> > > > Date: Tue, Jul 18, 2023 at 3:30 PM > > > Subject: [RFC] analyzer: Add optional trim of the analyzer > > > diagnostics > > > going too deep [PR110543] > > > To: <gcc-patches@gcc.gnu.org>, David Malcolm > > > <dmalcolm@redhat.com> > > > > > > > > > Hi, > > > > > > I'd like to request comments on a patch I am writing for > > > PR110543. > > > The goal of this patch is to reduce the noise of the analyzer > > > emitted > > > diagnostics when dealing with > > > system headers, or simply diagnostic paths that are too long. The > > > new > > > option only affects the display > > > of the diagnostics, but doesn't hinder the actual analysis. > > > > > > I've defaulted the new option to "system", thus preventing the > > > diagnostic > > > paths from showing system headers. > > > "never" corresponds to the pre-patch behavior, whereas you can > > > also > > > specify > > > an unsigned value <maxdepth> > > > that prevents paths to go deeper than <maxdepth> frames. > > > > > > fanalyzer-trim-diagnostics= > > > > Common Joined RejectNegative ToLower > > > > Var(flag_analyzer_trim_diagnostics) > > > > Init("system") > > > > -fanalyzer-trim-diagnostics=[never|system|<maxdepth>] Trim > > > > diagnostics > > > > path that are too long before emission. > > > > > > > > > > Does it sounds reasonable and user-friendly ? > > > > > > Regstrapping was a success against trunk, although one of the > > > newly > > > added > > > test case fails for c++14. > > > Note that the test case below was done with "never", thus behaves > > > exactly > > > as the pre-patch analyzer > > > on x86_64-linux-gnu. > > > > > > /* { dg-additional-options "-fdiagnostics-plain-output > > > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > > > diagnostics=never" > > > > } */ > > > > /* { dg-skip-if "" { c++98_only } } */ > > > > > > > > #include <memory> > > > > struct A {int x; int y;}; > > > > > > > > int main () { > > > > std::shared_ptr<A> a; > > > > a->x = 4; /* { dg-line deref_a } */ > > > > /* { dg-warning "dereference of NULL" "" { target *-*-* } > > > > deref_a > > > > } */ > > > > > > > > return 0; > > > > } > > > > > > > > /* { dg-begin-multiline-output "" } > > > > 'int main()': events 1-2 > > > > | > > > > | > > > > +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > > <anonymous> > > > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, > > > > > <anonymous>, > > > > <anonymous> >::operator->() const [with _Tp = A; > > > > __gnu_cxx::_Lock_policy > > > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > > > <anonymous> = > > > > false]': events 3-4 > > > > | > > > > | > > > > +--> 'std::__shared_ptr_access<_Tp, _Lp, > > > > <anonymous>, > > > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool > > > > <anonymous> = > > > > false; bool <anonymous> = false]': events 5-6 > > > > | > > > > | > > > > +--> 'std::__shared_ptr<_Tp, > > > > _Lp>::element_type* > > > > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7- > > > > 8 > > > > | > > > > | > > > > <------+ > > > > | > > > > 'std::__shared_ptr_access<_Tp, _Lp, > > > > <anonymous>, > > > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool > > > > <anonymous> = > > > > false; bool <anonymous> = false]': event 9 > > > > | > > > > | > > > > <------+ > > > > | > > > > 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > > <anonymous> > > > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, > > > > > <anonymous>, > > > > <anonymous> >::operator->() const [with _Tp = A; > > > > __gnu_cxx::_Lock_policy > > > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > > > <anonymous> = > > > > false]': event 10 > > > > | > > > > | > > > > <------+ > > > > | > > > > 'int main()': events 11-12 > > > > | > > > > | > > > > { dg-end-multiline-output "" } */ > > > > > > > > > > > > > The first events "'int main()': events 1-2" vary in c++14 (get > > > events > > > 1-3). > > > > > > > > > > > // c++14 with fully detailed output > > > > ‘int main()’: events 1-3 > > > > | > > > > | 8 | int main () { > > > > | | ^~~~ > > > > | | | > > > > | | (1) entry to ‘main’ > > > > | 9 | std::shared_ptr<A> a; > > > > | | ~ > > > > | | | > > > > | | (2) > > > > ‘a.std::shared_ptr<A>::<anonymous>.std::__shared_ptr<A, > > > > __gnu_cxx::_S_atomic>::_M_ptr’ is NULL > > > > | 10 | a->x = 4; /* { dg-line deref_a } */ > > > > | | ~~ > > > > | | | > > > > | | (3) calling ‘std::__shared_ptr_access<A, > > > > __gnu_cxx::_S_atomic, false, false>::operator->’ from ‘main’ > > > > > > > > > > whereas c++17 and posterior give > > > > > > > // c++17 with fully detailed output > > > > > > > // ./xg++ -fanalyzer > > > > ../../gcc/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > > > > diagnostics-never.C > > > > -B. -shared-libgcc -fanalyzer-trim-diagnostics=never - > > > > std=c++17 > > > > > > > ‘int main()’: events 1-2 > > > > | > > > > | 8 | int main () { > > > > | | ^~~~ > > > > | | | > > > > | | (1) entry to ‘main’ > > > > | 9 | std::shared_ptr<A> a; > > > > | 10 | a->x = 4; /* { dg-line deref_a } */ > > > > | | ~~ > > > > | | | > > > > | | (2) calling ‘std::__shared_ptr_access<A, > > > > __gnu_cxx::_S_atomic, false, false>::operator->’ from ‘main’ > > > > > > > > > > Is there a way to make dg-multiline-output check for a regex ? Or > > > would > > > checking the multiline-output only for c++17 and c++20 be > > > acceptable > > > ? > > > This divergence results from two slightly different IPA: > > > > > > // c++14 -fdump-ipa-analyzer > > > > // std::shared_ptr<A> a; becomes > > > > a.D.29392._M_ptr = 0B; > > > > a.D.29392._M_refcount._M_pi = 0B; > > > > > > > > > > whereas in c++17 > > > > > > > // c++17 -fdump-ipa-analyzer > > > > // std::shared_ptr<A> a; becomes > > > > a = {}; > > > > > > > > > > I know shared_ptr limited support is a coincidence more than a > > > feature, but > > > maybe there is still a way to make the above test case work ? > > > Otherwise, I'd fallback to a supported system function. If you > > > have > > > any > > > that you fancy, do tell me. > > > > > > Thanks, > > > Benjamin. > > > > > > --- > > > gcc/analyzer/analyzer.cc | 2 +- > > > gcc/analyzer/analyzer.h | 1 + > > > gcc/analyzer/analyzer.opt | 4 + > > > gcc/analyzer/diagnostic-manager.cc | 132 > > > ++++++++++++++++++ > > > gcc/analyzer/diagnostic-manager.h | 7 + > > > .../analyzer/fanalyzer-trim-diagnostics-0.C | 19 +++ > > > .../analyzer/fanalyzer-trim-diagnostics-1.C | 28 ++++ > > > .../analyzer/fanalyzer-trim-diagnostics-2.C | 44 ++++++ > > > .../fanalyzer-trim-diagnostics-default.C | 21 +++ > > > .../fanalyzer-trim-diagnostics-never.C | 44 ++++++ > > > .../fanalyzer-trim-diagnostics-system.C | 19 +++ > > > 11 files changed, 320 insertions(+), 1 deletion(-) > > > create mode 100644 > > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > > > create mode 100644 > > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > > > create mode 100644 > > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > > > create mode 100644 > > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > default.C > > > create mode 100644 > > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C > > > create mode 100644 > > > gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C > > > > > > diff --git a/gcc/analyzer/analyzer.cc b/gcc/analyzer/analyzer.cc > > > index 5091fb7a583..b27d8e359db 100644 > > > --- a/gcc/analyzer/analyzer.cc > > > +++ b/gcc/analyzer/analyzer.cc > > > @@ -274,7 +274,7 @@ is_named_call_p (const_tree fndecl, const > > > char > > > *funcname) > > > Compare with cp/typeck.cc: decl_in_std_namespace_p, but this > > > doesn't > > > rely on being the C++ FE (or handle inline namespaces inside > > > of > > > std). > > > */ > > > > > > -static inline bool > > > +bool > > > is_std_function_p (const_tree fndecl) > > > { > > > tree name_decl = DECL_NAME (fndecl); > > > diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h > > > index 579517c23e6..31597079153 100644 > > > --- a/gcc/analyzer/analyzer.h > > > +++ b/gcc/analyzer/analyzer.h > > > @@ -386,6 +386,7 @@ extern bool is_special_named_call_p (const > > > gcall > > > *call, > > > const char *funcname, > > > extern bool is_named_call_p (const_tree fndecl, const char > > > *funcname); > > > extern bool is_named_call_p (const_tree fndecl, const char > > > *funcname, > > > const gcall *call, unsigned int num_args); > > > +extern bool is_std_function_p (const_tree fndecl); > > > extern bool is_std_named_call_p (const_tree fndecl, const char > > > *funcname); > > > extern bool is_std_named_call_p (const_tree fndecl, const char > > > *funcname, > > > const gcall *call, unsigned int num_args); > > > diff --git a/gcc/analyzer/analyzer.opt > > > b/gcc/analyzer/analyzer.opt > > > index 2760aaa8151..78ff2c07483 100644 > > > --- a/gcc/analyzer/analyzer.opt > > > +++ b/gcc/analyzer/analyzer.opt > > > @@ -362,4 +362,8 @@ fdump-analyzer-untracked > > > Common RejectNegative Var(flag_dump_analyzer_untracked) > > > Emit custom warnings with internal details intended for analyzer > > > developers. > > > > > > +fanalyzer-trim-diagnostics= > > > +Common Joined RejectNegative ToLower > > > Var(flag_analyzer_trim_diagnostics) > > > Init("system") > > > +-fanalyzer-trim-diagnostics=[never|system|<maxdepth>] Trim > > > diagnostics > > > path that are too long before emission. > > > + > > > ; This comment is to ensure we retain the blank line above. > > > diff --git a/gcc/analyzer/diagnostic-manager.cc > > > b/gcc/analyzer/diagnostic-manager.cc > > > index cfca305d552..d946ad5bac7 100644 > > > --- a/gcc/analyzer/diagnostic-manager.cc > > > +++ b/gcc/analyzer/diagnostic-manager.cc > > > @@ -20,9 +20,11 @@ along with GCC; see the file COPYING3. If not > > > see > > > > > > #include "config.h" > > > #define INCLUDE_MEMORY > > > +#define INCLUDE_VECTOR > > > #include "system.h" > > > #include "coretypes.h" > > > #include "tree.h" > > > +#include "input.h" > > > #include "pretty-print.h" > > > #include "gcc-rich-location.h" > > > #include "gimple-pretty-print.h" > > > @@ -60,6 +62,37 @@ along with GCC; see the file COPYING3. If not > > > see > > > > > > #if ENABLE_ANALYZER > > > > > > +/* Return positive value of fanalyzer-trim-diagnostics if a > > > depth > > > was given > > > + or a negative value otherwise. */ > > > + > > > +long trim_diagnostics_depth () > > > +{ > > > + if (trim_diagnostics_system_p () || trim_diagnostics_never_p > > > ()) > > > + return -1; > > > + > > > + char *end = NULL; > > > + long depth = strtol (flag_analyzer_trim_diagnostics, &end, > > > 10); > > > + gcc_assert (end == NULL || *end == '\0'); > > > + gcc_assert (!(end == flag_analyzer_trim_diagnostics && depth > > > == > > > 0)); > > > + return depth; > > > +} > > > + > > > +/* Return true if fanalyzer-trim-diagnostics is set to > > > "system". */ > > > + > > > +bool trim_diagnostics_system_p () > > > +{ > > > + int len = strlen (flag_analyzer_trim_diagnostics); > > > + return len == 6 && strcmp (flag_analyzer_trim_diagnostics, > > > "system") == > > > 0; > > > +} > > > + > > > +/* Return true if fanalyzer-trim-diagnostics is set to "never". > > > */ > > > + > > > +bool trim_diagnostics_never_p () > > > +{ > > > + int len = strlen (flag_analyzer_trim_diagnostics); > > > + return len == 5 && strcmp (flag_analyzer_trim_diagnostics, > > > "never") == 0; > > > +} > > > + > > > namespace ana { > > > > > > class feasible_worklist; > > > @@ -2281,6 +2314,8 @@ diagnostic_manager::prune_path > > > (checker_path > > > *path, > > > path->maybe_log (get_logger (), "path"); > > > prune_for_sm_diagnostic (path, sm, sval, state); > > > prune_interproc_events (path); > > > + if (! trim_diagnostics_never_p ()) > > > + trim_diagnostic_path (path); > > > consolidate_conditions (path); > > > finish_pruning (path); > > > path->maybe_log (get_logger (), "pruned"); > > > @@ -2667,6 +2702,103 @@ > > > diagnostic_manager::prune_interproc_events > > > (checker_path *path) const > > > while (changed); > > > } > > > > > > +void > > > +diagnostic_manager::trim_diagnostic_path (checker_path *path) > > > const > > > +{ > > > + if (trim_diagnostics_system_p ()) > > > + prune_system_headers (path); > > > + else > > > + prune_events_too_deep (path); > > > +} > > > + > > > +/* Remove everything within ]callsite, IDX]. */ > > > +static void > > > +prune_within_frame (checker_path *path, int &idx) > > > +{ > > > + int nesting = 1; > > > + while (idx >= 0 && nesting != 0) > > > + { > > > + if (path->get_checker_event (idx)->is_call_p ()) > > > + nesting--; > > > + else if (path->get_checker_event (idx)->is_return_p ()) > > > + nesting++; > > > + > > > + path->delete_event (idx--); > > > + } > > > +} > > > + > > > +void > > > +diagnostic_manager::prune_system_headers (checker_path *path) > > > const > > > +{ > > > + int idx = (signed)path->num_events () - 1; > > > + while (idx >= 0) > > > + { > > > + const checker_event *event = path->get_checker_event > > > (idx); > > > + /* Prune everything between [..., system call, (...), > > > system > > > return, > > > ...]. */ > > > + if (event->is_return_p () > > > + && in_system_header_at (event->get_location ())) > > > + { > > > + int ret_idx = idx; > > > + prune_within_frame (path, idx); > > > + > > > + if (get_logger ()) > > > + { > > > + label_text desc > > > + (path->get_checker_event (idx)->get_desc (false)); > > > + log ("filtering event %i-%i:" > > > + " system header event: %s", > > > + idx, ret_idx, desc.get ()); > > > + } > > > + // delete callsite > > > + if (idx >= 0) > > > + path->delete_event (idx); > > > + } > > > + > > > + idx--; > > > + } > > > +} > > > + > > > +void > > > +diagnostic_manager::prune_events_too_deep (checker_path *path) > > > const > > > +{ > > > + long depth = 0; > > > + long maxdepth = trim_diagnostics_depth (); > > > + gcc_assert (maxdepth >= 0); > > > + int idx = (signed)path->num_events () - 1; > > > + if (idx >= 0) > > > + { > > > + depth = path->get_checker_event (0)->get_stack_depth (); > > > + maxdepth += depth; > > > + } > > > + > > > + while (idx >= 0) > > > + { > > > + const checker_event *event = path->get_checker_event > > > (idx); > > > + /* Prune everything between [..., call too deep, (...), > > > return > > > too > > > deep, ...]. */ > > > + if (event->is_return_p () > > > + && event->get_stack_depth () > maxdepth) > > > + { > > > + int ret_idx = idx; > > > + prune_within_frame (path, idx); > > > + > > > + if (get_logger ()) > > > + { > > > + label_text desc > > > + (path->get_checker_event (idx)->get_desc (false)); > > > + log ("filtering event %i-%i:" > > > + " event too deep: %s", > > > + idx, ret_idx, desc.get ()); > > > + } > > > + // delete callsite > > > + if (idx >= 0) > > > + path->delete_event (idx); > > > + } > > > + > > > + idx--; > > > + } > > > + > > > +} > > > + > > > /* Return true iff event IDX within PATH is on the same line as > > > REF_EXP_LOC. */ > > > > > > static bool > > > diff --git a/gcc/analyzer/diagnostic-manager.h > > > b/gcc/analyzer/diagnostic-manager.h > > > index 9b3e903fc5d..9afef0975be 100644 > > > --- a/gcc/analyzer/diagnostic-manager.h > > > +++ b/gcc/analyzer/diagnostic-manager.h > > > @@ -21,6 +21,10 @@ along with GCC; see the file COPYING3. If not > > > see > > > #ifndef GCC_ANALYZER_DIAGNOSTIC_MANAGER_H > > > #define GCC_ANALYZER_DIAGNOSTIC_MANAGER_H > > > > > > +long trim_diagnostics_depth (); > > > +bool trim_diagnostics_system_p (); > > > +bool trim_diagnostics_never_p (); > > > + > > > namespace ana { > > > > > > class epath_finder; > > > @@ -180,6 +184,9 @@ private: > > > state_machine::state_t state) const; > > > void update_for_unsuitable_sm_exprs (tree *expr) const; > > > void prune_interproc_events (checker_path *path) const; > > > + void trim_diagnostic_path (checker_path *path) const; > > > + void prune_system_headers (checker_path *path) const; > > > + void prune_events_too_deep (checker_path *path) const; > > > void consolidate_conditions (checker_path *path) const; > > > void finish_pruning (checker_path *path) const; > > > > > > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > > > diagnostics-0.C > > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C > > > new file mode 100644 > > > index 00000000000..ddf39ed1690 > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > 0.C > > > @@ -0,0 +1,19 @@ > > > +/* { dg-additional-options "-fdiagnostics-plain-output > > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > > diagnostics=0" } */ > > > +/* { dg-skip-if "" { c++98_only } } */ > > > + > > > +#include <memory> > > > + > > > +struct A {int x; int y;}; > > > + > > > +int main () { > > > + std::shared_ptr<A> a; > > > + a->x = 4; /* { dg-line deref_a } */ > > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } > > > deref_a > > > } */ > > > + > > > + return 0; > > > +} > > > +/* { dg-begin-multiline-output "" } > > > + 'int main()': events 1-2 > > > + | > > > + | > > > + { dg-end-multiline-output "" } */ > > > \ No newline at end of file > > > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > > > diagnostics-1.C > > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C > > > new file mode 100644 > > > index 00000000000..83a132078a1 > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > 1.C > > > @@ -0,0 +1,28 @@ > > > +/* { dg-additional-options "-fdiagnostics-plain-output > > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > > diagnostics=1" } */ > > > +/* { dg-skip-if "" { c++98_only } } */ > > > + > > > +#include <memory> > > > + > > > +struct A {int x; int y;}; > > > + > > > +int main () { > > > + std::shared_ptr<A> a; > > > + a->x = 4; /* { dg-line deref_a } */ > > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } > > > deref_a > > > } */ > > > + > > > + return 0; > > > +} > > > + > > > +/* { dg-begin-multiline-output "" } > > > + 'int main()': events 1-2 > > > + | > > > + | > > > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> > > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::operator->() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy > > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > > <anonymous> = > > > false]': event 3 > > > + | > > > + | > > > + <------+ > > > + | > > > + 'int main()': events 4-5 > > > + | > > > + | > > > + { dg-end-multiline-output "" } */ > > > \ No newline at end of file > > > diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim- > > > diagnostics-2.C > > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C > > > new file mode 100644 > > > index 00000000000..cf065929ec9 > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > 2.C > > > @@ -0,0 +1,44 @@ > > > +/* { dg-additional-options "-fdiagnostics-plain-output > > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > > diagnostics=2" } */ > > > +/* { dg-skip-if "" { c++98_only } } */ > > > + > > > +#include <memory> > > > + > > > +struct A {int x; int y;}; > > > + > > > +int main () { > > > + std::shared_ptr<A> a; > > > + a->x = 4; /* { dg-line deref_a } */ > > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } > > > deref_a > > > } */ > > > + > > > + return 0; > > > +} > > > + > > > +/* { dg-begin-multiline-output "" } > > > + 'int main()': events 1-2 > > > + | > > > + | > > > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> > > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::operator->() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy > > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > > <anonymous> = > > > false]': events 3-4 > > > + | > > > + | > > > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool > > > <anonymous> > > > = > > > false; bool <anonymous> = false]': events 5-6 > > > + | > > > + | > > > + +--> 'std::__shared_ptr<_Tp, > > > _Lp>::element_type* > > > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 > > > + | > > > + | > > > + <------+ > > > + | > > > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool > > > <anonymous> > > > = > > > false; bool <anonymous> = false]': event 9 > > > + | > > > + | > > > + <------+ > > > + | > > > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> > > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::operator->() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy > > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > > <anonymous> = > > > false]': event 10 > > > + | > > > + | > > > + <------+ > > > + | > > > + 'int main()': events 11-12 > > > + | > > > + | > > > + { dg-end-multiline-output "" } */ > > > \ No newline at end of file > > > diff --git > > > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > default.C > > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > default.C > > > new file mode 100644 > > > index 00000000000..fc271309d4e > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > default.C > > > @@ -0,0 +1,21 @@ > > > +/* { dg-additional-options "-O0 -fdiagnostics-plain-output > > > -fdiagnostics-path-format=inline-events" } */ > > > +/* { dg-skip-if "" { c++98_only } } */ > > > + > > > +// Must behave like -fanalyzer-trim-diagnostics=system > > > + > > > +#include <memory> > > > + > > > +struct A {int x; int y;}; > > > + > > > +int main () { > > > + std::shared_ptr<A> a; > > > + a->x = 4; /* { dg-line deref_a } */ > > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } > > > deref_a > > > } */ > > > + > > > + return 0; > > > +} > > > +/* { dg-begin-multiline-output "" } > > > + 'int main()': events 1-2 > > > + | > > > + | > > > + { dg-end-multiline-output "" } */ > > > \ No newline at end of file > > > diff --git > > > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > never.C > > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > never.C > > > new file mode 100644 > > > index 00000000000..a69416608da > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > never.C > > > @@ -0,0 +1,44 @@ > > > +/* { dg-additional-options "-fdiagnostics-plain-output > > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > > diagnostics=never" > > > } */ > > > +/* { dg-skip-if "" { c++98_only } } */ > > > + > > > +#include <memory> > > > + > > > +struct A {int x; int y;}; > > > + > > > +int main () { > > > + std::shared_ptr<A> a; > > > + a->x = 4; /* { dg-line deref_a } */ > > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } > > > deref_a > > > } */ > > > + > > > + return 0; > > > +} > > > + > > > +/* { dg-begin-multiline-output "" } > > > + 'int main()': events 1-2 > > > + | > > > + | > > > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> > > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::operator->() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy > > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > > <anonymous> = > > > false]': events 3-4 > > > + | > > > + | > > > + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool > > > <anonymous> > > > = > > > false; bool <anonymous> = false]': events 5-6 > > > + | > > > + | > > > + +--> 'std::__shared_ptr<_Tp, > > > _Lp>::element_type* > > > std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 > > > + | > > > + | > > > + <------+ > > > + | > > > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, > > > <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool > > > <anonymous> > > > = > > > false; bool <anonymous> = false]': event 9 > > > + | > > > + | > > > + <------+ > > > + | > > > + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> > > > > ::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, > > > <anonymous> >::operator->() const [with _Tp = A; > > > __gnu_cxx::_Lock_policy > > > _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool > > > <anonymous> = > > > false]': event 10 > > > + | > > > + | > > > + <------+ > > > + | > > > + 'int main()': events 11-12 > > > + | > > > + | > > > + { dg-end-multiline-output "" } */ > > > \ No newline at end of file > > > diff --git > > > a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > system.C > > > b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > system.C > > > new file mode 100644 > > > index 00000000000..22fadc84e75 > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics- > > > system.C > > > @@ -0,0 +1,19 @@ > > > +/* { dg-additional-options "-fdiagnostics-plain-output > > > -fdiagnostics-path-format=inline-events -fanalyzer-trim- > > > diagnostics=system" > > > } */ > > > +/* { dg-skip-if "" { c++98_only } } */ > > > + > > > +#include <memory> > > > + > > > +struct A {int x; int y;}; > > > + > > > +int main () { > > > + std::shared_ptr<A> a; > > > + a->x = 4; /* { dg-line deref_a } */ > > > + /* { dg-warning "dereference of NULL" "" { target *-*-* } > > > deref_a > > > } */ > > > + > > > + return 0; > > > +} > > > +/* { dg-begin-multiline-output "" } > > > + 'int main()': events 1-2 > > > + | > > > + | > > > + { dg-end-multiline-output "" } */ > > > \ No newline at end of file > > > >
diff --git a/gcc/analyzer/analyzer.cc b/gcc/analyzer/analyzer.cc index 5091fb7a583..b27d8e359db 100644 --- a/gcc/analyzer/analyzer.cc +++ b/gcc/analyzer/analyzer.cc @@ -274,7 +274,7 @@ is_named_call_p (const_tree fndecl, const char *funcname) Compare with cp/typeck.cc: decl_in_std_namespace_p, but this doesn't rely on being the C++ FE (or handle inline namespaces inside of std). */ -static inline bool +bool is_std_function_p (const_tree fndecl) { tree name_decl = DECL_NAME (fndecl); diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h index 579517c23e6..31597079153 100644 --- a/gcc/analyzer/analyzer.h +++ b/gcc/analyzer/analyzer.h @@ -386,6 +386,7 @@ extern bool is_special_named_call_p (const gcall *call, const char *funcname, extern bool is_named_call_p (const_tree fndecl, const char *funcname); extern bool is_named_call_p (const_tree fndecl, const char *funcname, const gcall *call, unsigned int num_args); +extern bool is_std_function_p (const_tree fndecl); extern bool is_std_named_call_p (const_tree fndecl, const char *funcname); extern bool is_std_named_call_p (const_tree fndecl, const char *funcname, const gcall *call, unsigned int num_args); diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index 2760aaa8151..78ff2c07483 100644 --- a/gcc/analyzer/analyzer.opt +++ b/gcc/analyzer/analyzer.opt @@ -362,4 +362,8 @@ fdump-analyzer-untracked Common RejectNegative Var(flag_dump_analyzer_untracked) Emit custom warnings with internal details intended for analyzer developers. +fanalyzer-trim-diagnostics= +Common Joined RejectNegative ToLower Var(flag_analyzer_trim_diagnostics) Init("system") +-fanalyzer-trim-diagnostics=[never|system|<maxdepth>] Trim diagnostics path that are too long before emission. + ; This comment is to ensure we retain the blank line above. diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index cfca305d552..d946ad5bac7 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/gcc/analyzer/diagnostic-manager.cc @@ -20,9 +20,11 @@ along with GCC; see the file COPYING3. If not see #include "config.h" #define INCLUDE_MEMORY +#define INCLUDE_VECTOR #include "system.h" #include "coretypes.h" #include "tree.h" +#include "input.h" #include "pretty-print.h" #include "gcc-rich-location.h" #include "gimple-pretty-print.h" @@ -60,6 +62,37 @@ along with GCC; see the file COPYING3. If not see #if ENABLE_ANALYZER +/* Return positive value of fanalyzer-trim-diagnostics if a depth was given + or a negative value otherwise. */ + +long trim_diagnostics_depth () +{ + if (trim_diagnostics_system_p () || trim_diagnostics_never_p ()) + return -1; + + char *end = NULL; + long depth = strtol (flag_analyzer_trim_diagnostics, &end, 10); + gcc_assert (end == NULL || *end == '\0'); + gcc_assert (!(end == flag_analyzer_trim_diagnostics && depth == 0)); + return depth; +} + +/* Return true if fanalyzer-trim-diagnostics is set to "system". */ + +bool trim_diagnostics_system_p () +{ + int len = strlen (flag_analyzer_trim_diagnostics); + return len == 6 && strcmp (flag_analyzer_trim_diagnostics, "system") == 0; +} + +/* Return true if fanalyzer-trim-diagnostics is set to "never". */ + +bool trim_diagnostics_never_p () +{ + int len = strlen (flag_analyzer_trim_diagnostics); + return len == 5 && strcmp (flag_analyzer_trim_diagnostics, "never") == 0; +} + namespace ana { class feasible_worklist; @@ -2281,6 +2314,8 @@ diagnostic_manager::prune_path (checker_path *path, path->maybe_log (get_logger (), "path"); prune_for_sm_diagnostic (path, sm, sval, state); prune_interproc_events (path); + if (! trim_diagnostics_never_p ()) + trim_diagnostic_path (path); consolidate_conditions (path); finish_pruning (path); path->maybe_log (get_logger (), "pruned"); @@ -2667,6 +2702,103 @@ diagnostic_manager::prune_interproc_events (checker_path *path) const while (changed); } +void +diagnostic_manager::trim_diagnostic_path (checker_path *path) const +{ + if (trim_diagnostics_system_p ()) + prune_system_headers (path); + else + prune_events_too_deep (path); +} + +/* Remove everything within ]callsite, IDX]. */ +static void +prune_within_frame (checker_path *path, int &idx) +{ + int nesting = 1; + while (idx >= 0 && nesting != 0) + { + if (path->get_checker_event (idx)->is_call_p ()) + nesting--; + else if (path->get_checker_event (idx)->is_return_p ()) + nesting++; + + path->delete_event (idx--); + } +} + +void +diagnostic_manager::prune_system_headers (checker_path *path) const +{ + int idx = (signed)path->num_events () - 1; + while (idx >= 0) + { + const checker_event *event = path->get_checker_event (idx); + /* Prune everything between [..., system call, (...), system return, ...]. */ + if (event->is_return_p () + && in_system_header_at (event->get_location ())) + { + int ret_idx = idx; + prune_within_frame (path, idx); + + if (get_logger ()) + { + label_text desc + (path->get_checker_event (idx)->get_desc (false)); + log ("filtering event %i-%i:" + " system header event: %s", + idx, ret_idx, desc.get ()); + } + // delete callsite + if (idx >= 0) + path->delete_event (idx); + } + + idx--; + } +} + +void +diagnostic_manager::prune_events_too_deep (checker_path *path) const +{ + long depth = 0; + long maxdepth = trim_diagnostics_depth (); + gcc_assert (maxdepth >= 0); + int idx = (signed)path->num_events () - 1; + if (idx >= 0) + { + depth = path->get_checker_event (0)->get_stack_depth (); + maxdepth += depth; + } + + while (idx >= 0) + { + const checker_event *event = path->get_checker_event (idx); + /* Prune everything between [..., call too deep, (...), return too deep, ...]. */ + if (event->is_return_p () + && event->get_stack_depth () > maxdepth) + { + int ret_idx = idx; + prune_within_frame (path, idx); + + if (get_logger ()) + { + label_text desc + (path->get_checker_event (idx)->get_desc (false)); + log ("filtering event %i-%i:" + " event too deep: %s", + idx, ret_idx, desc.get ()); + } + // delete callsite + if (idx >= 0) + path->delete_event (idx); + } + + idx--; + } + +} + /* Return true iff event IDX within PATH is on the same line as REF_EXP_LOC. */ static bool diff --git a/gcc/analyzer/diagnostic-manager.h b/gcc/analyzer/diagnostic-manager.h index 9b3e903fc5d..9afef0975be 100644 --- a/gcc/analyzer/diagnostic-manager.h +++ b/gcc/analyzer/diagnostic-manager.h @@ -21,6 +21,10 @@ along with GCC; see the file COPYING3. If not see #ifndef GCC_ANALYZER_DIAGNOSTIC_MANAGER_H #define GCC_ANALYZER_DIAGNOSTIC_MANAGER_H +long trim_diagnostics_depth (); +bool trim_diagnostics_system_p (); +bool trim_diagnostics_never_p (); + namespace ana { class epath_finder; @@ -180,6 +184,9 @@ private: state_machine::state_t state) const; void update_for_unsuitable_sm_exprs (tree *expr) const; void prune_interproc_events (checker_path *path) const; + void trim_diagnostic_path (checker_path *path) const; + void prune_system_headers (checker_path *path) const; + void prune_events_too_deep (checker_path *path) const; void consolidate_conditions (checker_path *path) const; void finish_pruning (checker_path *path) const; diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C new file mode 100644 index 00000000000..ddf39ed1690 --- /dev/null +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-0.C @@ -0,0 +1,19 @@ +/* { dg-additional-options "-fdiagnostics-plain-output -fdiagnostics-path-format=inline-events -fanalyzer-trim-diagnostics=0" } */ +/* { dg-skip-if "" { c++98_only } } */ + +#include <memory> + +struct A {int x; int y;}; + +int main () { + std::shared_ptr<A> a; + a->x = 4; /* { dg-line deref_a } */ + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ + + return 0; +} +/* { dg-begin-multiline-output "" } + 'int main()': events 1-2 + | + | + { dg-end-multiline-output "" } */ \ No newline at end of file diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C new file mode 100644 index 00000000000..83a132078a1 --- /dev/null +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-1.C @@ -0,0 +1,28 @@ +/* { dg-additional-options "-fdiagnostics-plain-output -fdiagnostics-path-format=inline-events -fanalyzer-trim-diagnostics=1" } */ +/* { dg-skip-if "" { c++98_only } } */ + +#include <memory> + +struct A {int x; int y;}; + +int main () { + std::shared_ptr<A> a; + a->x = 4; /* { dg-line deref_a } */ + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ + + return 0; +} + +/* { dg-begin-multiline-output "" } + 'int main()': events 1-2 + | + | + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = false]': event 3 + | + | + <------+ + | + 'int main()': events 4-5 + | + | + { dg-end-multiline-output "" } */ \ No newline at end of file diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C new file mode 100644 index 00000000000..cf065929ec9 --- /dev/null +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-2.C @@ -0,0 +1,44 @@ +/* { dg-additional-options "-fdiagnostics-plain-output -fdiagnostics-path-format=inline-events -fanalyzer-trim-diagnostics=2" } */ +/* { dg-skip-if "" { c++98_only } } */ + +#include <memory> + +struct A {int x; int y;}; + +int main () { + std::shared_ptr<A> a; + a->x = 4; /* { dg-line deref_a } */ + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ + + return 0; +} + +/* { dg-begin-multiline-output "" } + 'int main()': events 1-2 + | + | + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = false]': events 3-4 + | + | + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = false]': events 5-6 + | + | + +--> 'std::__shared_ptr<_Tp, _Lp>::element_type* std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 + | + | + <------+ + | + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = false]': event 9 + | + | + <------+ + | + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = false]': event 10 + | + | + <------+ + | + 'int main()': events 11-12 + | + | + { dg-end-multiline-output "" } */ \ No newline at end of file diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C new file mode 100644 index 00000000000..fc271309d4e --- /dev/null +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-default.C @@ -0,0 +1,21 @@ +/* { dg-additional-options "-O0 -fdiagnostics-plain-output -fdiagnostics-path-format=inline-events" } */ +/* { dg-skip-if "" { c++98_only } } */ + +// Must behave like -fanalyzer-trim-diagnostics=system + +#include <memory> + +struct A {int x; int y;}; + +int main () { + std::shared_ptr<A> a; + a->x = 4; /* { dg-line deref_a } */ + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ + + return 0; +} +/* { dg-begin-multiline-output "" } + 'int main()': events 1-2 + | + | + { dg-end-multiline-output "" } */ \ No newline at end of file diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C new file mode 100644 index 00000000000..a69416608da --- /dev/null +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-never.C @@ -0,0 +1,44 @@ +/* { dg-additional-options "-fdiagnostics-plain-output -fdiagnostics-path-format=inline-events -fanalyzer-trim-diagnostics=never" } */ +/* { dg-skip-if "" { c++98_only } } */ + +#include <memory> + +struct A {int x; int y;}; + +int main () { + std::shared_ptr<A> a; + a->x = 4; /* { dg-line deref_a } */ + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ + + return 0; +} + +/* { dg-begin-multiline-output "" } + 'int main()': events 1-2 + | + | + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = false]': events 3-4 + | + | + +--> 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = false]': events 5-6 + | + | + +--> 'std::__shared_ptr<_Tp, _Lp>::element_type* std::__shared_ptr<_Tp, _Lp>::get() const [with _Tp = A; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]': events 7-8 + | + | + <------+ + | + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::_M_get() const [with _Tp = A; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = false]': event 9 + | + | + <------+ + | + 'std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::element_type* std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::operator->() const [with _Tp = A; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = false]': event 10 + | + | + <------+ + | + 'int main()': events 11-12 + | + | + { dg-end-multiline-output "" } */ \ No newline at end of file diff --git a/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C new file mode 100644 index 00000000000..22fadc84e75 --- /dev/null +++ b/gcc/testsuite/g++.dg/analyzer/fanalyzer-trim-diagnostics-system.C @@ -0,0 +1,19 @@ +/* { dg-additional-options "-fdiagnostics-plain-output -fdiagnostics-path-format=inline-events -fanalyzer-trim-diagnostics=system" } */ +/* { dg-skip-if "" { c++98_only } } */ + +#include <memory> + +struct A {int x; int y;}; + +int main () { + std::shared_ptr<A> a; + a->x = 4; /* { dg-line deref_a } */ + /* { dg-warning "dereference of NULL" "" { target *-*-* } deref_a } */ + + return 0; +} +/* { dg-begin-multiline-output "" } + 'int main()': events 1-2 + | + |