[clang] [Clang][Diagnose] Minimal support on emit-include-location in sarif mode (PR #170415)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Dec 6 22:51:53 PST 2025
https://github.com/anonymouspc updated https://github.com/llvm/llvm-project/pull/170415
>From 49adc295407650f1da1b70ffb1c596907564e0b4 Mon Sep 17 00:00:00 2001
From: anonymouspc <shyeyian at petalmail.com>
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 <shyeyian at petalmail.com>
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);
More information about the cfe-commits
mailing list