https://github.com/anonymouspc updated https://github.com/llvm/llvm-project/pull/170415
>From 49adc295407650f1da1b70ffb1c596907564e0b4 Mon Sep 17 00:00:00 2001 From: anonymouspc <[email protected]> Date: Wed, 3 Dec 2025 11:27:53 +0800 Subject: [PATCH 1/2] [Clang][Diagnose] Minimal support on emit-include-location in sarif mode Implement minimal relatedLocations support for include/module notes to avoid crashes when using -fdiagnostics-format=sarif. --- clang/include/clang/Basic/Sarif.h | 14 +++++ .../include/clang/Frontend/SARIFDiagnostic.h | 10 ++++ clang/lib/Basic/Sarif.cpp | 8 +++ clang/lib/Frontend/SARIFDiagnostic.cpp | 54 ++++++++++++++----- 4 files changed, 73 insertions(+), 13 deletions(-) diff --git a/clang/include/clang/Basic/Sarif.h b/clang/include/clang/Basic/Sarif.h index a88d1ee2965a9..b4f3610cc7568 100644 --- a/clang/include/clang/Basic/Sarif.h +++ b/clang/include/clang/Basic/Sarif.h @@ -325,6 +325,7 @@ class SarifResult { std::string HostedViewerURI; llvm::SmallDenseMap<StringRef, std::string, 4> PartialFingerprints; llvm::SmallVector<CharSourceRange, 8> Locations; + llvm::SmallVector<CharSourceRange, 8> RelatedLocations; llvm::SmallVector<ThreadFlow, 8> ThreadFlows; std::optional<SarifResultLevel> LevelOverride; @@ -364,6 +365,19 @@ class SarifResult { Locations.assign(DiagLocs.begin(), DiagLocs.end()); return *this; } + + SarifResult setRelatedLocations(llvm::ArrayRef<CharSourceRange> DiagLocs) { +#ifndef NDEBUG + for (const auto &Loc : DiagLocs) { + assert( + Loc.isCharRange() && + "SARIF RelatedLocations require character granular source ranges!"); + } +#endif + RelatedLocations.assign(DiagLocs.begin(), DiagLocs.end()); + return *this; + } + SarifResult setThreadFlows(llvm::ArrayRef<ThreadFlow> ThreadFlowResults) { ThreadFlows.assign(ThreadFlowResults.begin(), ThreadFlowResults.end()); return *this; diff --git a/clang/include/clang/Frontend/SARIFDiagnostic.h b/clang/include/clang/Frontend/SARIFDiagnostic.h index 780f36c874109..7a6f27eb3b9fa 100644 --- a/clang/include/clang/Frontend/SARIFDiagnostic.h +++ b/clang/include/clang/Frontend/SARIFDiagnostic.h @@ -63,10 +63,20 @@ class SARIFDiagnostic : public DiagnosticRenderer { ArrayRef<CharSourceRange> Ranges, const Diagnostic &Diag); + SarifResult addRelatedLocationToResult(SarifResult Result, FullSourceLoc Loc, + PresumedLoc PLoc); + + llvm::SmallVector<CharSourceRange> + getSarifLocation(FullSourceLoc Loc, PresumedLoc PLoc, + ArrayRef<CharSourceRange> Ranges); + SarifRule addDiagnosticLevelToRule(SarifRule Rule, DiagnosticsEngine::Level Level); llvm::StringRef emitFilename(StringRef Filename, const SourceManager &SM); + + llvm::SmallVector<std::pair<FullSourceLoc, PresumedLoc>> + RelatedLocationsCache; }; } // end namespace clang diff --git a/clang/lib/Basic/Sarif.cpp b/clang/lib/Basic/Sarif.cpp index b3fb9a21249e9..448de96d474af 100644 --- a/clang/lib/Basic/Sarif.cpp +++ b/clang/lib/Basic/Sarif.cpp @@ -404,6 +404,14 @@ void SarifDocumentWriter::appendResult(const SarifResult &Result) { Ret["locations"] = std::move(Locs); } + if (!Result.RelatedLocations.empty()) { + json::Array ReLocs; + for (auto &Range : Result.RelatedLocations) { + ReLocs.emplace_back(createLocation(createPhysicalLocation(Range))); + } + Ret["relatedLocations"] = std::move(ReLocs); + } + if (!Result.PartialFingerprints.empty()) { json::Object fingerprints = {}; for (auto &pair : Result.PartialFingerprints) { diff --git a/clang/lib/Frontend/SARIFDiagnostic.cpp b/clang/lib/Frontend/SARIFDiagnostic.cpp index ac27d7480de3e..fd765f0eacbb2 100644 --- a/clang/lib/Frontend/SARIFDiagnostic.cpp +++ b/clang/lib/Frontend/SARIFDiagnostic.cpp @@ -58,12 +58,48 @@ void SARIFDiagnostic::emitDiagnosticMessage( if (Loc.isValid()) Result = addLocationToResult(Result, Loc, PLoc, Ranges, *Diag); + for (auto &[RelLoc, RelPLoc] : RelatedLocationsCache) + Result = addRelatedLocationToResult(Result, RelLoc, RelPLoc); + RelatedLocationsCache.clear(); + Writer->appendResult(Result); } +void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) { + // We always emit include location before results, for example: + // + // In file included from ... + // In file included from ... + // error: ... + // + // At this time We cannot peek the SarifRule. But what we + // do is to push it into a cache and wait for next time + // \ref SARIFDiagnostic::emitDiagnosticMessage to pick it up. + RelatedLocationsCache.push_back({Loc, PLoc}); +} + +void SARIFDiagnostic::emitImportLocation(FullSourceLoc Loc, PresumedLoc PLoc, + StringRef ModuleName) { + RelatedLocationsCache.push_back({Loc, PLoc}); +} + SarifResult SARIFDiagnostic::addLocationToResult( SarifResult Result, FullSourceLoc Loc, PresumedLoc PLoc, ArrayRef<CharSourceRange> Ranges, const Diagnostic &Diag) { + auto Locations = getSarifLocation(Loc, PLoc, Ranges); + return Result.setLocations(Locations); +} + +SarifResult SARIFDiagnostic::addRelatedLocationToResult(SarifResult Result, + FullSourceLoc Loc, + PresumedLoc PLoc) { + auto Locations = getSarifLocation(Loc, PLoc, {}); + return Result.setRelatedLocations(Locations); +} + +llvm::SmallVector<CharSourceRange> +SARIFDiagnostic::getSarifLocation(FullSourceLoc Loc, PresumedLoc PLoc, + ArrayRef<CharSourceRange> Ranges) { SmallVector<CharSourceRange> Locations = {}; if (PLoc.isInvalid()) { @@ -75,7 +111,7 @@ SarifResult SARIFDiagnostic::addLocationToResult( // FIXME(llvm-project/issues/57366): File-only locations } } - return Result; + return {}; } FileID CaretFileID = Loc.getExpansionLoc().getFileID(); @@ -127,10 +163,11 @@ SarifResult SARIFDiagnostic::addLocationToResult( SourceLocation DiagLoc = SM.translateLineCol(FID, PLoc.getLine(), ColNo); // FIXME(llvm-project/issues/57366): Properly process #line directives. - Locations.push_back( - CharSourceRange{SourceRange{DiagLoc, DiagLoc}, /* ITR = */ false}); + CharSourceRange Range = {SourceRange{DiagLoc, DiagLoc}, /* ITR = */ false}; + if (Range.isValid()) + Locations.push_back(std::move(Range)); - return Result.setLocations(Locations); + return Locations; } SarifRule @@ -207,15 +244,6 @@ void SARIFDiagnostic::emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc, assert(false && "Not implemented in SARIF mode"); } -void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) { - assert(false && "Not implemented in SARIF mode"); -} - -void SARIFDiagnostic::emitImportLocation(FullSourceLoc Loc, PresumedLoc PLoc, - StringRef ModuleName) { - assert(false && "Not implemented in SARIF mode"); -} - void SARIFDiagnostic::emitBuildingModuleLocation(FullSourceLoc Loc, PresumedLoc PLoc, StringRef ModuleName) { >From 6a7889949913a8157ba834f562d0a9db0b2cd810 Mon Sep 17 00:00:00 2001 From: anonymouspc <[email protected]> Date: Sun, 7 Dec 2025 14:51:04 +0800 Subject: [PATCH 2/2] add test and release noes --- clang/docs/ReleaseNotes.rst | 4 ++++ clang/include/clang/Basic/Sarif.h | 8 ++++---- clang/lib/Frontend/SARIFDiagnostic.cpp | 4 ++-- clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp | 2 +- clang/unittests/Basic/SarifTest.cpp | 8 ++++---- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 3526ffb40f350..db4c2dd769762 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -449,6 +449,10 @@ Improvements to Clang's diagnostics comparison operators when mixed with bitwise operators in enum value initializers. This can be locally disabled by explicitly casting the initializer value. +- Fixed a crash when enabling ``-fdiagnostics-format=sarif`` and the output + carries messages like 'In file included from ...' or 'In module ...'. + Now the include/import locations are written into `sarif.run.result.relatedLocations`. + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Basic/Sarif.h b/clang/include/clang/Basic/Sarif.h index b4f3610cc7568..6f82d253876d9 100644 --- a/clang/include/clang/Basic/Sarif.h +++ b/clang/include/clang/Basic/Sarif.h @@ -355,18 +355,18 @@ class SarifResult { return *this; } - SarifResult setLocations(llvm::ArrayRef<CharSourceRange> DiagLocs) { + SarifResult addLocations(llvm::ArrayRef<CharSourceRange> DiagLocs) { #ifndef NDEBUG for (const auto &Loc : DiagLocs) { assert(Loc.isCharRange() && "SARIF Results require character granular source ranges!"); } #endif - Locations.assign(DiagLocs.begin(), DiagLocs.end()); + Locations.append(DiagLocs.begin(), DiagLocs.end()); return *this; } - SarifResult setRelatedLocations(llvm::ArrayRef<CharSourceRange> DiagLocs) { + SarifResult addRelatedLocations(llvm::ArrayRef<CharSourceRange> DiagLocs) { #ifndef NDEBUG for (const auto &Loc : DiagLocs) { assert( @@ -374,7 +374,7 @@ class SarifResult { "SARIF RelatedLocations require character granular source ranges!"); } #endif - RelatedLocations.assign(DiagLocs.begin(), DiagLocs.end()); + RelatedLocations.append(DiagLocs.begin(), DiagLocs.end()); return *this; } diff --git a/clang/lib/Frontend/SARIFDiagnostic.cpp b/clang/lib/Frontend/SARIFDiagnostic.cpp index fd765f0eacbb2..2cd32ce97ea85 100644 --- a/clang/lib/Frontend/SARIFDiagnostic.cpp +++ b/clang/lib/Frontend/SARIFDiagnostic.cpp @@ -87,14 +87,14 @@ SarifResult SARIFDiagnostic::addLocationToResult( SarifResult Result, FullSourceLoc Loc, PresumedLoc PLoc, ArrayRef<CharSourceRange> Ranges, const Diagnostic &Diag) { auto Locations = getSarifLocation(Loc, PLoc, Ranges); - return Result.setLocations(Locations); + return Result.addLocations(Locations); } SarifResult SARIFDiagnostic::addRelatedLocationToResult(SarifResult Result, FullSourceLoc Loc, PresumedLoc PLoc) { auto Locations = getSarifLocation(Loc, PLoc, {}); - return Result.setRelatedLocations(Locations); + return Result.addRelatedLocations(Locations); } llvm::SmallVector<CharSourceRange> diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp index aafd8d45537e3..c9f5774c7db7c 100644 --- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp @@ -218,7 +218,7 @@ SarifDiagnostics::createResult(const PathDiagnostic *Diag, .setRuleId(CheckName) .setDiagnosticMessage(Diag->getVerboseDescription()) .setDiagnosticLevel(SarifResultLevel::Warning) - .setLocations({Range}) + .addLocations({Range}) .addPartialFingerprint(IssueHashKey, IssueHash) .setHostedViewerURI(HtmlReportURL) .setThreadFlows(Flows); diff --git a/clang/unittests/Basic/SarifTest.cpp b/clang/unittests/Basic/SarifTest.cpp index 089b6cb01ded8..42e85085d646e 100644 --- a/clang/unittests/Basic/SarifTest.cpp +++ b/clang/unittests/Basic/SarifTest.cpp @@ -290,7 +290,7 @@ TEST_F(SarifDocumentWriterTest, checkSerializingResultsWithCustomRuleConfig) { TEST_F(SarifDocumentWriterTest, checkSerializingArtifacts) { // GIVEN: const std::string ExpectedOutput = - R"({"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":40,"location":{"index":0,"uri":"file:///main.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file:///main.cpp"},"region":{"endColumn":14,"startColumn":14,"startLine":3}}}],"message":{"text":"expected ';' after top level declarator"},"ruleId":"clang.unittest","ruleIndex":0}],"tool":{"driver":{"fullName":"sarif test runner","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"sarif test","rules":[{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":"Example rule created during unit tests"},"id":"clang.unittest","name":"clang unit test"}],"version":"1.0.0"}}}],"version":"2.1.0"})"; + R"({"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":40,"location":{"index":0,"uri":"file:///main.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file:///main.cpp"},"region":{"endColumn":14,"startColumn":14,"startLine":3}}}],"message":{"text":"expected ';' after top level declarator"},"relatedLocations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file:///main.cpp"},"region":{"endColumn":14,"startColumn":14,"startLine":3}}}],"ruleId":"clang.unittest","ruleIndex":0}],"tool":{"driver":{"fullName":"sarif test runner","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"sarif test","rules":[{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":"Example rule created during unit tests"},"id":"clang.unittest","name":"clang unit test"}],"version":"1.0.0"}}}],"version":"2.1.0"})"; SarifDocumentWriter Writer{SourceMgr}; const SarifRule &Rule = @@ -312,12 +312,12 @@ TEST_F(SarifDocumentWriterTest, checkSerializingArtifacts) { registerSource("/main.cpp", SourceText, /* IsMainFile = */ true); CharSourceRange SourceCSR = getFakeCharSourceRange(MainFileID, {3, 14}, {3, 14}); - DiagLocs.push_back(SourceCSR); const SarifResult &Result = SarifResult::create(RuleIdx) - .setLocations(DiagLocs) + .addLocations(DiagLocs) + .addRelatedLocations(DiagLocs) .setDiagnosticMessage("expected ';' after top level declarator") .setDiagnosticLevel(SarifResultLevel::Error); Writer.appendResult(Result); @@ -377,7 +377,7 @@ TEST_F(SarifDocumentWriterTest, checkSerializingCodeflows) { unsigned RuleIdx = Writer.createRule(Rule); const SarifResult &Result = SarifResult::create(RuleIdx) - .setLocations({DiagLoc}) + .addLocations({DiagLoc}) .setDiagnosticMessage("Redefinition of 'foo'") .setThreadFlows(Threadflows) .setDiagnosticLevel(SarifResultLevel::Warning); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
