Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[analyzer] Add the 'executable' config to all analyzers #4041

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Szelethus
Copy link
Collaborator

@Szelethus Szelethus commented Oct 13, 2023

By default, we look for clang, clang-tidy, gcc, etc. These binaries (and
compilers in general) are notorious for having their version number in
the binary name, making it difficult to run clang++-16 instead of
clang++. Its not impossible, but really inconvenient, hence this patch.

We debated a few approaches to this (including a CodeChecker-wide option
called --analyzer-binary, that could havve been used like this:
--analyzer-binary clangsa:/usr/bin/clang-16

We decided that since not all analyzers might have an executable binary
(like pylint), we shouldn't commit to this naming. We also decided that
this is a flag that should be implemented by individual analyzer
plugins.

The PR adds the 'executable' config to all currently supported
analyzers.

I also needed to make adjustments to CodeChecker analyzers, since the
new config must be visible, even if we couldn't get hold of an analyzer
binary that we support. The list of changes:

  • Deprecate both --all and --details, because they were more
    confusing than useful
  • We list all supported (not available) analyzers by default, but print
    a warning message when its not available.
  • We used to query the version of each analyzer binary in a uniform way,
    but clang, cppcheck, gcc prints their version using different flags,
    and in different formats. I changed this to call the analyzer plugins'
    get_binary_version method (which I also implemented in this patch).

I don't like enlarging patches too much, but this is how the PR turned
out. Its honestly not my proudest work (you can only do so much in so
little time), but I think the interface is fine, even if the
implementation can use a little improvement.

Closes #3898.

@Szelethus Szelethus added the analyzer 📈 Related to the analyze commands (analysis driver) label Oct 13, 2023
@Szelethus Szelethus added this to the release 6.23.0 milestone Oct 13, 2023
@bruntib
Copy link
Contributor

bruntib commented Oct 13, 2023

I think, that even if we merge this PR now, this options should be introduced as an analyzer config option by the final 6.23.0 release. The analyzer binary could be given through --analyzer-config.

@Szelethus
Copy link
Collaborator Author

This is kind of a dealbreaker for gcc support. It is in our best interest that the community has at least a somewhat convenient way to make CodeChecker accept gcc-13.

@Szelethus Szelethus changed the title [analyzer] Add the --analyzer-binary option [analyzer] Add the 'executable' config to all analyzers Oct 16, 2023
Copy link
Member

@dkrupp dkrupp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please list the analyzers even if no binary available or supported.
Please list the config options even if no analyzer binary found.

@@ -700,6 +707,11 @@ def construct_config_handler(cls, args):
isinstance(args.analyzer_config, list):
for cfg in args.analyzer_config:
if cfg.analyzer == cls.ANALYZER_NAME:
if cfg.option == 'executable':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The probelm with this patch is that I can only specify the gcc:executable analyzer config if CodeChecker found already a gcc analyzer executable. Witch defies its purpos. Catch of 22...

The analyzer configs should be listable even if I don't have an analyzer binary. Analyzer plugins exist no matter if I have the analyzer on my manchine.

Please implement the following behaviour.

Let's assume I have no gcc in the PATH:

CodeChecker analyzers
clangsa 12.0.0 /usr/bin/clang
clang-tidy 11.0.0 /usr/bin/clang-tidy
cppcheck no analyzer binary found

CodeChecker --analyzer-config gcc

gcc:executable Use the specified analyzer binary. This supersedes any other method CodeChecker might use to get hold of one.

By default, we look for clang, clang-tidy, gcc, etc. These binaries (and
compilers in general) are notorious for having their version number in
the binary name, making it difficult to run clang++-16 instead of
clang++. Its not impossible, but really inconvenient, hence this patch.

We debated a few approaches to this (including a CodeChecker-wide option
called --analyzer-binary, that could havve been used like this:
  --analyzer-binary clangsa:/usr/bin/clang-16

We decided that since not all analyzers might have an executable binary
(like pylint), we shouldn't commit to this naming. We also decided that
this is a flag that should be implemented by individual analyzer
plugins.

The PR adds the 'executable' config to all currently supported
analyzers.

I also needed to make adjustments to `CodeChecker analyzers`, since the
new config must be visible, even if we couldn't get hold of an analyzer
binary that we support. The list of changes:
* Deprecate both `--all` and `--details`, because they were more
  confusing than useful
* We list all supported (not available) analyzers by default, but print
  a warning message when its not available.
* We used to query the version of each analyzer binary in a uniform way,
  but clang, cppcheck, gcc prints their version using different flags,
  and in different formats. I changed this to call the analyzer plugins'
  get_binary_version method (which I also implemented in this patch).

I don't like enlarging patches too much, but this is how the PR turned
out. Its honestly not my proudest work (you can only do so much in so
little time), but I think the interface is fine, even if the
implementation can use a little improvement.

Closes Ericsson#3898.
@Szelethus
Copy link
Collaborator Author

Szelethus commented Oct 20, 2023

Some examples for CodeChecker analyzers:

gcc-13 is available:

$ CodeChecker analyzers -o table
---------------------------------------------------------------------------------------
Name       | Path                                                             | Version
---------------------------------------------------------------------------------------
clangsa    | (my absolute path)/llvm-project/releaseBuild/bin/clang-17        | 16.0.4 
clang-tidy | (my absolute path)/llvm-project/releaseBuild/bin/clang-tidy      | 16.0.4 
cppcheck   | /usr/bin/cppcheck                                                | 2.7    
gcc        | /usr/lib/ccache/gcc-13                                           | 13.1.0 
---------------------------------------------------------------------------------------

gcc-13 is NOT available:

$ CodeChecker analyzers -o table
---------------------------------------------------------------------------------------
Name       | Path                                                             | Version
---------------------------------------------------------------------------------------
clangsa    | (my absolute path)/llvm-project/releaseBuild/bin/clang-17        | 16.0.4 
clang-tidy | (my absolute path)/llvm-project/releaseBuild/bin/clang-tidy      | 16.0.4 
cppcheck   | /usr/bin/cppcheck                                                | 2.7    
gcc        | /usr/lib/ccache/gcc                                              | 11.4.0 
---------------------------------------------------------------------------------------
[WARNING 2023-10-20 12:17] - Can't analyze with 'gcc': Incompatible version: GCC binary found is too old at v11.4.0; minimum version is 13.0.0.

gcc-13 is available:

$ CodeChecker analyzers --analyzer-config=gcc -o table # gcc-13 is available
------------------------------------------------------------------------------------------------------------------------------
Option         | Description                                                                                                  
------------------------------------------------------------------------------------------------------------------------------
gcc:executable | Use the specified analyzer binary. This supersedes any other method CodeChecker might use to get hold of one.
------------------------------------------------------------------------------------------------------------------------------

gcc-13 is NOT available:

$ CodeChecker analyzers --analyzer-config=gcc -o table
------------------------------------------------------------------------------------------------------------------------------
Option         | Description                                                                                                  
------------------------------------------------------------------------------------------------------------------------------
gcc:executable | Use the specified analyzer binary. This supersedes any other method CodeChecker might use to get hold of one.
------------------------------------------------------------------------------------------------------------------------------
[WARNING 2023-10-20 12:18] - Can't analyze with 'gcc': Incompatible version: GCC binary found is too old at v11.4.0; minimum version is 13.0.0.
$ CodeChecker analyzers --analyzer-config=cppcheck -o table
-------------------------------------------------------------------------------------------------------------------------------------
Option                | Description                                                                                                  
-------------------------------------------------------------------------------------------------------------------------------------
cppcheck:executable   | Use the specified analyzer binary. This supersedes any other method CodeChecker might use to get hold of one.
cppcheck:addons       | A list of cppcheck addon files.                                                                              
cppcheck:libraries    | A list of cppcheck library definiton files.                                                                  
cppcheck:platform     | The platform configuration .xml file.                                                                        
cppcheck:inconclusive | Enable inconclusive reports.                                                                                 
-------------------------------------------------------------------------------------------------------------------------------------

@dkrupp
Copy link
Member

dkrupp commented Oct 20, 2023

Something does not seem to be right
CodeChecker analyze ./compile_commands.json --analyzer-config gcc:executable=/usr/bin/gcc-13 -o reports_gcc

I get this error Analyzer 'gcc' is enabled but CodeChecker is failed to execute analysis with it: 'Incompatible version: GCC binary found is too old at v9.4.0; minimum version is 13.0.0.'. Please check your 'PATH' environment variable and the 'config/package_layout.json' file!

Also, make sure the first thing we do is to parse the 'executable'
config.
Copy link
Member

@dkrupp dkrupp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works as expected:

CodeChecker analyze ./compile_commands.json -e gcc --analyzer-config gcc:executable=/usr/bin/gcc-13 -o reports_gcc

However the user cannot list gcc provided checkers or fetch the analyzer options with alternative executable path.
this does not work:

#listing gcc checkers does not work
CodeChecker checkers --analyzer-config=clang:executable=/usr/bin/gcc-13

#listing clang analyzer options does not work
CodeChecker analyzers --analyzer-config=clang:executable=/usr/bin/clang-11  --analyzer-config clang

Instead of the above introduce an environment variable which enables defining the analyzers executables

CC_ANALYZER_BIN=gcc:/usr/bin/gcc-13;clangsa:/usr/bin/clang-11

CodeCheckers analyzers output looks good like in this patch.
Extend it with the following:

CodeChecker analyzers
 clangsa /local/workspace/llvm-project/build/bin/clang-16   18.0.0
 clang-tidy /local/workspace/llvm-project/build/bin/clang-tidy 18.0.0
 cppcheck /usr/bin/cppcheck                                  1.90
 gcc /usr/bin/x86_64-linux-gnu-g++-9                    9.4.0
[WARNING 2023-10-24 12:15] - Can't analyze with 'gcc': Incompatible version: GCC binary found is too old at v9.4.0; minimum version is 13.0.0.
**[WARNING 2023-10-24 12:15] - You can specify an alternative analyzer binary in the CC_ANALYZER_BIN environment variable.**

For now, please remove the [gcc/clang/clang-tidy/cppcheck]:executable analyzer options.

@Szelethus Szelethus marked this pull request as draft October 24, 2023 12:06
Szelethus added a commit to Szelethus/codechecker that referenced this pull request Oct 24, 2023
This environmental variable can be used the specify the absolute path of
an analyzer. Similar to Ericsson#4041, but this is the only solution we can
realistically get though the door.
Szelethus added a commit to Szelethus/codechecker that referenced this pull request Oct 24, 2023
This environmental variable can be used the specify the absolute path of
an analyzer. Similar to Ericsson#4041, but this is the only solution we can
realistically get though the door.
Szelethus added a commit to Szelethus/codechecker that referenced this pull request Oct 26, 2023
This environmental variable can be used the specify the absolute path of
an analyzer. Similar to Ericsson#4041, but this is the only solution we can
realistically get though the door.
@dkrupp dkrupp removed this from the release 6.23.0 milestone Nov 9, 2023
@whisperity whisperity added the WIP 💣 Work In Progress label Nov 22, 2023
@whisperity whisperity added this to the release 6.24.0 milestone Dec 10, 2023
@bruntib bruntib removed this from the release 6.24.0 milestone Mar 21, 2024
@bruntib bruntib added this to the release 6.25.0 milestone Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer 📈 Related to the analyze commands (analysis driver) WIP 💣 Work In Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configurable location of an analyzer
4 participants