[analyzer] Emit IssueHash in SARIF #159445
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The change adds two new properties to each `result` object in the SARIF log: `partialFingerprints`: Contains the "issue hash" that the analyzer already generates for each result, which can help identify a result across runs even if surrounding code changes. `hostedViewUri`: If running with `-analyzer-format=sarif-html`, this property will now be emitted with the `file:` URL of the generated HTML report for that result. Along the way, I discovered an existing bug where the HTML diagnostic consumer does not record the path to the generated report if another compilation already created that report. This caused both the SARIF and Plist consumers to be missing the link to the file in all but one of the compilations in case of a warning in a header file. I added a new test to ensure that the generated SARIF for each compilation contains the property. Finally, I made a few changes to the `normalize_sarif` processing in the tests. I switched to `sed` to allow substitutions. The normalization now removes directory components from `file:` URLs, replaces the `length` property of the source file with a constant `-1`, and puts placeholders in the values of the `version` properties rather than just deleting them. The URL transformation in particular lets us verify that the right filename is generated for each HTML report. Fixes llvm#158159 rdar://160410408
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang Author: Dave Bartolomeo (dbartol) ChangesThis change adds two new properties to each
Along the way, I discovered an existing bug where the HTML diagnostic consumer does not record the path to the generated report if another compilation already created that report. This caused both the SARIF and Plist consumers to be missing the link to the file in all but one of the compilations in case of a warning in a header file. I added a new test to ensure that the generated SARIF for each compilation contains the property. Finally, I made a few changes to the Fixes #158159 rdar://160410408 Patch is 33.47 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/159445.diff 13 Files Affected:
diff --git a/clang/include/clang/Analysis/PathDiagnostic.h b/clang/include/clang/Analysis/PathDiagnostic.h
index 5907df022e449..197920d4cd100 100644
--- a/clang/include/clang/Analysis/PathDiagnostic.h
+++ b/clang/include/clang/Analysis/PathDiagnostic.h
@@ -885,6 +885,10 @@ class PathDiagnostic : public llvm::FoldingSetNode {
return UniqueingDecl;
}
+ /// Get a hash that identifies the issue.
+ SmallString<32> getIssueHash(const SourceManager &SrcMgr,
+ const LangOptions &LangOpts) const;
+
void flattenLocations() {
Loc.flatten();
for (const auto &I : pathImpl)
diff --git a/clang/include/clang/Basic/Sarif.h b/clang/include/clang/Basic/Sarif.h
index e6c46224b316d..a88d1ee2965a9 100644
--- a/clang/include/clang/Basic/Sarif.h
+++ b/clang/include/clang/Basic/Sarif.h
@@ -322,6 +322,8 @@ class SarifResult {
uint32_t RuleIdx;
std::string RuleId;
std::string DiagnosticMessage;
+ std::string HostedViewerURI;
+ llvm::SmallDenseMap<StringRef, std::string, 4> PartialFingerprints;
llvm::SmallVector<CharSourceRange, 8> Locations;
llvm::SmallVector<ThreadFlow, 8> ThreadFlows;
std::optional<SarifResultLevel> LevelOverride;
@@ -347,6 +349,11 @@ class SarifResult {
return *this;
}
+ SarifResult setHostedViewerURI(llvm::StringRef URI) {
+ HostedViewerURI = URI.str();
+ return *this;
+ }
+
SarifResult setLocations(llvm::ArrayRef<CharSourceRange> DiagLocs) {
#ifndef NDEBUG
for (const auto &Loc : DiagLocs) {
@@ -366,6 +373,12 @@ class SarifResult {
LevelOverride = TheLevel;
return *this;
}
+
+ SarifResult addPartialFingerprint(llvm::StringRef key,
+ llvm::StringRef value) {
+ PartialFingerprints[key] = value;
+ return *this;
+ }
};
/// This class handles creating a valid SARIF document given various input
@@ -475,6 +488,8 @@ class SarifDocumentWriter {
/// reported diagnostics, resulting in an expensive call.
llvm::json::Object createDocument();
+ static std::string fileNameToURI(llvm::StringRef Filename);
+
private:
/// Source Manager to use for the current SARIF document.
const SourceManager &SourceMgr;
diff --git a/clang/lib/Analysis/PathDiagnostic.cpp b/clang/lib/Analysis/PathDiagnostic.cpp
index ef24efd3c4bd0..e42731b93bfb2 100644
--- a/clang/lib/Analysis/PathDiagnostic.cpp
+++ b/clang/lib/Analysis/PathDiagnostic.cpp
@@ -24,6 +24,7 @@
#include "clang/AST/Type.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/IssueHash.h"
#include "clang/Analysis/ProgramPoint.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
@@ -1075,6 +1076,19 @@ unsigned PathDiagnostic::full_size() {
return size;
}
+SmallString<32>
+PathDiagnostic::getIssueHash(const SourceManager &SrcMgr,
+ const LangOptions &LangOpts) const {
+ PathDiagnosticLocation UPDLoc = getUniqueingLoc();
+ FullSourceLoc FullLoc(
+ SrcMgr.getExpansionLoc(UPDLoc.isValid() ? UPDLoc.asLocation()
+ : getLocation().asLocation()),
+ SrcMgr);
+
+ return clang::getIssueHash(FullLoc, getCheckerName(), getBugType(),
+ getDeclWithIssue(), LangOpts);
+}
+
//===----------------------------------------------------------------------===//
// FoldingSet profiling methods.
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Basic/Sarif.cpp b/clang/lib/Basic/Sarif.cpp
index 69862b73febd7..b3fb9a21249e9 100644
--- a/clang/lib/Basic/Sarif.cpp
+++ b/clang/lib/Basic/Sarif.cpp
@@ -67,7 +67,7 @@ static std::string percentEncodeURICharacter(char C) {
/// \param Filename The filename to be represented as URI.
///
/// \return RFC3986 URI representing the input file name.
-static std::string fileNameToURI(StringRef Filename) {
+std::string SarifDocumentWriter::fileNameToURI(StringRef Filename) {
SmallString<32> Ret = StringRef("file://");
// Get the root name to see if it has a URI authority.
@@ -391,6 +391,11 @@ void SarifDocumentWriter::appendResult(const SarifResult &Result) {
json::Object Ret{{"message", createMessage(Result.DiagnosticMessage)},
{"ruleIndex", static_cast<int64_t>(RuleIdx)},
{"ruleId", Rule.Id}};
+
+ if (!Result.HostedViewerURI.empty()) {
+ Ret["hostedViewerUri"] = Result.HostedViewerURI;
+ }
+
if (!Result.Locations.empty()) {
json::Array Locs;
for (auto &Range : Result.Locations) {
@@ -398,6 +403,15 @@ void SarifDocumentWriter::appendResult(const SarifResult &Result) {
}
Ret["locations"] = std::move(Locs);
}
+
+ if (!Result.PartialFingerprints.empty()) {
+ json::Object fingerprints = {};
+ for (auto &pair : Result.PartialFingerprints) {
+ fingerprints[pair.first] = pair.second;
+ }
+ Ret["partialFingerprints"] = std::move(fingerprints);
+ }
+
if (!Result.ThreadFlows.empty())
Ret["codeFlows"] = json::Array{createCodeFlow(Result.ThreadFlows)};
diff --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
index 4c9c619f2487a..217b853305ed1 100644
--- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//
+#include "HTMLDiagnostics.h"
#include "PlistDiagnostics.h"
#include "SarifDiagnostics.h"
#include "clang/AST/Decl.h"
@@ -82,7 +83,7 @@ class HTMLDiagnostics : public PathDiagnosticConsumer {
void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
FilesMade *filesMade) override;
- StringRef getName() const override { return "HTMLDiagnostics"; }
+ StringRef getName() const override { return HTML_DIAGNOSTICS_NAME; }
bool supportsCrossFileDiagnostics() const override {
return SupportsCrossFileDiagnostics;
@@ -254,18 +255,6 @@ void HTMLDiagnostics::FlushDiagnosticsImpl(
ReportDiag(*Diag, filesMade);
}
-static llvm::SmallString<32> getIssueHash(const PathDiagnostic &D,
- const Preprocessor &PP) {
- SourceManager &SMgr = PP.getSourceManager();
- PathDiagnosticLocation UPDLoc = D.getUniqueingLoc();
- FullSourceLoc L(SMgr.getExpansionLoc(UPDLoc.isValid()
- ? UPDLoc.asLocation()
- : D.getLocation().asLocation()),
- SMgr);
- return getIssueHash(L, D.getCheckerName(), D.getBugType(),
- D.getDeclWithIssue(), PP.getLangOpts());
-}
-
void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D,
FilesMade *filesMade) {
// Create the HTML directory if it is missing.
@@ -310,7 +299,8 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D,
}
}
- SmallString<32> IssueHash = getIssueHash(D, PP);
+ SmallString<32> IssueHash =
+ D.getIssueHash(PP.getSourceManager(), PP.getLangOpts());
auto [It, IsNew] = EmittedHashes.insert(IssueHash);
if (!IsNew) {
// We've already emitted a duplicate issue. It'll get overwritten anyway.
@@ -369,6 +359,12 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D,
if (EC != llvm::errc::file_exists) {
llvm::errs() << "warning: could not create file in '" << Directory
<< "': " << EC.message() << '\n';
+ } else if (filesMade) {
+ // Record that we created the file so that it gets referenced in the
+ // plist and SARIF reports for every translation unit that found the
+ // issue.
+ filesMade->addDiagnostic(D, getName(),
+ llvm::sys::path::filename(ResultPath));
}
return;
}
@@ -679,8 +675,8 @@ void HTMLDiagnostics::FinalizeHTML(const PathDiagnostic &D, Rewriter &R,
os << "\n<!-- FUNCTIONNAME " << declName << " -->\n";
- os << "\n<!-- ISSUEHASHCONTENTOFLINEINCONTEXT " << getIssueHash(D, PP)
- << " -->\n";
+ os << "\n<!-- ISSUEHASHCONTENTOFLINEINCONTEXT "
+ << D.getIssueHash(PP.getSourceManager(), PP.getLangOpts()) << " -->\n";
os << "\n<!-- BUGLINE "
<< LineNumber
diff --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.h b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.h
new file mode 100644
index 0000000000000..d6e4d6b344ddb
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.h
@@ -0,0 +1,14 @@
+//==- HTMLDiagnostics.h - HTML Diagnostics for Paths ---------------*- C++ -*-//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_STATICANALYZER_CORE_HTMLDIAGNOSTICS_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CORE_HTMLDIAGNOSTICS_H
+
+#define HTML_DIAGNOSTICS_NAME "HTMLDiagnostics"
+
+#endif
diff --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
index f4b08e265f9e7..3e3fff900cde8 100644
--- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -706,13 +706,11 @@ void PlistDiagnostics::FlushDiagnosticsImpl(
o << " <key>issue_hash_content_of_line_in_context</key>";
PathDiagnosticLocation UPDLoc = D->getUniqueingLoc();
FullSourceLoc L(SM.getExpansionLoc(UPDLoc.isValid()
- ? UPDLoc.asLocation()
- : D->getLocation().asLocation()),
+ ? UPDLoc.asLocation()
+ : D->getLocation().asLocation()),
SM);
- const Decl *DeclWithIssue = D->getDeclWithIssue();
- EmitString(o, getIssueHash(L, D->getCheckerName(), D->getBugType(),
- DeclWithIssue, LangOpts))
- << '\n';
+
+ EmitString(o, D->getIssueHash(SM, LangOpts)) << '\n';
// Output information about the semantic context where
// the issue occurred.
diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
index d2c46cf00ac80..6673f2f319c0e 100644
--- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -11,6 +11,8 @@
//===----------------------------------------------------------------------===//
#include "SarifDiagnostics.h"
+#include "HTMLDiagnostics.h"
+#include "clang/Analysis/IssueHash.h"
#include "clang/Analysis/MacroExpansionContext.h"
#include "clang/Analysis/PathDiagnostic.h"
#include "clang/Basic/Sarif.h"
@@ -31,12 +33,13 @@ namespace {
class SarifDiagnostics : public PathDiagnosticConsumer {
std::string OutputFile;
const LangOptions &LO;
+ const SourceManager &SM;
SarifDocumentWriter SarifWriter;
public:
SarifDiagnostics(const std::string &Output, const LangOptions &LO,
const SourceManager &SM)
- : OutputFile(Output), LO(LO), SarifWriter(SM) {}
+ : OutputFile(Output), LO(LO), SM(SM), SarifWriter(SM) {}
~SarifDiagnostics() override = default;
void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
@@ -46,6 +49,11 @@ class SarifDiagnostics : public PathDiagnosticConsumer {
PathGenerationScheme getGenerationScheme() const override { return Minimal; }
bool supportsLogicalOpControlFlow() const override { return true; }
bool supportsCrossFileDiagnostics() const override { return true; }
+
+private:
+ SarifResult createResult(const PathDiagnostic *Diag,
+ const StringMap<uint32_t> &RuleMapping,
+ const LangOptions &LO, FilesMade *FM);
};
} // end anonymous namespace
@@ -173,9 +181,12 @@ createRuleMapping(const std::vector<const PathDiagnostic *> &Diags,
return RuleMapping;
}
-static SarifResult createResult(const PathDiagnostic *Diag,
- const StringMap<uint32_t> &RuleMapping,
- const LangOptions &LO) {
+static const llvm::StringRef IssueHashKey = "clang/issueHash/v1";
+
+SarifResult
+SarifDiagnostics::createResult(const PathDiagnostic *Diag,
+ const StringMap<uint32_t> &RuleMapping,
+ const LangOptions &LO, FilesMade *FM) {
StringRef CheckName = Diag->getCheckerName();
uint32_t RuleIdx = RuleMapping.lookup(CheckName);
@@ -183,17 +194,40 @@ static SarifResult createResult(const PathDiagnostic *Diag,
Diag->getLocation().asRange(), Diag->getLocation().getManager(), LO);
SmallVector<ThreadFlow, 8> Flows = createThreadFlows(Diag, LO);
+
+ auto IssueHash = Diag->getIssueHash(SM, LO);
+
+ std::string HtmlReportURL;
+ if (FM && !FM->empty()) {
+ // Find the HTML report that was generated for this issue, if one exists.
+ PDFileEntry::ConsumerFiles *Files = FM->getFiles(*Diag);
+ if (Files) {
+ auto HtmlFile =
+ std::find_if(Files->cbegin(), Files->cend(), [](auto &File) {
+ return File.first == HTML_DIAGNOSTICS_NAME;
+ });
+ if (HtmlFile != Files->cend()) {
+ SmallString<128> HtmlReportPath =
+ llvm::sys::path::parent_path(OutputFile);
+ llvm::sys::path::append(HtmlReportPath, HtmlFile->second);
+ HtmlReportURL = SarifDocumentWriter::fileNameToURI(HtmlReportPath);
+ }
+ }
+ }
+
auto Result = SarifResult::create(RuleIdx)
.setRuleId(CheckName)
.setDiagnosticMessage(Diag->getVerboseDescription())
.setDiagnosticLevel(SarifResultLevel::Warning)
.setLocations({Range})
+ .addPartialFingerprint(IssueHashKey, IssueHash)
+ .setHostedViewerURI(HtmlReportURL)
.setThreadFlows(Flows);
return Result;
}
void SarifDiagnostics::FlushDiagnosticsImpl(
- std::vector<const PathDiagnostic *> &Diags, FilesMade *) {
+ std::vector<const PathDiagnostic *> &Diags, FilesMade *FM) {
// We currently overwrite the file if it already exists. However, it may be
// useful to add a feature someday that allows the user to append a run to an
// existing SARIF file. One danger from that approach is that the size of the
@@ -210,7 +244,7 @@ void SarifDiagnostics::FlushDiagnosticsImpl(
SarifWriter.createRun("clang", "clang static analyzer", ToolVersion);
StringMap<uint32_t> RuleMapping = createRuleMapping(Diags, SarifWriter);
for (const PathDiagnostic *D : Diags) {
- SarifResult Result = createResult(D, RuleMapping, LO);
+ SarifResult Result = createResult(D, RuleMapping, LO, FM);
SarifWriter.appendResult(Result);
}
auto Document = SarifWriter.createDocument();
diff --git a/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif b/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
index 0bded6f0925d1..76f25475e3b21 100644
--- a/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
+++ b/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
@@ -4,9 +4,10 @@
{
"artifacts": [
{
- "length": 425,
+ "length": -1,
"location": {
"index": 0,
+ "uri": "file:///[...]/sarif-diagnostics-taint-test.c"
},
"mimeType": "text/plain",
"roles": [
@@ -31,6 +32,7 @@
"physicalLocation": {
"artifactLocation": {
"index": 0,
+ "uri": "file:///[...]/sarif-diagnostics-taint-test.c"
},
"region": {
"endColumn": 6,
@@ -50,6 +52,7 @@
"physicalLocation": {
"artifactLocation": {
"index": 0,
+ "uri": "file:///[...]/sarif-diagnostics-taint-test.c"
},
"region": {
"endColumn": 18,
@@ -71,6 +74,7 @@
"physicalLocation": {
"artifactLocation": {
"index": 0,
+ "uri": "file:///[...]/sarif-diagnostics-taint-test.c"
},
"region": {
"endColumn": 18,
@@ -84,6 +88,9 @@
"message": {
"text": "tainted"
},
+ "partialFingerprints": {
+ "clang/issueHash/v1": "5c964815b8d6db3989bacdd308e657d0"
+ },
"ruleId": "debug.TaintTest",
"ruleIndex": 0
}
@@ -108,8 +115,10 @@
"name": "debug.TaintTest"
}
],
+ "version": "[clang version]"
}
}
}
],
+ "version": "[SARIF version]"
}
diff --git a/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif b/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
index e35ab695bb38e..4aa6239f6312d 100644
--- a/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ b/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -4,9 +4,10 @@
{
"artifacts": [
{
- "length": 1152,
+ "length": -1,
"location": {
"index": 0,
+ "uri": "file:///[...]/sarif-multi-diagnostic-test.c"
},
"mimeType": "text/plain",
"roles": [
@@ -31,6 +32,7 @@
"physicalLocation": {
"artifactLocation": {
"index": 0,
+ "uri": "file:///[...]/sarif-multi-diagnostic-test.c"
},
"region": {
"endColumn": 6,
@@ -50,6 +52,7 @@
"physicalLocation": {
"artifactLocation": {
"index": 0,
+ "uri": "file:///[...]/sarif-multi-diagnostic-test.c"
},
"region": {
"endColumn": 18,
@@ -65,12 +68,14 @@
]
}
],
+ "hostedViewerUri": "file:///[...]/report-5c9648.html",
"level": "warning",
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"index": 0,
+ "uri": "file:///[...]/sarif-multi-diagnostic-test.c"
},
"region": {
"endColumn": 18,
@@ -84,6 +89,9 @@
"message": {
"text": "tainted"
},
+ "partialFingerprints": {
+ "clang/issueHash/v1": "5c964815b8d6db3989bacdd308e657d0"
+ },
"ruleId": "debug.TaintTest",
"ruleIndex": 0
},
@@ -102,6 +110,7 @@
"physicalLocation": {
"artifactLocation": {
"index": 0,
+ "uri": "file:///[...]/sarif-multi-diagnostic-test.c"
},
"region": {
"endColumn": 6,
@@ -121,6 +130,7 @@
"physicalLocation": {
"artifactLocation": {
"index": 0,
+ "uri": "file:///[...]/sarif-multi-diagnostic-test.c"
},
"region": {
"endColumn": 12,
@@ -140,6 +150,7 @@
"physicalLocation": {
"artifactLocation": {
"index": 0,
+ "uri": "file:///[...]/sarif-multi-diagnostic-test.c"
},
"regio...
[truncated]
|
|
Im off this week. I resign. |
ziqingluo-90
approved these changes
Sep 24, 2025
There was a problem hiding this comment.
Looks good to me. Moving getIssueHash out of HTMLDiagnostics is good since it is not something HTML-specific.
If I have to nitpick, I would suggest to keep the directories that are relative to the llvm repo in hostedViewerUri. (I guess this might not be an trivial task with sed though?)
It would be tricky, and I wasn't sure if the all of the file URLs would always point into the LLVM repo anyway. I could probably leave at least the last directory component, though. If it's OK, I'll attempt that in a future PR. I've got a bunch more SARIF-related PRs to open in the next few weeks. |
|
The as |
I post #161242 to fix it. |
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
This change adds two new properties to each `result` object in the SARIF log: `partialFingerprints`: Contains the "issue hash" that the analyzer already generates for each result, which can help identify a result across runs even if surrounding code changes. `hostedViewUri`: If running with `-analyzer-format=sarif-html`, this property will now be emitted with the `file:` URL of the generated HTML report for that result. Along the way, I discovered an existing bug where the HTML diagnostic consumer does not record the path to the generated report if another compilation already created that report. This caused both the SARIF and Plist consumers to be missing the link to the file in all but one of the compilations in case of a warning in a header file. I added a new test to ensure that the generated SARIF for each compilation contains the property. Finally, I made a few changes to the `normalize_sarif` processing in the tests. I switched to `sed` to allow substitutions. The normalization now removes directory components from `file:` URLs, replaces the `length` property of the source file with a constant `-1`, and puts placeholders in the values of the `version` properties rather than just deleting them. The URL transformation in particular lets us verify that the right filename is generated for each HTML report. Fixes llvm#158159 rdar://160410408
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds two new properties to each
resultobject in the SARIF log:partialFingerprints: Contains the "issue hash" that the analyzer already generates for each result, which can help identify a result across runs even if surrounding code changes.hostedViewUri: If running with-analyzer-format=sarif-html, this property will now be emitted with thefile:URL of the generated HTML report for that result.Along the way, I discovered an existing bug where the HTML diagnostic consumer does not record the path to the generated report if another compilation already created that report. This caused both the SARIF and Plist consumers to be missing the link to the file in all but one of the compilations in case of a warning in a header file. I added a new test to ensure that the generated SARIF for each compilation contains the property.
Finally, I made a few changes to the
normalize_sarifprocessing in the tests. I switched tosedto allow substitutions. The normalization now removes directory components fromfile:URLs, replaces thelengthproperty of the source file with a constant-1, and puts placeholders in the values of theversionproperties rather than just deleting them. The URL transformation in particular lets us verify that the right filename is generated for each HTML report.Fixes #158159
rdar://160410408