CodeChecker disables apiModeling by default *CRITICAL*

[Szelethus]

Describe the bug CodeChecker runs the analyzer with the flag --analyzer-no-default-checks, resulting in apiModeling being disabled by default.

CodeChecker version

---------------------------------------------------------------
Kind                 | Version                                 
---------------------------------------------------------------
Base package version | 6.10.1                                  
Package build date   | 2019-08-22T21:20                        
Git commit ID (hash) | 65d42ed762814d942546a27121b8fc4fefdd655f
Git tag information  | 6.10.1                                  
---------------------------------------------------------------

CodeChecker web version:
------------------------------------------------------------------------
Kind                          | Version                                 
------------------------------------------------------------------------
Base package version          | 6.10.1                                  
Package build date            | 2019-08-22T21:20                        
Git commit ID (hash)          | 65d42ed762814d942546a27121b8fc4fefdd655f
Git tag information           | 6.10.1                                  
Configuration schema          | v6.0                                    
Database schema               | v6.0                                    
Server supported API (Thrift) | 6.20                                    
Client API (Thrift)           | 6.20                                    
------------------------------------------------------------------------

Expected behavior The way CodeChecker invokes the analyzer is totally out of whack. There is no sane explanation for what we're doing right now. I find the addition of --analyzer-no-default-checks in particular extremely concerning.

I expect CodeChecker to lean on the analyzer configuring itself a lot more. Don't use --analyzer-no-default-checks. Don't disable anything purely because it wasn't explicitly enabled.

[Szelethus]

I pinned this because I'm legitimately worried. apiModeling is a super impactful package, and it seems like we've been missing out on it for a long while. This also implies that we haven't tested an important part of the analyzer well enough in our own playground. I also feel uncomfortable with our nonsensical invocation -- I see how it was needed back then due to how dependencies work (not that it solved the issue, but I feel for the desire to do something about it), but it's well overdue to get this right.

[Szelethus]

I've been digging deeper into this and I'm fairly positive that considering the current state of the analyzer, the way we enable/disable checkers is fundamentally flawed, but I'm none the wiser as to how we should go about this.

The thing that bugs me (haha) the most is .config/config.json, and that I don't think it should exist at all. The analyzer categorizes its checker according to verbosity and usability: incomplete checkers are in alpha, checkers that report code smell (like UninitializedObjectChecker) are in optin, and checkers that are essentially a must lie in core and apiModeling. On everything else, it makes a reasonable judgement on whether it should be enabled.

I'd like to draw you attention to the following line:

"optin.portability.UnixAPI" : ["default", "sensitive", "extreme", "portability"],

So this checker fits all of the following descriptions, according to the documentation:

• High-quality standard check with a low false positive rate
• Check with a low false positive rate
• Checks with a manageable false positive rate

Why is there a need to specify these individually? Shouldn't extreme just imply the rest? Why are we doing this at all? Shouldn't we just trust the analyzer?

Well, unfortunately, clang-tidy doesn't have the same approach, and there (aside from that extreme really should imply its subcategories) I do seriously feel the need for this system.

[gyorb]

Just to clear things up.

apiModeling checkers are enabled (except one which will be fixed in #2292) in the profiles so they are turned on during the analysis.

The hardcoded checkers produce/produced warnings even if the user did not enabled them which is confusing during report management, why should I get reports from some checkers I did not enable? --analyzer-no-default-checks was the only solution to disable all checkers from reporting because core checkers are hardcoded in clang and there was no clear separation between the modeling checkers and checkers providing diagnostics. With that the user can actually fine tune which checkers to run. I know this is not ideal but clang does not supported anything better until clang v9.

Hardcoding the modelling checkers should be fine if they do not produce reports so the user can enable/disable the diagnostics for the non modeling checkers. If that works in clang we can remove the --analyzer-no-default-checks option and change how the checkers are enabled right now (everything is disabled by default and based on the command line and profile configuration the required checkers are enabled.)

The checker profiles is an analyzer independent selection of checkers for easier usage. If the user does not want to use the profiles they can disable them and create a list of specific checkers which they want to enable/disable it is optional to use them.

[Szelethus]

Just to clear things up.

apiModeling checkers are enabled (except one which will be fixed in #2292) in the profiles so they are turned on during the analysis.

Are you sure about that? I just checked a fairly recent version of CodeChecekr a couple days ago with -analyzer-list-enabled-checkers, and not a single apiModeling checker showed up.

The hardcoded checkers produce/produced warnings even if the user did not enabled them which is confusing during report management, why should I get reports from some checkers I did not enable? --analyzer-no-default-checks was the only solution to disable all checkers from reporting because core checkers are hardcoded in clang and there was no clear separation between the modeling checkers and checkers providing diagnostics. With that the user can actually fine tune which checkers to run.

apiModeling doesn't emit any diagnostics.

I know this is not ideal but clang does not supported anything better until clang v9.

Hardcoding the modelling checkers should be fine if they do not produce reports so the user can enable/disable the diagnostics for the non modeling checkers. If that works in clang we can remove the --analyzer-no-default-checks option and change how the checkers are enabled right now (everything is disabled by default and based on the command line and profile configuration the required checkers are enabled.)

I suspect --analyzer-no-default-checks is redundant with our current "disable everything that isn't enabled" approach. If the analyzer was crafty enough to hide a checker in a way that it doesn't show up in either of those lists, it has a very strong reason to do so.

The checker profiles is an analyzer independent selection of checkers for easier usage. If the user does not want to use the profiles they can disable them and create a list of specific checkers which they want to enable/disable it is optional to use them.

My criticism of that system was kinda dumb, I retract it :^)

[gyorb]

with the CodeChecker checkers --details |grep api command you can see that only the apiModeling.TrustNonnull checker was not enabled the other two are in the default profile so they are enabled by default

[dkrupp]

PR https://github.com/Ericsson/codechecker/pull/2305 solved this