[anayzer] Basic checker option validation

[xazax.hun]

Summary

This patch adds basic validation to checker options in the static analyzer.
This validation triggers a diagnostic error in case an option is set for a nonexistent checker or package.
It is not an error to set an option for a checker that is not enabled. I think disallowing that would be too restrictive and would have negative impact on user friendliness.
Once this validation is in place, the only one left is to make sure all package and checker names are identifiers (I think that check should be done at checker registration.) and
make sure the name of the option is an identifier as well.

[xazax.hun]

Small reminder just in case this got forgotten.

[xazax.hun]

Update to the latest trunk.
Minor code style improvements.

[zaks.anna]

Please,

Tighten the check so that we reject the prefixes of the full checker name that are not valid package names.

Also, add tests!

[xazax.hun]

• Made the checker/package name validation more strict (do not accept any prefixes)
• Added some tests

cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h

 // the analyzer infrastructure can change a lot between releases.
 // Unfortunately, this string has to be statically embedded in each plugin,
 // so we can't just use the functions defined in Version.h.
 #include "clang/Basic/Version.h"
 #define CLANG_ANALYZER_API_VERSION_STRING CLANG_VERSION_STRING
 #endif
 
 namespace clang {
+class DiagnosticsEngine;
+class AnalyzerOptions;
+
 namespace ento {
 
 class CheckerOptInfo;
 
 /// Manages a set of available checkers for running a static analysis.
 /// The checkers are organized into packages by full name, where including
 /// a package will recursively include all subpackages and checkers within it.
 /// For example, the checker "core.builtin.NoReturnFunctionChecker" will be
public:
 
   /// Initializes a CheckerManager by calling the initialization functions for
   /// all checkers specified by the given CheckerOptInfo list. The order of this
   /// list is significant; later options can be used to reverse earlier ones.
   /// This can be used to exclude certain checkers in an included package.
   void initializeManager(CheckerManager &mgr,
                          SmallVectorImpl<CheckerOptInfo> &opts) const;
 
+  /// Check if every option corresponds to a specific checker or package.
+  void validateCheckerOptions(const AnalyzerOptions &opts,
+                              DiagnosticsEngine &diags) const;
+
   /// Prints the name and description of all checkers in this registry.
   /// This output is not intended to be machine-parseable.
   void printHelp(raw_ostream &out, size_t maxNameChars = 30) const ;
 
 private:
   mutable CheckerInfoList Checkers;
   mutable llvm::StringMap<size_t> Packages;
 };
 
 } // end namespace ento
 } // end namespace clang
 
 #endif

cfe/trunk/lib/StaticAnalyzer/Core/CheckerRegistry.cpp

 //===--- CheckerRegistry.cpp - Maintains all available checkers -*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/CheckerRegistry.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/CheckerOptInfo.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
 using namespace ento;
 
 static const char PackageSeparator = '.';
 typedef llvm::SetVector<const CheckerRegistry::CheckerInfo *> CheckerInfoSet;
void CheckerRegistry::initializeManager(CheckerManager &checkerMgr,
   // Initialize the CheckerManager with all enabled checkers.
   for (CheckerInfoSet::iterator
          i = enabledCheckers.begin(), e = enabledCheckers.end(); i != e; ++i) {
     checkerMgr.setCurrentCheckName(CheckName((*i)->FullName));
     (*i)->Initialize(checkerMgr);
   }
 }
 
+void CheckerRegistry::validateCheckerOptions(const AnalyzerOptions &opts,
+                                             DiagnosticsEngine &diags) const {
+  for (auto &config : opts.Config) {
+    size_t pos = config.getKey().find(':');
+    if (pos == StringRef::npos)
+      continue;
+
+    bool hasChecker = false;
+    StringRef checkerName = config.getKey().substr(0, pos);
+    for (auto &checker : Checkers) {
+      if (checker.FullName.startswith(checkerName) &&
+          (checker.FullName.size() == pos || checker.FullName[pos] == '.')) {
+        hasChecker = true;
+        break;
+      }
+    }
+    if (!hasChecker) {
+      diags.Report(diag::err_unknown_analyzer_checker) << checkerName;
+    }
+  }
+}
+
 void CheckerRegistry::printHelp(raw_ostream &out,
                                 size_t maxNameChars) const {
   // FIXME: Alphabetical sort puts 'experimental' in the middle.
   // Would it be better to name it '~experimental' or something else
   // that's ASCIIbetically last?
   std::sort(Checkers.begin(), Checkers.end(), checkerNameLT);
 
   // FIXME: Print available packages.

cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp

ento::createCheckerManager(AnalyzerOptions &opts, const LangOptions &langOpts,
   SmallVector<CheckerOptInfo, 8> checkerOpts;
   for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
     const std::pair<std::string, bool> &opt = opts.CheckersControlList[i];
     checkerOpts.push_back(CheckerOptInfo(opt.first.c_str(), opt.second));
   }
 
   ClangCheckerRegistry allCheckers(plugins, &diags);
   allCheckers.initializeManager(*checkerMgr, checkerOpts);
+  allCheckers.validateCheckerOptions(opts, diags);
   checkerMgr->finishedCheckerRegistration();
 
   for (unsigned i = 0, e = checkerOpts.size(); i != e; ++i) {
     if (checkerOpts[i].isUnclaimed()) {
       diags.Report(diag::err_unknown_analyzer_checker)
           << checkerOpts[i].getName();
       diags.Report(diag::note_suggest_disabling_all_checkers);
     }

cfe/trunk/test/Analysis/analyzer-checker-config.c

+// RUN: not %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-config unix.mallo:Optimistic=true 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-config uni:Optimistic=true 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-config uni.:Optimistic=true 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-config ..:Optimistic=true 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-config unix.:Optimistic=true 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-config unrelated:Optimistic=true 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-config unix.Malloc:Optimistic=true
+
+// Just to test clang is working.
+# foo
+
+// CHECK: error: