Page MenuHomePhabricator

[analyzer] Configuration file for scan-build.
Needs ReviewPublic

Authored by ayartsev on Nov 12 2015, 3:47 PM.

Details

Summary

A simple INI configuration file for scan-build with options - aliases to scan-build command line arguments. This configuration file can be used to customize scan-build runs as it is done via command line. Options from this config are processed before any command line arguments so equivalent command line arguments passed to scan-build will overwrite values obtained from config. An order in which options are passed to scan-build is the same in which they occur in the config.
PR8203 is related to the issue.
In the future I intend to use this config to also customize ccc/c++-analyzer script runs (to e.g pass special includes to Clang-analyzer, e.t.c).
Please review!

Currently looking through CMakeLists.txt in scan-build folder I do not exactly know where 'CommonStuff.pm' and 'scan-build.cfg' should be placed (BinFiles/LibexecFiles/ResourceFiles). Perhaps 'CommonStuff.pm' will go to the 'ResourceFiles' dir.
I am in favor of placing 'scan-build.cfg' to 'BinFiles' even bearing in mind hypothetical usage of this config by ccc/c++-analyzer scripts.

Tested the patch under Windows/Unix, any additional testing is appreciated!

Diff Detail

Event Timeline

ayartsev updated this revision to Diff 40086.Nov 12 2015, 3:47 PM
ayartsev retitled this revision from to [analyzer] Configuration file for scan-build..
ayartsev updated this object.
ayartsev added a reviewer: zaks.anna.

CommonStuff.pm (which could stand to have a more descriptive name, libscanbuild.pm maybe?) and scan-build.cfg belong in share.

Also, how is this different from -analyzer-config?

Actually, now that I think about it more, scan-build.cfg doesn't belong in share at all. The location of an instance of it should be read in via a command-line argument, and it shouldn't be installed.

CommonStuff.pm (which could stand to have a more descriptive name, libscanbuild.pm maybe?) and scan-build.cfg belong in share.

Got it, I'll rename CommonStuff.pm and put it to share in the next patch iteration.

Also, how is this different from -analyzer-config?

-analyzer-config is used to transfers a number of options to the analyzer, while configuration file is used to customize scan-build, ccc-analyzer and c++-analyzer scripts.

Actually, now that I think about it more, scan-build.cfg doesn't belong in share at all. The location of an instance of it should be read in via a command-line argument, and it shouldn't be installed.

I intend to add a command-line argument to customize the configuration file location, but there must be some default example configuration file to describe all available options and rules (scan-build.cfg from this patch).

Thanks for looking at this!

zaks.anna edited edge metadata.Nov 18 2015, 6:14 PM

Also, how is this different from -analyzer-config?

-analyzer-config is used to transfers a number of options to the analyzer, while configuration file is
used to customize scan-build, ccc-analyzer and c++-analyzer scripts.

Could you elaborate on this a bit and explain the motivation behind this feature.

Laszlo Nagy is working on "reimplementing scan-build in python" (and improving the tool) effort. If the configuration file is a desirable feature, we should make sure it is applicable to both scan-builds.

Also, we should strive to move away from using Perl.

Adding Laszlo as a reviewer.

I think the scan-build user experience would be improved by config file! And I really like how the clang-tidy guys were doing it. (In case if you are not familiar with it, copy from the help output)

Configuration files:
  clang-tidy attempts to read configuration for each source file from a
  .clang-tidy file located in the closest parent directory of the source
  file. If any configuration options have a corresponding command-line
  option, command-line option takes precedence. The effective
  configuration can be inspected using -dump-config:

  $ clang-tidy -dump-config - --
    ---
    Checks:          '-*,some-check'
    HeaderFilterRegex: ''
    AnalyzeTemporaryDtors: false
    User:            user
    CheckOptions:
      - key:             some-check.SomeOption
        value:           'some value'
    ...

So the major difference from the current patch would be:

  • It's not a windows .ini file, but YAML/JSON syntax.
  • No need to provide default config file and check into the source repository.
  • Loading of the config file is a bit more complex logic, than taking value from a specific location.

Benefit for the user to have common usage pattern of the clang related analyzer tools. What do you think?

And I really like how the clang-tidy guys were doing it.

It would be great if the clang static analyzer config file would be similar to the clang-tidy one. We would avoid user confusion if they are consistent. Is some reuse possible? Currently, when clang-tidy calls the analyzer, can it pass the arguments to it through the config file?

Clang tidy can pass per checker configuration to the static analyzer at the moment. I think it should not be hard to extend it.

ayartsev updated this revision to Diff 42762.Dec 14 2015, 2:31 PM
ayartsev edited edge metadata.

Updated the patch following Laszlo's advises:
+ Changed config file format to YAML.
+ Dump current configuration.
+ Search for config file from the current folder and upper.

Learning Python and looking forward to the patch being committed.

Currently I try to launch scan-build over large codebases and note rooms for improvement and potentially helpful options. Find the configuration file that allows to customize ccc/c++-analyzer scripts very handy. The successful improvements to Perl implementation may be freely transferred to Python implementation in the future when the python patch gets in.

Tested the solution under Windows ant Unix.

Upd: Forgot to remove the printHash() subroutine from libscanbuild.pm. Using it locally for debug purpose.

Anton,

Have you investigated if we can reuse code from clang-tidy?

Also, the hope is that the python rewrite of scan-build will replace the current scan-build in the near future. Of cause, it still needs to be tested on Windows. I would really appreciate if you could help out with that since you are our most active contributor who uses Windows!

Unfortunately, because of this, I am not convinced that large improvements to the old scan-build script is where the effort should go at this point, especially, if the code cannot be reused by the new scan-build implementation.