[clang-tidy] Static Analyzer checker configuration options passthrough

[xazax.hun]

Summary

Starting from http://reviews.llvm.org/rL231266 options for individual static analyzer checkers can be specified.

This patch makes it possible to specify static analyzer checker options in clang tidy configuration files.

[xazax.hun]

One minor question: the options are stored in std::map<std::string, std::string>. I wonder if there is any specific reason not to use llvm::StringMap in this case.

[alexfh]

In D8164#137453, @xazax.hun wrote:

One minor question: the options are stored in std::map<std::string, std::string>. I wonder if there is any specific reason not to use llvm::StringMap in this case.

I don't remember any specific reason. You can try to replace map<> with StringMap in a separate patch.

clang-tidy/ClangTidy.cpp

207 ↗

(On Diff #21479)

I'd say that the function _sets_ static analyzer checker options, not gets them. Also, IMHO the other order of arguments would be better, as it seems more intuitive if the source comes first (it may be a matter of taste, though, and memcpy, for example, puts destination first).

210 ↗

(On Diff #21479)

I'd move the declaration into the loop.

211 ↗

(On Diff #21479)

const auto &

218 ↗

(On Diff #21479)

Can we just use the same format in clang-tidy options (prepended with 'clang-analyzer-')?

[xazax.hun]

clang-tidy/ClangTidy.cpp

218 ↗

(On Diff #21479)

Unfortunately if colons are present in the key of a config option, there will be an yaml error message: unexpected ':'. It might be possible to escape it somehow but I find it an uglier solution, so I went ahead with the period syntax. It is more consistent with the tidy checker configuration syntax anyways.

[alexfh]

clang-tidy/ClangTidy.cpp

218 ↗

(On Diff #21479)

I think, the consistency between the syntax here and in the -analyzer-config analyzer command-line option is more important than the negligible inconvenience of having to enclose option names in quotes. Option values frequently require quotes anyway.

[xazax.hun]

Changes:

• Renamed the function that sets analyzer checker configuration option and swapped the parameters
• Using the same format for checker options as the -analyzer-config analyzer command-line option.
• Minor code tweaks

[alexfh]

clang-tidy/ClangTidy.cpp

214 ↗

(On Diff #21710)

It's not a check name now. And I think, you can just remove the variable and use OptName.substr(...) as the index.

test/clang-tidy/static-analyzer-config.cpp

1 ↗

(On Diff #21710)

You can use single quotes for the -checks option and double quotes without backslash-escaping for the key:.

[alexfh]

Looks good!

clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp

ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
   for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
                                          E = ClangTidyModuleRegistry::end();
        I != E; ++I) {
     std::unique_ptr<ClangTidyModule> Module(I->instantiate());
     Module->addCheckFactories(*CheckFactories);
   }
 }
 
+static void setStaticAnalyzerCheckerOpts(const ClangTidyOptions &Opts,
+                                         AnalyzerOptionsRef AnalyzerOptions) {
+  StringRef AnalyzerPrefix(AnalyzerCheckNamePrefix);
+  for (const auto &Opt : Opts.CheckOptions) {
+    StringRef OptName(Opt.first);
+    if (!OptName.startswith(AnalyzerPrefix))
+      continue;
+    AnalyzerOptions->Config[OptName.substr(AnalyzerPrefix.size())] = Opt.second;
+  }
+}
+
 std::unique_ptr<clang::ASTConsumer>
 ClangTidyASTConsumerFactory::CreateASTConsumer(
     clang::CompilerInstance &Compiler, StringRef File) {
   // FIXME: Move this to a separate method, so that CreateASTConsumer doesn't
   // modify Compiler.
   Context.setSourceManager(&Compiler.getSourceManager());
   Context.setCurrentFile(File);
   Context.setASTContext(&Compiler.getASTContext());
ClangTidyASTConsumerFactory::CreateASTConsumer(
   // FIXME: Remove this option once clang's cfg-temporary-dtors option defaults
   // to true.
   AnalyzerOptions->Config["cfg-temporary-dtors"] =
       Context.getOptions().AnalyzeTemporaryDtors ? "true" : "false";
 
   GlobList &Filter = Context.getChecksFilter();
   AnalyzerOptions->CheckersControlList = getCheckersControlList(Filter);
   if (!AnalyzerOptions->CheckersControlList.empty()) {
+    setStaticAnalyzerCheckerOpts(Context.getOptions(), AnalyzerOptions);
     AnalyzerOptions->AnalysisStoreOpt = RegionStoreModel;
     AnalyzerOptions->AnalysisDiagOpt = PD_NONE;
     AnalyzerOptions->AnalyzeNestedBlocks = true;
     AnalyzerOptions->eagerlyAssumeBinOpBifurcation = true;
     std::unique_ptr<ento::AnalysisASTConsumer> AnalysisConsumer =
         ento::CreateAnalysisConsumer(Compiler);
     AnalysisConsumer->AddDiagnosticConsumer(
         new AnalyzerDiagnosticConsumer(Context));

clang-tools-extra/trunk/test/clang-tidy/static-analyzer-config.cpp

+// RUN: clang-tidy %s -checks='-*,clang-analyzer-unix.Malloc' -config='{CheckOptions: [{ key: "clang-analyzer-unix.Malloc:Optimistic", value: true}]}' -- | FileCheck %s
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void free(void *);
+void __attribute((ownership_returns(malloc))) *my_malloc(size_t);
+void __attribute((ownership_takes(malloc, 1))) my_free(void *);
+
+void f1() {
+  void *p = malloc(12);
+  return; 
+  // CHECK: warning: Potential leak of memory pointed to by 'p' [clang-analyzer-unix.Malloc]
+}
+
+void af2() {
+  void *p = my_malloc(12);
+  my_free(p);
+  free(p);
+  // CHECK: warning: Attempt to free released memory [clang-analyzer-unix.Malloc]
+}