[clang] [analyzer] Fix -analyzer-output=sarif crash on macro-expanded ranges (PR #214462)
Balázs Benics via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 10 08:55:22 PDT 2026
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/214462
>From d54a0a531dc08d1f19e94b909278280ea35ee5de Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Thu, 6 Aug 2026 11:04:59 +0100
Subject: [PATCH] [analyzer] Fix -analyzer-output=sarif crash on macro-expanded
ranges
A path piece whose range ends inside a macro expansion aborted the whole
document: https://godbolt.org/z/61vWYcsWj
Cannot create a physicalLocation from invalid SourceRange!
convertTokenRangeToCharRange() built the end with
Lexer::getLocForEndOfToken(), which returns an invalid location for a macro
ID that is not at the end of its expansion, and used it unchecked. The
analyzer's own test corpus hits this in nine files; text and plist output
were unaffected because both already map such ranges to the expansion.
- Use getExpansionRangeInFile(), so the region covers the macro use like the
other two outputs.
- Fall back to a caret when the range is unusable. A thread flow needs a
location per piece, so dropping one would truncate the reported path. This
also stops reversed ranges producing regions with endColumn < startColumn.
Single-token ranges keep their zero-width regions; widening them would churn
every expected-sarif file, so that is left for a separate change.
Assisted-By: claude
---
.../StaticAnalyzer/Core/SarifDiagnostics.cpp | 45 ++++++++++---------
.../diagnostics/sarif-macro-expansion.c | 35 +++++++++++++++
2 files changed, 58 insertions(+), 22 deletions(-)
create mode 100644 clang/test/Analysis/diagnostics/sarif-macro-expansion.c
diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
index c9f5774c7db7c..1d5139c067c7b 100644
--- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -18,6 +18,8 @@
#include "clang/Basic/Sarif.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/Version.h"
+#include "clang/Frontend/DiagnosticRenderer.h"
+#include "clang/Lex/Lexer.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
#include "llvm/ADT/StringMap.h"
@@ -124,23 +126,25 @@ calculateImportance(const PathDiagnosticPiece &Piece) {
return ThreadFlowImportance::Unimportant;
}
-/// Accepts a SourceRange corresponding to a pair of the first and last tokens
-/// and converts to a Character granular CharSourceRange.
-static CharSourceRange convertTokenRangeToCharRange(const SourceRange &R,
- const SourceManager &SM,
- const LangOptions &LO) {
- // Caret diagnostics have the first and last locations pointed at the same
- // location, return these as-is.
- if (R.getBegin() == R.getEnd())
- return CharSourceRange::getCharRange(R);
-
- SourceLocation BeginCharLoc = R.getBegin();
- // For token ranges, the raw end SLoc points at the first character of the
- // last token in the range. This must be moved to one past the end of the
- // last character using the lexer.
- SourceLocation EndCharLoc =
- Lexer::getLocForEndOfToken(R.getEnd(), /* Offset = */ 0, SM, LO);
- return CharSourceRange::getCharRange(BeginCharLoc, EndCharLoc);
+/// Returns the character range to report for \p Loc.
+///
+/// A thread flow needs a location for every piece, so an unusable range falls
+/// back to a caret rather than being dropped, which would truncate the path.
+static CharSourceRange getDisplayCharRange(const PathDiagnosticLocation &Loc,
+ const LangOptions &LO) {
+ const SourceManager &SM = Loc.getManager();
+ FullSourceLoc Caret = Loc.asLocation().getExpansionLoc();
+ SourceRange Range = Loc.asRange();
+
+ // FIXME: A single-token range is reported as a zero-width region. Widening it
+ // would churn every expected-sarif file, so it is left alone for now.
+ if (Range.getBegin() != Range.getEnd()) {
+ if (std::optional<CharSourceRange> FileRange = getExpansionRangeInFile(
+ CharSourceRange::getTokenRange(Range), Caret.getFileID(), SM))
+ return Lexer::getAsCharRange(*FileRange, SM, LO);
+ }
+
+ return CharSourceRange::getCharRange(Caret, Caret);
}
static SmallVector<ThreadFlow, 8> createThreadFlows(const PathDiagnostic *Diag,
@@ -148,11 +152,9 @@ static SmallVector<ThreadFlow, 8> createThreadFlows(const PathDiagnostic *Diag,
SmallVector<ThreadFlow, 8> Flows;
const PathPieces &Pieces = Diag->path.flatten(false);
for (const auto &Piece : Pieces) {
- auto Range = convertTokenRangeToCharRange(
- Piece->getLocation().asRange(), Piece->getLocation().getManager(), LO);
auto Flow = ThreadFlow::create()
.setImportance(calculateImportance(*Piece))
- .setRange(Range)
+ .setRange(getDisplayCharRange(Piece->getLocation(), LO))
.setMessage(Piece->getString());
Flows.push_back(Flow);
}
@@ -190,8 +192,7 @@ SarifDiagnostics::createResult(const PathDiagnostic *Diag,
StringRef CheckName = Diag->getCheckerName();
uint32_t RuleIdx = RuleMapping.lookup(CheckName);
- auto Range = convertTokenRangeToCharRange(
- Diag->getLocation().asRange(), Diag->getLocation().getManager(), LO);
+ CharSourceRange Range = getDisplayCharRange(Diag->getLocation(), LO);
SmallVector<ThreadFlow, 8> Flows = createThreadFlows(Diag, LO);
diff --git a/clang/test/Analysis/diagnostics/sarif-macro-expansion.c b/clang/test/Analysis/diagnostics/sarif-macro-expansion.c
new file mode 100644
index 0000000000000..3f8ee27b03a00
--- /dev/null
+++ b/clang/test/Analysis/diagnostics/sarif-macro-expansion.c
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc \
+// RUN: -analyzer-output=sarif -verify %s -o - | FileCheck %s
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
+#define ALLOC int *x = (int *)malloc(12);
+
+void ends_inside_expansion(void) {
+ ALLOC // no-crash
+} // expected-warning {{Potential leak of memory pointed to by 'x'}}
+
+// The note covers the 'ALLOC' use rather than reaching into the macro body.
+// CHECK: "text": "Memory is allocated"
+// CHECK: "region": {
+// CHECK-NEXT: "endColumn": 8,
+// CHECK-NEXT: "endLine": [[#ALLOC_LINE:]],
+// CHECK-NEXT: "startColumn": 3,
+// CHECK-NEXT: "startLine": [[#ALLOC_LINE]]
+
+#define IS_NULL !p
+
+void ends_at_end_of_expansion(int *p) {
+ if (IS_NULL)
+ *p = 1; // expected-warning {{Dereference of null pointer}}
+}
+
+// This range already ended at the end of the expansion, so it worked before.
+// CHECK: "text": "Assuming 'p' is null"
+// CHECK: "region": {
+// CHECK-NEXT: "endColumn": 14,
+// CHECK-NEXT: "endLine": [[#IS_NULL_LINE:]],
+// CHECK-NEXT: "startColumn": 7,
+// CHECK-NEXT: "startLine": [[#IS_NULL_LINE]]
+
More information about the cfe-commits
mailing list