public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [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
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).