llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Matthew (matthew-j-code) <details> <summary>Changes</summary> Clang-tidy is a static analysis framework, and should therefore be able to output results in the Static Analysis Results Interchange Format (SARIF). This brings clang-tidy in line with clang and clang-static-analyzer. This PR implements SARIF output for clang-tidy. --- Patch is 36.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/219182.diff 9 Files Affected: - (modified) clang-tools-extra/clang-tidy/ClangTidy.cpp (+125-5) - (modified) clang-tools-extra/clang-tidy/ClangTidy.h (+2-1) - (modified) clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp (+20-1) - (modified) clang-tools-extra/docs/ReleaseNotes.md (+4) - (added) clang-tools-extra/test/clang-tidy/infrastructure/export-SARIF-Thread-flows.cpp (+136) - (added) clang-tools-extra/test/clang-tidy/infrastructure/export-SARIF-diagnostics-terminal.cpp (+98) - (added) clang-tools-extra/test/clang-tidy/infrastructure/export-SARIF-diagnostics.cpp (+165) - (modified) clang/include/clang/Frontend/SARIFDiagnostic.h (+11-5) - (modified) clang/lib/Frontend/SARIFDiagnostic.cpp (+32-28) ``````````diff diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index d065bb2c4bcfc..05084b33a92b3 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -24,10 +24,12 @@ #include "clang/AST/ASTConsumer.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/DiagnosticFrontend.h" +#include "clang/Basic/Sarif.h" #include "clang/Format/Format.h" #include "clang/Frontend/ASTConsumers.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/MultiplexConsumer.h" +#include "clang/Frontend/SARIFDiagnostic.h" #include "clang/Frontend/TextDiagnosticPrinter.h" #include "clang/Lex/Preprocessor.h" #include "clang/Lex/PreprocessorOptions.h" @@ -36,6 +38,7 @@ #include "clang/Tooling/DiagnosticsYaml.h" // IWYU pragma: keep #include "clang/Tooling/Refactoring.h" #include "clang/Tooling/Tooling.h" +#include "llvm/Support/Error.h" #include "llvm/Support/Process.h" #include <memory> #include <utility> @@ -101,11 +104,18 @@ class AnalyzerDiagnosticConsumer : public ento::PathDiagnosticConsumer { class ErrorReporter { public: ErrorReporter(ClangTidyContext &Context, FixBehaviour ApplyFixes, - llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, + llvm::raw_ostream *SarifOS = nullptr) : Files(FileSystemOptions(), std::move(BaseFS)), DiagPrinter(new TextDiagnosticPrinter(llvm::outs(), DiagOpts)), Diags(DiagnosticIDs::create(), DiagOpts, DiagPrinter), - SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes) { + SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes), + SarifOS(SarifOS) { + if (SarifOS) { + SarifWriter.emplace(SourceMgr); + SarifWriter->createRun("clang-tidy", "clang-tidy"); + } + DiagOpts.setShowColors(Context.getOptions().UseColor.value_or( llvm::sys::Process::StandardOutHasColors()) ? ShowColorsKind::On @@ -195,6 +205,97 @@ class ErrorReporter { } for (const auto &Note : Error.Notes) reportNote(Note); + + if (SarifWriter) + exportSarifResult(Error, Loc); + } + + SmallVector<ThreadFlow, 8> createThreadFlows(const ClangTidyError &Error) { + SmallVector<ThreadFlow, 8> Flows; + if (!StringRef(Error.DiagnosticName).starts_with("clang-analyzer-")) + return Flows; + + Flows.reserve(Error.Notes.size()); + for (const tooling::DiagnosticMessage &Note : Error.Notes) { + tooling::FileByteRange FBR; + if (Note.Ranges.empty()) { + FBR.FilePath = Note.FilePath; + FBR.FileOffset = Note.FileOffset; + FBR.Length = 1; + } else { + FBR = Note.Ranges.front(); + } + const CharSourceRange Range = getRange(FBR); + Flows.push_back( + ThreadFlow::create().setRange(Range).setMessage(Note.Message)); + } + return Flows; + } + + static std::string buildClangTidyHelpURI(StringRef Name) { + const auto [Module, Check] = Name.starts_with("clang-analyzer-") + ? Name.rsplit('-') + : Name.split('-'); + return llvm::formatv( + "https://clang.llvm.org/extra/clang-tidy/checks/{0}/{1}.html";, + Module, Check); + } + + void exportSarifResult(const ClangTidyError &Error, + const SourceLocation Loc) { + assert(SarifWriter && + "SarifWriter must be initialized to export SARIF results"); + + const std::pair<llvm::StringMap<size_t>::iterator, bool> RuleIndexEntry = + SarifRuleIdx.try_emplace(Error.DiagnosticName, 0); + llvm::StringMap<size_t>::iterator RuleIndexEntryIt = RuleIndexEntry.first; + bool Inserted = RuleIndexEntry.second; + size_t &RuleIndex = RuleIndexEntryIt->second; + + const DiagnosticsEngine::Level EffectiveLevel = + Error.IsWarningAsError + ? DiagnosticsEngine::Error + : static_cast<DiagnosticsEngine::Level>(Error.DiagLevel); + + if (Inserted) { + const StringRef Name = Error.DiagnosticName; + SarifRule Rule = + SarifRule::create().setRuleId(Name).setName(Name).setDescription( + Error.Message.Message); + + if (!Name.starts_with("clang-diagnostic")) + Rule = Rule.setHelpURI(buildClangTidyHelpURI(Name)); + + Rule = addDiagnosticLevelToRule(Rule, EffectiveLevel); + RuleIndex = SarifWriter->createRule(Rule); + } + + SarifResult Result = + SarifResult::create(RuleIndex) + .setDiagnosticMessage(Error.Message.Message) + .setDiagnosticLevel(getSarifResultLevel(EffectiveLevel)) + .setThreadFlows(createThreadFlows(Error)) + .addLocations(getResultRanges(Error, Loc)); + + SarifWriter->appendResult(Result); + } + + SmallVector<CharSourceRange, 4> getResultRanges(const ClangTidyError &Error, + SourceLocation Loc) { + SmallVector<CharSourceRange, 4> Ranges; + Ranges.reserve(Error.Message.Ranges.size()); + for (const FileByteRange &FBR : Error.Message.Ranges) + Ranges.push_back(getRange(FBR)); + + if (Ranges.empty() && Loc.isValid()) { + // Some Clang-Tidy diagnostics are issued with a single location (not a + // range). For these, we create a range of length 1 at the diagnostic + // location. As, SARIF results require a character range for each + // location. + Ranges.push_back( + CharSourceRange::getCharRange(Loc, Loc.getLocWithOffset(1))); + } + return Ranges; } void finish() { @@ -254,6 +355,8 @@ class ErrorReporter { if (OriginalCWD) VFS.setCurrentWorkingDirectory(*OriginalCWD); } + if (SarifWriter) + finalizeSarif(); } unsigned getWarningsAsErrorsCount() const { return WarningsAsErrors; } @@ -271,6 +374,14 @@ class ErrorReporter { return SourceMgr.getLocForStartOfFile(ID).getLocWithOffset(Offset); } + void finalizeSarif() { + assert(SarifWriter && + "SarifWriter must be initialized to export SARIF results"); + assert(SarifOS && "SarifOS must be initialized to export SARIF results"); + llvm::json::Value Document = SarifWriter->createDocument(); + *SarifOS << llvm::formatv("{0:2}", Document); + } + void reportFix(const DiagnosticBuilder &Diag, const llvm::StringMap<Replacements> &Fix) { for (const auto &FileAndReplacements : Fix) { @@ -328,6 +439,13 @@ class ErrorReporter { unsigned TotalFixes = 0U; unsigned AppliedFixes = 0U; unsigned WarningsAsErrors = 0U; + llvm::raw_ostream *SarifOS = nullptr; + // ExportSarifResult is called for each SARIF rule that gets created, while a + // SarifWriter run is created once at ErrorReporter construction. + std::optional<SarifDocumentWriter> SarifWriter; + // SarifRuleIdx is used across multiple calls of exportSarifResult as cache to + // avoid creating duplicate SARIF rules. + llvm::StringMap<size_t> SarifRuleIdx; }; class ClangTidyASTConsumer : public MultiplexConsumer { @@ -613,7 +731,8 @@ runClangTidy(ClangTidyContext &Context, const CompilationDatabase &Compilations, Context.setEnableProfiling(EnableCheckProfile); Context.setProfileStoragePrefix(StoreCheckProfile); - ClangTidyDiagnosticConsumer DiagConsumer(Context, nullptr, true, ApplyAnyFix); + ClangTidyDiagnosticConsumer DiagConsumer(Context, nullptr, true, ApplyAnyFix, + true); auto DiagOpts = std::make_unique<DiagnosticOptions>(); DiagnosticsEngine DE(DiagnosticIDs::create(), *DiagOpts, &DiagConsumer, /*ShouldOwnClient=*/false); @@ -668,8 +787,9 @@ runClangTidy(ClangTidyContext &Context, const CompilationDatabase &Compilations, void handleErrors(llvm::ArrayRef<ClangTidyError> Errors, ClangTidyContext &Context, FixBehaviour Fix, unsigned &WarningsAsErrorsCount, - llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) { - ErrorReporter Reporter(Context, Fix, std::move(BaseFS)); + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, + llvm::raw_ostream *SarifOS) { + ErrorReporter Reporter(Context, Fix, std::move(BaseFS), SarifOS); llvm::vfs::FileSystem &FileSystem = Reporter.getSourceManager().getFileManager().getVirtualFileSystem(); auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory(); diff --git a/clang-tools-extra/clang-tidy/ClangTidy.h b/clang-tools-extra/clang-tidy/ClangTidy.h index 5fac25bd3bbca..5069cb3cca629 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.h +++ b/clang-tools-extra/clang-tidy/ClangTidy.h @@ -118,7 +118,8 @@ enum FixBehaviour { void handleErrors(llvm::ArrayRef<ClangTidyError> Errors, ClangTidyContext &Context, FixBehaviour Fix, unsigned &WarningsAsErrorsCount, - llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS); + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, + llvm::raw_ostream *SarifOS = nullptr); /// Serializes replacements into YAML and writes them to the specified /// output stream. diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index 7ee556fa7c51f..cc39ae2b507bf 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -21,6 +21,7 @@ #include "clang/Tooling/CommonOptionsParser.h" #include "llvm/ADT/StringSet.h" #include "llvm/Support/CommandLine.h" +#include "llvm/Support/FileSystem.h" #include "llvm/Support/InitLLVM.h" #include "llvm/Support/PluginLoader.h" // IWYU pragma: keep #include "llvm/Support/Process.h" @@ -311,6 +312,12 @@ code with clang-apply-replacements. cl::value_desc("filename"), cl::cat(ClangTidyCategory)); +static cl::opt<std::string> ExportSarif("export-sarif", desc(R"( +File in which to store diagnostics in SARIF format. +)"), + cl::value_desc("filename"), + cl::cat(ClangTidyCategory)); + static cl::opt<bool> Quiet("quiet", desc(R"( Run clang-tidy in quiet mode. This suppresses printing statistics about ignored warnings and @@ -652,6 +659,18 @@ int clangTidyMain(int argc, const char **argv) { FileName = PathList.front(); const SmallString<256> FilePath = makeAbsolute(FileName); + + std::unique_ptr<llvm::raw_fd_ostream> SarifOS; + if (!ExportSarif.empty()) { + std::error_code EC; + SarifOS = std::make_unique<llvm::raw_fd_ostream>(ExportSarif, EC, + llvm::sys::fs::OF_Text); + if (EC) { + llvm::errs() << "Error opening output file: " << EC.message() << '\n'; + return 1; + } + } + ClangTidyOptions EffectiveOptions = OptionsProvider->getOptions(FilePath); const std::vector<std::string> EnabledChecks = @@ -754,7 +773,7 @@ int clangTidyMain(int argc, const char **argv) { unsigned WErrorCount = 0; handleErrors(Errors, Context, DisableFixes ? FB_NoFix : Behaviour, - WErrorCount, BaseFS); + WErrorCount, BaseFS, SarifOS.get()); if (!ExportFixes.empty() && !Errors.empty()) { std::error_code EC; diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 6fe497e5f6eaf..8c689dc337441 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -100,6 +100,10 @@ infrastructure are described first, followed by tool-specific sections. `-std=cXX-or-earlier` values, mirroring the existing `-std=cXX-or-later`. New construct expands to the given standard and every earlier one. +- Added a new {program}`clang-tidy` command line option `-export-sarif=<file>` + in which clang-tidy diagnostics are exported in the `SARIF https://sarifweb.azurewebsites.net/` + format. + #### New checks - New {doc}`llvm-invalid-regex-pattern diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/export-SARIF-Thread-flows.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/export-SARIF-Thread-flows.cpp new file mode 100644 index 0000000000000..63ff3653cbc3f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/export-SARIF-Thread-flows.cpp @@ -0,0 +1,136 @@ +// RUN: grep -Ev "// *[A-Z-]+:" %s > %t-input.cpp +// RUN: clang-tidy %t-input.cpp -checks='-*,clang-analyzer*' -export-sarif=%t.sarif > %t.msg 2>&1 +// RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s -implicit-check-not='{{warning|error|note}}:' +// RUN: FileCheck -input-file=%t.sarif -check-prefix=CHECK-SARIF %s +void f() { + int *ptr = nullptr; + *ptr = 1; +} + +//CHECK-MESSAGES: -input.cpp:3:8: warning: Dereference of null pointer (loaded from variable 'ptr') [clang-analyzer-core.NullDereference] +//CHECK-MESSAGES: -input.cpp:2:3: note: 'ptr' initialized to a null pointer value +//CHECK-MESSAGES: -input.cpp:3:8: note: Dereference of null pointer (loaded from variable 'ptr') + +//CHECK-SARIF: { +//CHECK-SARIF-NEXT: "$schema": "https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json";, +//CHECK-SARIF-NEXT: "runs": [ +//CHECK-SARIF-NEXT: { +//CHECK-SARIF-NEXT: "artifacts": [ +//CHECK-SARIF-NEXT: { +//CHECK-SARIF-NEXT: "length": {{[0-9]+}}, +//CHECK-SARIF-NEXT: "location": { +//CHECK-SARIF-NEXT: "index": 0, +//CHECK-SARIF-NEXT: "uri": "{{.*}}-input.cpp" +//CHECK-SARIF-NEXT: }, +//CHECK-SARIF-NEXT: "mimeType": "text/plain", +//CHECK-SARIF-NEXT: "roles": [ +//CHECK-SARIF-NEXT: "resultFile" +//CHECK-SARIF-NEXT: ] +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: ], +//CHECK-SARIF-NEXT: "columnKind": "unicodeCodePoints", +//CHECK-SARIF-NEXT: "results": [ +//CHECK-SARIF-NEXT: { +//CHECK-SARIF-NEXT: "codeFlows": [ +//CHECK-SARIF-NEXT: { +//CHECK-SARIF-NEXT: "threadFlows": [ +//CHECK-SARIF-NEXT: { +//CHECK-SARIF-NEXT: "locations": [ +//CHECK-SARIF-NEXT: { +//CHECK-SARIF-NEXT: "importance": "important", +//CHECK-SARIF-NEXT: "location": { +//CHECK-SARIF-NEXT: "message": { +//CHECK-SARIF-NEXT: "text": "'ptr' initialized to a null pointer value" +//CHECK-SARIF-NEXT: }, +//CHECK-SARIF-NEXT: "physicalLocation": { +//CHECK-SARIF-NEXT: "artifactLocation": { +//CHECK-SARIF-NEXT: "index": 0, +//CHECK-SARIF-NEXT: "uri": "{{.*}}-input.cpp" +//CHECK-SARIF-NEXT: }, +//CHECK-SARIF-NEXT: "region": { +//CHECK-SARIF-NEXT: "endColumn": 11, +//CHECK-SARIF-NEXT: "endLine": 2, +//CHECK-SARIF-NEXT: "startColumn": 3, +//CHECK-SARIF-NEXT: "startLine": 2 +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: }, +//CHECK-SARIF-NEXT: { +//CHECK-SARIF-NEXT: "importance": "important", +//CHECK-SARIF-NEXT: "location": { +//CHECK-SARIF-NEXT: "message": { +//CHECK-SARIF-NEXT: "text": "Dereference of null pointer (loaded from variable 'ptr')" +//CHECK-SARIF-NEXT: }, +//CHECK-SARIF-NEXT: "physicalLocation": { +//CHECK-SARIF-NEXT: "artifactLocation": { +//CHECK-SARIF-NEXT: "index": 0, +//CHECK-SARIF-NEXT: "uri": "{{.*}}-input.cpp" +//CHECK-SARIF-NEXT: }, +//CHECK-SARIF-NEXT: "region": { +//CHECK-SARIF-NEXT: "endColumn": 7, +//CHECK-SARIF-NEXT: "endLine": 3, +//CHECK-SARIF-NEXT: "startColumn": 4, +//CHECK-SARIF-NEXT: "startLine": 3 +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: ] +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: ] +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: ], +//CHECK-SARIF-NEXT: "level": "warning", +//CHECK-SARIF-NEXT: "locations": [ +//CHECK-SARIF-NEXT: { +//CHECK-SARIF-NEXT: "physicalLocation": { +//CHECK-SARIF-NEXT: "artifactLocation": { +//CHECK-SARIF-NEXT: "index": 0, +//CHECK-SARIF-NEXT: "uri": "{{.*}}-input.cpp" +//CHECK-SARIF-NEXT: }, +//CHECK-SARIF-NEXT: "region": { +//CHECK-SARIF-NEXT: "endColumn": 7, +//CHECK-SARIF-NEXT: "endLine": 3, +//CHECK-SARIF-NEXT: "startColumn": 4, +//CHECK-SARIF-NEXT: "startLine": 3 +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: ], +//CHECK-SARIF-NEXT: "message": { +//CHECK-SARIF-NEXT: "text": "Dereference of null pointer (loaded from variable 'ptr')" +//CHECK-SARIF-NEXT: }, +//CHECK-SARIF-NEXT: "ruleId": "clang-analyzer-core.NullDereference", +//CHECK-SARIF-NEXT: "ruleIndex": 0 +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: ], +//CHECK-SARIF-NEXT: "tool": { +//CHECK-SARIF-NEXT: "driver": { +//CHECK-SARIF-NEXT: "fullName": "clang-tidy", +//CHECK-SARIF-NEXT: "informationUri": "https://clang.llvm.org/docs/UsersManual.html";, +//CHECK-SARIF-NEXT: "language": "en-US", +//CHECK-SARIF-NEXT: "name": "clang-tidy", +//CHECK-SARIF-NEXT: "rules": [ +//CHECK-SARIF-NEXT: { +//CHECK-SARIF-NEXT: "defaultConfiguration": { +//CHECK-SARIF-NEXT: "enabled": true, +//CHECK-SARIF-NEXT: "level": "warning", +//CHECK-SARIF-NEXT: "rank": -1 +//CHECK-SARIF-NEXT: }, +//CHECK-SARIF-NEXT: "fullDescription": { +//CHECK-SARIF-NEXT: "text": "Dereference of null pointer (loaded from variable 'ptr')" +//CHECK-SARIF-NEXT: }, +//CHECK-SARIF-NEXT: "helpUri": "https://clang.llvm.org/extra/clang-tidy/checks/clang-analyzer/core.NullDereference.html";, +//CHECK-SARIF-NEXT: "id": "clang-analyzer-core.NullDereference", +//CHECK-SARIF-NEXT: "name": "clang-analyzer-core.NullDereference" +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: ], +//CHECK-SARIF-NEXT: "version": "{{.*}}" +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: } +//CHECK-SARIF-NEXT: ], +//CHECK-SARIF-NEXT: "version": "{{.*}}" +//CHECK-SARIF-NEXT: } + diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/export-SARIF-diagnostics-terminal.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/export-SARIF-diagnostics-terminal.cpp new file mode 100644 index 0000000000000..52fb3fc3a639c --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/export-SARIF-diagnostics-terminal.cpp @@ -0,0 +1,98 @@ +// RUN: grep -Ev "// *[A-Z-]+:" %s > %t-input.cpp +// RUN: not clang-tidy %t-input.cpp -checks='-*,google-explicit-constructor,clang-diagnostic-missing-prototypes,clang-diagnostic-zero-length-array' --warnings-as-errors='clang-diagnostic-missing-prototypes,google-explicit-constructor' -export-sarif - -- -Wmissing-prototypes -Wzero-length-array 2>/dev/null | ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/219182 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
