* [RFC] analyzer: Add optional trim of the analyzer diagnostics going too deep [PR110543]
@ 2023-07-18 13:30 Benjamin Priour
2023-07-21 15:35 ` [WIP RFC] " Benjamin Priour
0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Priour @ 2023-07-18 13:30 UTC (permalink / raw)
To: gcc-patches, David Malcolm
[-- Attachment #1: Type: text/plain, Size: 5728 bytes --]
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [WIP RFC] analyzer: Add optional trim of the analyzer diagnostics going too deep [PR110543]
2023-07-18 13:30 [RFC] analyzer: Add optional trim of the analyzer diagnostics going too deep [PR110543] Benjamin Priour
@ 2023-07-21 15:35 ` Benjamin Priour
2023-07-21 22:04 ` David Malcolm
2023-07-22 14:47 ` Prathamesh Kulkarni
0 siblings, 2 replies; 6+ messages in thread
From: Benjamin Priour @ 2023-07-21 15:35 UTC (permalink / raw)
To: gcc-patches, David Malcolm
[-- Attachment #1: Type: text/plain, Size: 24413 bytes --]
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;
+}
+
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [WIP RFC] analyzer: Add optional trim of the analyzer diagnostics going too deep [PR110543]
2023-07-21 15:35 ` [WIP RFC] " Benjamin Priour
@ 2023-07-21 22:04 ` David Malcolm
2023-07-26 9:27 ` Benjamin Priour
2023-07-22 14:47 ` Prathamesh Kulkarni
1 sibling, 1 reply; 6+ messages in thread
From: David Malcolm @ 2023-07-21 22:04 UTC (permalink / raw)
To: Benjamin Priour, gcc-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [WIP RFC] analyzer: Add optional trim of the analyzer diagnostics going too deep [PR110543]
2023-07-21 15:35 ` [WIP RFC] " Benjamin Priour
2023-07-21 22:04 ` David Malcolm
@ 2023-07-22 14:47 ` Prathamesh Kulkarni
1 sibling, 0 replies; 6+ messages in thread
From: Prathamesh Kulkarni @ 2023-07-22 14:47 UTC (permalink / raw)
To: Benjamin Priour; +Cc: gcc-patches, David Malcolm
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [WIP RFC] analyzer: Add optional trim of the analyzer diagnostics going too deep [PR110543]
2023-07-21 22:04 ` David Malcolm
@ 2023-07-26 9:27 ` Benjamin Priour
2023-07-26 14:26 ` David Malcolm
0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Priour @ 2023-07-26 9:27 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 29992 bytes --]
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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [WIP RFC] analyzer: Add optional trim of the analyzer diagnostics going too deep [PR110543]
2023-07-26 9:27 ` Benjamin Priour
@ 2023-07-26 14:26 ` David Malcolm
0 siblings, 0 replies; 6+ messages in thread
From: David Malcolm @ 2023-07-26 14:26 UTC (permalink / raw)
To: Benjamin Priour; +Cc: gcc-patches
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
> >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-26 14:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-18 13:30 [RFC] analyzer: Add optional trim of the analyzer diagnostics going too deep [PR110543] Benjamin Priour
2023-07-21 15:35 ` [WIP RFC] " Benjamin Priour
2023-07-21 22:04 ` David Malcolm
2023-07-26 9:27 ` Benjamin Priour
2023-07-26 14:26 ` David Malcolm
2023-07-22 14:47 ` Prathamesh Kulkarni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).