Index: clang/include/clang/Driver/Options.td =================================================================== --- clang/include/clang/Driver/Options.td +++ clang/include/clang/Driver/Options.td @@ -4287,6 +4287,11 @@ HelpText<"Disable all static analyzer checks">, MarshallingInfoFlag<"AnalyzerOpts->DisableAllCheckers">; +def analyzer_tidy_checker : Separate<["-"], "analyzer-tidy-checker">, + HelpText<"Add clang-tidy check string (enable, -disable, comma-separated)">; +def analyzer_tidy_checker_EQ : Joined<["-"], "analyzer-tidy-checker=">, + Alias; + def analyzer_checker_help : Flag<["-"], "analyzer-checker-help">, HelpText<"Display the list of analyzer checkers that are available">, MarshallingInfoFlag<"AnalyzerOpts->ShowCheckerHelp">; Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -202,6 +202,9 @@ /// Vector of checker/package names which will not emit warnings. std::vector SilencedCheckersAndPackages; + /// Vector of clang-tidy check flags. + std::vector TidyChecks; + /// A key-value table of use-specified configuration values. // TODO: This shouldn't be public. ConfigTable Config; Index: clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h =================================================================== --- clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h +++ clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h @@ -16,6 +16,7 @@ #include "clang/AST/ASTConsumer.h" #include "clang/Basic/LLVM.h" +#include "clang/Frontend/MultiplexConsumer.h" #include #include @@ -31,8 +32,11 @@ class CheckerManager; class CheckerRegistry; -class AnalysisASTConsumer : public ASTConsumer { +class AnalysisASTConsumer : public MultiplexConsumer { public: + AnalysisASTConsumer(std::vector> V) + : MultiplexConsumer(std::move(V)) {} + virtual void AddDiagnosticConsumer(PathDiagnosticConsumer *Consumer) = 0; /// This method allows registering statically linked custom checkers that are Index: clang/lib/Driver/ToolChains/Clang.cpp =================================================================== --- clang/lib/Driver/ToolChains/Clang.cpp +++ clang/lib/Driver/ToolChains/Clang.cpp @@ -2960,6 +2960,12 @@ // Default nullability checks. CmdArgs.push_back("-analyzer-checker=nullability.NullPassedToNonnull"); CmdArgs.push_back("-analyzer-checker=nullability.NullReturnedFromNonnull"); + + // Default clang-tidy checks. + CmdArgs.push_back("-analyzer-tidy-checker=bugprone-assert-side-effect"); + CmdArgs.push_back("-analyzer-tidy-checker=bugprone-infinite-loop"); + CmdArgs.push_back( + "-analyzer-tidy-checker=bugprone-move-forwarding-reference"); } // Set the output format. The default is plist, for (lame) historical reasons. Index: clang/lib/Frontend/CompilerInvocation.cpp =================================================================== --- clang/lib/Frontend/CompilerInvocation.cpp +++ clang/lib/Frontend/CompilerInvocation.cpp @@ -634,6 +634,12 @@ IsEnabled); } + Opts.TidyChecks.clear(); + for (const Arg *A : Args.filtered(OPT_analyzer_tidy_checker)) { + StringRef CheckList = A->getValue(); + Opts.TidyChecks.emplace_back(CheckList); + } + // Go through the analyzer configuration options. for (const auto *A : Args.filtered(OPT_analyzer_config)) { Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp =================================================================== --- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -16,15 +16,20 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Analysis/Analyses/LiveVariables.h" #include "clang/Analysis/CFG.h" #include "clang/Analysis/CallGraph.h" #include "clang/Analysis/CodeInjector.h" #include "clang/Analysis/PathDiagnostic.h" #include "clang/Analysis/PathDiagnosticConsumers.h" +#include "clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h" +#include "clang/Basic/FileManager.h" #include "clang/Basic/SourceManager.h" #include "clang/CrossTU/CrossTranslationUnit.h" #include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/MultiplexConsumer.h" +#include "clang/Frontend/TextDiagnosticPrinter.h" #include "clang/Lex/Preprocessor.h" #include "clang/Rewrite/Core/Rewriter.h" #include "clang/StaticAnalyzer/Checkers/LocalCheckers.h" @@ -38,21 +43,35 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include "llvm/Support/Program.h" +#include "llvm/Support/Registry.h" +#include "llvm/Support/SmallVectorMemoryBuffer.h" #include "llvm/Support/Timer.h" #include "llvm/Support/raw_ostream.h" #include #include #include +#include "../../clang-tools-extra/clang-tidy/ClangTidyCheck.h" +#include "../../clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h" +#include "../../clang-tools-extra/clang-tidy/ClangTidyForceLinker.h" +#include "../../clang-tools-extra/clang-tidy/ClangTidyModule.h" +#include "../../clang-tools-extra/clang-tidy/ClangTidyModuleRegistry.h" +#include "../../clang-tools-extra/clang-tidy/ClangTidyOptions.h" +#include "../../clang-tools-extra/clang-tidy/ClangTidyProfiling.h" + +LLVM_INSTANTIATE_REGISTRY(clang::tidy::ClangTidyModuleRegistry) + using namespace clang; using namespace ento; +using namespace tidy; +using namespace llvm; #define DEBUG_TYPE "AnalysisConsumer" STATISTIC(NumFunctionTopLevel, "The # of functions at top level."); STATISTIC(NumFunctionsAnalyzed, - "The # of functions and blocks analyzed (as top level " - "with inlining turned on)."); + "The # of functions and blocks analyzed (as top level " + "with inlining turned on)."); STATISTIC(NumBlocksInAnalyzedFunctions, "The # of basic blocks in the analyzed functions."); STATISTIC(NumVisitedBlocksInAnalyzedFunctions, @@ -66,7 +85,7 @@ namespace { -class AnalysisConsumer : public AnalysisASTConsumer, +class AnalysisConsumer : public ASTConsumer, public RecursiveASTVisitor { enum { AM_None = 0, @@ -83,7 +102,7 @@ std::vector> CheckerRegistrationFns; public: - ASTContext *Ctx; + ASTContext &Ctx; Preprocessor &PP; const std::string OutDir; AnalyzerOptionsRef Opts; @@ -117,10 +136,96 @@ /// translation unit. FunctionSummariesTy FunctionSummaries; +private: // Clang-tidy integration. + ClangTidyGlobalOptions ClangTidyGlobalOpts{}; + std::unique_ptr ClangTidyCtx; + std::unique_ptr ClangTidyProfile; + + // Provide bug types and categories for clang-tidy, also convert + // check names to checker names. + struct OwningBugTypeInfo { + const std::string CheckName; + const std::string BugType; + const std::string BugCategory; + + PathDiagnosticConverterDiagnosticConsumer::BugTypeInfo toBugTypeInfo() { + // Take a non-owning reference to the same data. Very fast. + return PathDiagnosticConverterDiagnosticConsumer::BugTypeInfo{ + CheckName, BugType, BugCategory}; + } + + OwningBugTypeInfo(std::string &&CheckName, std::string &&BugType, + std::string &&BugCategory) + : CheckName(std::move(CheckName)), BugType(std::move(BugType)), + BugCategory(std::move(BugCategory)) {} + + // Move-only. We don't want the cache to copy strings and then delete + // original strings that we have references to. + OwningBugTypeInfo(const OwningBugTypeInfo &) = delete; + OwningBugTypeInfo(OwningBugTypeInfo &&) = default; + OwningBugTypeInfo &operator=(const OwningBugTypeInfo &) = delete; + }; + + std::map BugTypeCache; + + // Fold clang-tidy check categories into common static analyzer categories. + std::map OverriddenCategories{ + {"bugprone-assert-side-effect", categories::LogicError}, + {"bugprone-infinite-loop", categories::LogicError}, + {"bugprone-move-forwarding-reference", categories::CXXMoveSemantics}, + }; + + PathDiagnosticConverterDiagnosticConsumer::BugTypeInfo + defaultBugTypeInfoProvider(const Diagnostic &Info) { + unsigned ID = Info.getID(); + auto I = BugTypeCache.find(ID); + if (I != BugTypeCache.end()) + return I->second.toBugTypeInfo(); + + std::string CheckName = ClangTidyCtx->getCheckName(Info.getID()); + assert(!CheckName.empty() && + "No check name available for clang-tidy diagnostic"); + // Eg., Clang-Tidy [bugprone-assert-side-effect]. + std::string Bugtype = "Clang-Tidy [" + CheckName + "]"; + + std::string Category; + auto J = OverriddenCategories.find(CheckName); + if (J != OverriddenCategories.end()) { + Category = J->second; + } else { + auto I = CheckName.find('-'); + assert(I != std::string::npos && "Clang-tidy check name isn't dashed"); + std::string CheckCategory = + (I != std::string::npos) ? CheckName.substr(0, I) : CheckName; + // Eg., Clang-Tidy [bugprone] + Category = "Clang-Tidy [" + CheckCategory + "]"; + } + + auto IF = BugTypeCache.insert(std::make_pair( + ID, OwningBugTypeInfo(std::move(CheckName), std::move(Bugtype), + std::move(Category)))); + return IF.first->second.toBugTypeInfo(); + } + + PathDiagnosticConverterDiagnosticConsumer ClangTidyDiagConsumer{ + PathConsumers, + [this](const Diagnostic &Info) { + return defaultBugTypeInfoProvider(Info); + }, + /*ShouldDisplayNotesAsEvents=*/true, + /*ShouldTrimCheckerName=*/true}; + DiagnosticsEngine ClangTidyDiagEngine{ + new DiagnosticIDs, new DiagnosticOptions, &ClangTidyDiagConsumer, + /*ShouldOwnClient=*/false}; + ClangTidyCheckFactories ClangTidyChkFactories{}; + std::unique_ptr ClangTidyMatchFinder; + std::vector> ClangTidyChecks{}; + +public: AnalysisConsumer(CompilerInstance &CI, const std::string &outdir, AnalyzerOptionsRef opts, ArrayRef plugins, CodeInjector *injector) - : RecVisitorMode(0), RecVisitorBR(nullptr), Ctx(nullptr), + : RecVisitorMode(0), RecVisitorBR(nullptr), Ctx(CI.getASTContext()), PP(CI.getPreprocessor()), OutDir(outdir), Opts(std::move(opts)), Plugins(plugins), Injector(injector), CTU(CI) { DigestAnalyzerOptions(); @@ -207,11 +312,10 @@ } void Initialize(ASTContext &Context) override { - Ctx = &Context; - checkerMgr = std::make_unique(*Ctx, *Opts, PP, Plugins, + checkerMgr = std::make_unique(Ctx, *Opts, PP, Plugins, CheckerRegistrationFns); - Mgr = std::make_unique(*Ctx, PP, PathConsumers, + Mgr = std::make_unique(Ctx, PP, PathConsumers, CreateStoreMgr, CreateConstraintMgr, checkerMgr.get(), *Opts, Injector); } @@ -268,7 +372,7 @@ return true; if (VD->hasExternalStorage() || VD->isStaticDataMember()) { - if (!cross_tu::containsConst(VD, *Ctx)) + if (!cross_tu::containsConst(VD, Ctx)) return true; } else { // Cannot be initialized in another TU. @@ -327,12 +431,62 @@ return true; } - void AddDiagnosticConsumer(PathDiagnosticConsumer *Consumer) override { - PathConsumers.push_back(Consumer); - } + friend class AnalysisMultiplexConsumer; - void AddCheckerRegistrationFn(std::function Fn) override { - CheckerRegistrationFns.push_back(std::move(Fn)); + std::unique_ptr createClangTidyConsumer() { + SmallString<128> Buf; + raw_svector_ostream ClangTidyConfig(Buf); + ClangTidyConfig << "Checks: '-*"; + if (Opts->TidyChecks.size() == 0) { + // TODO: Detect if no checks are enabled even when some flags are present. + return nullptr; + } + for (const std::string &TidyCheck: Opts->TidyChecks) { + // Should we try to prevent config injections? + ClangTidyConfig << "," << TidyCheck; + } + ClangTidyConfig << "'\n"; + // raw_svector_ostream doesn't need flush. + SmallVectorMemoryBuffer ClangTidyConfigBuffer{std::move(Buf)}; + ClangTidyCtx = std::make_unique( + std::make_unique( + ClangTidyGlobalOpts, *parseConfiguration(ClangTidyConfigBuffer))); + + ClangTidyCtx->setDiagnosticsEngine(&ClangTidyDiagEngine); + + SourceManager &SM = Ctx.getSourceManager(); + ClangTidyCtx->setSourceManager(&SM); + ClangTidyCtx->setCurrentFile( + SM.getFileEntryForID(SM.getMainFileID())->getName()); + ClangTidyCtx->setASTContext(&Ctx); + + for (ClangTidyModuleRegistry::entry E : + ClangTidyModuleRegistry::entries()) { + std::unique_ptr Module = E.instantiate(); + Module->addCheckFactories(ClangTidyChkFactories); + } + ClangTidyChecks = ClangTidyChkFactories.createChecks(&*ClangTidyCtx); + + ast_matchers::MatchFinder::MatchFinderOptions MatchFinderOpts{}; + if (Opts->PrintStats) { + ClangTidyCtx->setEnableProfiling(true); + ClangTidyCtx->setProfileStoragePrefix(""); + ClangTidyProfile = std::make_unique( + ClangTidyCtx->getProfileStorageParams()); + MatchFinderOpts.CheckProfiling.emplace(ClangTidyProfile->Records); + } + + ClangTidyMatchFinder = std::make_unique( + MatchFinderOpts); + + for (auto &Check : ClangTidyChecks) { + if (!Check->isLanguageVersionSupported(Ctx.getLangOpts())) + continue; + Check->registerMatchers(&*ClangTidyMatchFinder); + Check->registerPPCallbacks(SM, &PP, &PP); + } + + return ClangTidyMatchFinder->newASTConsumer(); } private: @@ -346,8 +500,39 @@ /// Print \p S to stderr if \c Opts->AnalyzerDisplayProgress is set. void reportAnalyzerProgress(StringRef S); }; // namespace -} // end anonymous namespace +class AnalysisMultiplexConsumer : public AnalysisASTConsumer { + AnalysisConsumer *Impl; + + std::vector> + hijackImplAndMultiplex(std::unique_ptr Consumer1, + std::unique_ptr Consumer2) { + Impl = Consumer1.get(); + std::vector> Consumers; + if (Consumer2) + Consumers.push_back(std::move(Consumer2)); + Consumers.push_back(std::move(Consumer1)); + return Consumers; + } + +public: + AnalysisMultiplexConsumer(std::unique_ptr Consumer1, + std::unique_ptr Consumer2) + : AnalysisASTConsumer(hijackImplAndMultiplex(std::move(Consumer1), + std::move(Consumer2))) { + assert(Impl); + } + + void AddDiagnosticConsumer(PathDiagnosticConsumer *Consumer) override { + Impl->PathConsumers.push_back(Consumer); + } + + void + AddCheckerRegistrationFn(std::function Fn) override { + Impl->CheckerRegistrationFns.push_back(std::move(Fn)); + } +}; +} // end anonymous namespace //===----------------------------------------------------------------------===// // AnalysisConsumer implementation. @@ -553,6 +738,9 @@ (FunctionSummaries.getTotalNumVisitedBasicBlocks() * 100) / NumBlocksInAnalyzedFunctions; + // No more notes are coming in for the last warning. + ClangTidyDiagConsumer.finish(); + // Explicitly destroy the PathDiagnosticConsumer. This will flush its output. // FIXME: This should be replaced with something that doesn't rely on // side-effects in PathDiagnosticConsumer's destructor. This is required when @@ -568,7 +756,7 @@ OS << FD->getQualifiedNameAsString(); // In C++, there are overloads. - if (Ctx->getLangOpts().CPlusPlus) { + if (Ctx.getLangOpts().CPlusPlus) { OS << '('; for (const auto &P : FD->parameters()) { if (P != *FD->param_begin()) @@ -579,7 +767,7 @@ } } else if (isa(D)) { - PresumedLoc Loc = Ctx->getSourceManager().getPresumedLoc(D->getLocation()); + PresumedLoc Loc = Ctx.getSourceManager().getPresumedLoc(D->getLocation()); if (Loc.isValid()) { OS << "block (line: " << Loc.getLine() << ", col: " << Loc.getColumn() @@ -624,7 +812,7 @@ // - Main source file: run both path-sensitive and non-path-sensitive checks. // - Header files: run non-path-sensitive checks only. // - System headers: don't run any checks. - SourceManager &SM = Ctx->getSourceManager(); + SourceManager &SM = Ctx.getSourceManager(); const Stmt *Body = D->getBody(); SourceLocation SL = Body ? Body->getBeginLoc() : D->getLocation(); SL = SM.getExpansionLoc(SL); @@ -730,8 +918,14 @@ AnalyzerOptionsRef analyzerOpts = CI.getAnalyzerOpts(); bool hasModelPath = analyzerOpts->Config.count("model-path") > 0; - return std::make_unique( + auto StaticAnalyzerConsumer = std::make_unique( CI, CI.getFrontendOpts().OutputFile, analyzerOpts, CI.getFrontendOpts().Plugins, hasModelPath ? new ModelInjector(CI) : nullptr); + + std::unique_ptr ClangTidyConsumer = + StaticAnalyzerConsumer->createClangTidyConsumer(); + + return std::make_unique( + std::move(StaticAnalyzerConsumer), std::move(ClangTidyConsumer)); } Index: clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt =================================================================== --- clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt +++ clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt @@ -1,3 +1,7 @@ +configure_file( + ${CMAKE_SOURCE_DIR}/../clang-tools-extra/clang-tidy/clang-tidy-config.h.cmake + ${CMAKE_CURRENT_BINARY_DIR}/clang-tidy-config.h) + include_directories( ${CMAKE_CURRENT_BINARY_DIR}/../Checkers ) set(LLVM_LINK_COMPONENTS @@ -23,6 +27,31 @@ clangLex clangStaticAnalyzerCheckers clangStaticAnalyzerCore + clangTidy + clangTidyAbseilModule + clangTidyAndroidModule + clangTidyAlteraModule + clangTidyBoostModule + clangTidyBugproneModule + clangTidyCERTModule + clangTidyConcurrencyModule + clangTidyCppCoreGuidelinesModule + clangTidyDarwinModule + clangTidyFuchsiaModule + clangTidyGoogleModule + clangTidyHICPPModule + clangTidyLinuxKernelModule + clangTidyLLVMModule + clangTidyLLVMLibcModule + clangTidyMiscModule + clangTidyModernizeModule + clangTidyMPIModule + clangTidyObjCModule + clangTidyOpenMPModule + clangTidyPerformanceModule + clangTidyPortabilityModule + clangTidyReadabilityModule + clangTidyZirconModule DEPENDS omp_gen Index: clang/test/Analysis/tidy-integration.c =================================================================== --- /dev/null +++ clang/test/Analysis/tidy-integration.c @@ -0,0 +1,38 @@ +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-tidy-checker=bugprone-assert-side-effect \ +// RUN: -verify=bugprone-assert-side-effect + +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-tidy-checker=bugprone-infinite-loop \ +// RUN: -verify=bugprone-infinite-loop + +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-tidy-checker=cppcoreguidelines-init-variables \ +// RUN: -verify=cppcoreguidelines-init-variables + +// Test multiple conflicting -analyzer-tidy-checker flags. +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-tidy-checker=bugprone-infinite-loop \ +// RUN: -analyzer-tidy-checker=-bugprone-* \ +// RUN: -analyzer-tidy-checker=cppcoreguidelines-init-variables \ +// RUN: -verify=cppcoreguidelines-init-variables + +// Test enabling checkers by default. +// RUN: %clang --analyze %s \ +// RUN: -Xclang -verify=bugprone-assert-side-effect,bugprone-infinite-loop + +void test_bugprone_assert_side_effect() { + void exit(int); + #define assert(x) if (!(x)) exit(1); + int x = 0; + assert((++x) == 1); // bugprone-assert-side-effect-warning-re{{{{^}}Found assert() with side effect [bugprone-assert-side-effect]{{$}}}} +} + +void test_bugprone_infinite_loop(int y) { + for (int x = 0; x < 10; ++y) { // bugprone-infinite-loop-warning-re{{{{^}}This loop is infinite; none of its condition variables (x) are updated in the loop body [bugprone-infinite-loop]{{$}}}} + } +} + +void test_cppcoreguidelines_init_variables() { + int x; // cppcoreguidelines-init-variables-warning-re{{{{^}}Variable 'x' is not initialized [cppcoreguidelines-init-variables]{{$}}}} +}