https://github.com/kmehltretter82 created
https://github.com/llvm/llvm-project/pull/229316
The analyzer ends a path at a block whose terminator is an asm goto
("TODO: Handle jumping to labels" in CoreEngine::HandleBlockExit()).
Linux kernel static keys compile to an asm goto with CONFIG_JUMP_LABEL.
Every tracepoint has one, and with allocation profiling every kmalloc()
has one too, so kernel paths end at the first of them.
An asm goto is a block terminator and never reaches VisitGCCAsmStmt().
Invalidate its operands the same way, through a shared helper, and
continue at every successor.
The three FIXMEs in egraph-asm-goto-no-crash.cpp become expected
warnings. asm-goto-paths.c is new.
370 files of an x86_64 allmodconfig kernel (drivers/leds, sound/isa,
fs/ext4, net/ipv4), core checkers:
before after
analysis time, summed over files 858 s 973 s
wall clock, 4 jobs 236 s 265 s
reports 54 77
step limit reached 320 396
25 reports in 10 files are new, all in code that was not reached before.
24 are existing false positives, one is a harmless uninitialized read
(fs/ext4/extents.c:3158). None comes from the asm goto handling itself.
2 reports are gone. One function is now inlined into its caller and no
longer analyzed alone, and one path no longer fits the node budget.
Assisted-by: LLM
Fixes #229302
>From 3e0837ebbecddb7054eb8f7fd7227b9b7422d981 Mon Sep 17 00:00:00 2001
From: Karl Mehltretter <[email protected]>
Date: Sat, 3 Oct 2026 22:39:36 +0000
Subject: [PATCH] [analyzer] Follow the successors of an asm goto
The engine stopped at a block that ends in an asm goto, with a TODO. No
path that reaches one was analyzed any further.
In the Linux kernel that is most paths. A static branch is an asm goto,
and one sits in every kmalloc() (memory allocation profiling), in every
tracepoint and behind every static key. A seeded kernel module showed
the effect: a kmalloc(GFP_KERNEL) under a spinlock went unreported
because the path ended inside mem_alloc_profiling_enabled().
Nothing is known about what the assembly does. Invalidate the regions
its outputs and inputs refer to, as VisitGCCAsmStmt() does for a plain
asm statement (an asm goto is only the terminator of its block, so it
never passes through VisitGCCAsmStmt()), and continue at every
successor, the fallthrough and each label. This is what the FIXMEs in
egraph-asm-goto-no-crash.cpp ask for.
Assisted-by: LLM
---
.../Core/PathSensitive/ExprEngine.h | 15 +++
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp | 11 +-
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 64 +++++++---
clang/test/Analysis/asm-goto-paths.c | 110 ++++++++++++++++++
.../Analysis/egraph-asm-goto-no-crash.cpp | 11 +-
5 files changed, 185 insertions(+), 26 deletions(-)
create mode 100644 clang/test/Analysis/asm-goto-paths.c
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index af813714e6831..ce98616549c56 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -387,6 +387,12 @@ class ExprEngine {
void processIndirectGoto(ExplodedNodeSet &Dst, const Expr *Tgt,
const CFGBlock *Dispatch, ExplodedNode *Pred);
+ /// processAsmGoto - Called by CoreEngine. Used to generate successor
+ /// nodes for an asm goto, which may fall through or jump to any of its
+ /// labels. The operands are invalidated as for a plain asm statement.
+ void processAsmGoto(const GCCAsmStmt *A, const CFGBlock *B,
+ ExplodedNode *Pred, ExplodedNodeSet &Dst);
+
/// ProcessSwitch - Called by CoreEngine. Used to generate successor
/// nodes by processing the 'effects' of a switch statement.
void processSwitch(const SwitchStmt *Switch, ExplodedNode *Pred,
@@ -489,6 +495,15 @@ class ExprEngine {
void VisitGCCAsmStmt(const GCCAsmStmt *A, ExplodedNode *Pred,
ExplodedNodeSet &Dst);
+ /// Invalidate the regions that the outputs and inputs of an inline asm
+ /// statement refer to. Nothing is known about what the assembly does with
+ /// them.
+ ProgramStateRef invalidateAsmOperands(const GCCAsmStmt *A,
+ ProgramStateRef State,
+ ConstCFGElementRef Elem,
+ unsigned BlockCount,
+ const StackFrame *SF);
+
/// VisitMSAsmStmt - Transfer function logic for MS inline asm.
void VisitMSAsmStmt(const MSAsmStmt *A, ExplodedNode *Pred,
ExplodedNodeSet &Dst);
diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
index 04b700726fbc6..9ad83b76f1554 100644
--- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -460,10 +460,15 @@ void CoreEngine::HandleBlockExit(const CFGBlock * B,
ExplodedNode *Pred) {
HandleBranch(cast<WhileStmt>(Term)->getCond(), Term, B, Pred);
return;
- case Stmt::GCCAsmStmtClass:
- assert(cast<GCCAsmStmt>(Term)->isAsmGoto() && "Encountered GCCAsmStmt
without labels");
- // TODO: Handle jumping to labels
+ case Stmt::GCCAsmStmtClass: {
+ // An asm goto may fall through or jump to any of its labels.
+ assert(cast<GCCAsmStmt>(Term)->isAsmGoto() &&
+ "Encountered GCCAsmStmt without labels");
+ ExplodedNodeSet Dst;
+ ExprEng.processAsmGoto(cast<GCCAsmStmt>(Term), B, Pred, Dst);
+ enqueue(Dst);
return;
+ }
}
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 059bc4770da91..6d010291e8406 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -66,6 +66,7 @@
#include "llvm/ADT/ImmutableMap.h"
#include "llvm/ADT/ImmutableSet.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/Compiler.h"
@@ -3926,40 +3927,73 @@ bool
ExprEngine::didEagerlyAssumeBifurcateAt(ProgramStateRef State,
return Ex && State->get<LastEagerlyAssumeExprIfSuccessful>() == Ex;
}
-void ExprEngine::VisitGCCAsmStmt(const GCCAsmStmt *A, ExplodedNode *Pred,
- ExplodedNodeSet &Dst) {
- // We have processed both the inputs and the outputs. All of the outputs
- // should evaluate to Locs. Nuke all of their values.
+ProgramStateRef ExprEngine::invalidateAsmOperands(const GCCAsmStmt *A,
+ ProgramStateRef State,
+ ConstCFGElementRef Elem,
+ unsigned BlockCount,
+ const StackFrame *SF) {
+ // All of the outputs should evaluate to Locs. Nuke all of their values.
// FIXME: Some day in the future it would be nice to allow a "plug-in"
// which interprets the inline asm and stores proper results in the
// outputs.
- ProgramStateRef state = Pred->getState();
-
for (const Expr *O : A->outputs()) {
- SVal X = state->getSVal(O, Pred->getStackFrame());
+ SVal X = State->getSVal(O, SF);
assert(!isa<NonLoc>(X)); // Should be an Lval, or unknown, undef.
if (std::optional<Loc> LV = X.getAs<Loc>())
- state = state->invalidateRegions(*LV, getCFGElementRef(),
- getNumVisitedCurrent(),
- Pred->getStackFrame(),
+ State = State->invalidateRegions(*LV, Elem, BlockCount, SF,
/*CausedByPointerEscape=*/true);
}
// Do not reason about locations passed inside inline assembly.
for (const Expr *I : A->inputs()) {
- SVal X = state->getSVal(I, Pred->getStackFrame());
+ SVal X = State->getSVal(I, SF);
if (std::optional<Loc> LV = X.getAs<Loc>())
- state = state->invalidateRegions(*LV, getCFGElementRef(),
- getNumVisitedCurrent(),
- Pred->getStackFrame(),
+ State = State->invalidateRegions(*LV, Elem, BlockCount, SF,
/*CausedByPointerEscape=*/true);
}
- Dst.insert(Engine.makePostStmtNode(A, state, Pred));
+ return State;
+}
+
+void ExprEngine::VisitGCCAsmStmt(const GCCAsmStmt *A, ExplodedNode *Pred,
+ ExplodedNodeSet &Dst) {
+ // We have processed both the inputs and the outputs.
+ ProgramStateRef State =
+ invalidateAsmOperands(A, Pred->getState(), getCFGElementRef(),
+ getNumVisitedCurrent(), Pred->getStackFrame());
+
+ Dst.insert(Engine.makePostStmtNode(A, State, Pred));
+}
+
+void ExprEngine::processAsmGoto(const GCCAsmStmt *A, const CFGBlock *B,
+ ExplodedNode *Pred, ExplodedNodeSet &Dst) {
+ ProgramStateRef State = Pred->getState();
+ const StackFrame *SF = Pred->getStackFrame();
+
+ // An asm goto is the terminator of its block, not one of its elements, so
+ // its operands do not pass through VisitGCCAsmStmt. Invalidate them here.
+ // A terminator has no CFG element of its own, so the new values are
+ // conjured at the last element of its block. Parts of an operand, such as
+ // the arms of a conditional, may have been evaluated in earlier blocks.
+ // Without operands the block can be empty, and nothing is invalidated.
+ if (!B->empty())
+ State =
+ invalidateAsmOperands(A, State, ConstCFGElementRef(B, B->size() - 1),
+ getNumVisited(SF, B), SF);
+
+ // Nothing is known about what the assembly does, so it may fall through or
+ // jump to any of its labels. Follow every successor once.
+ llvm::SmallPtrSet<const CFGBlock *, 4> Followed;
+ for (const CFGBlock::AdjacentBlock &Succ : B->succs()) {
+ const CFGBlock *Next = Succ.getReachableBlock();
+ if (!Next || !Followed.insert(Next).second)
+ continue;
+ Dst.insert(Engine.makeNode(BlockEdge(B, Next, SF), State, Pred));
+ }
}
void ExprEngine::VisitMSAsmStmt(const MSAsmStmt *A, ExplodedNode *Pred,
diff --git a/clang/test/Analysis/asm-goto-paths.c
b/clang/test/Analysis/asm-goto-paths.c
new file mode 100644
index 0000000000000..e863d5f115724
--- /dev/null
+++ b/clang/test/Analysis/asm-goto-paths.c
@@ -0,0 +1,110 @@
+// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu \
+// RUN: -analyzer-checker=core,debug.ExprInspection \
+// RUN: -analyzer-config eagerly-assume=false -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached(void);
+
+// The shape of a Linux kernel static branch.
+static inline int static_branch(const int *key) {
+ asm goto("jmp %l[l_yes]" : : "i"(key) : : l_yes);
+ return 0;
+l_yes:
+ return 1;
+}
+
+int key;
+
+void both_ways(void) {
+ if (static_branch(&key))
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+int null_after_asm_goto(int *p) {
+ if (static_branch(&key))
+ p = 0;
+ return *p; // expected-warning {{Dereference of null pointer (loaded from
variable 'p')}}
+}
+
+void outputs_are_forgotten(void) {
+ int x = 1;
+
+ asm goto("" : "=r"(x) : : : out);
+ clang_analyzer_eval(x == 1); // expected-warning {{UNKNOWN}}
+ return;
+out:
+ clang_analyzer_eval(x == 1); // expected-warning {{UNKNOWN}}
+}
+
+struct S { int a; };
+
+void struct_outputs_are_forgotten(void) {
+ struct S s = {1};
+
+ asm goto("" : "=m"(s) : : : out);
+ clang_analyzer_eval(s.a == 1); // expected-warning {{UNKNOWN}}
+ return;
+out:
+ clang_analyzer_eval(s.a == 1); // expected-warning {{UNKNOWN}}
+}
+
+// Memory that an input points to may have been written, as for a plain asm.
+void input_pointees_are_forgotten(int *p) {
+ *p = 1;
+
+ asm goto("" : : "r"(p) : "memory" : out);
+ clang_analyzer_eval(*p == 1); // expected-warning {{UNKNOWN}}
+ return;
+out:
+ clang_analyzer_eval(*p == 1); // expected-warning {{UNKNOWN}}
+}
+
+// The arms of a conditional operand are evaluated in earlier blocks. Only
+// the memory behind the arm that was taken is forgotten.
+void conditional_operand(int c, int *p, int *q) {
+ *p = 1;
+ *q = 1;
+
+ asm goto("" : : "r"(c ? p : q) : "memory" : out);
+ clang_analyzer_eval(*p == 1); // expected-warning {{TRUE}}
+ // expected-warning@-1 {{UNKNOWN}}
+ clang_analyzer_eval(*q == 1); // expected-warning {{TRUE}}
+ // expected-warning@-1 {{UNKNOWN}}
+ return;
+out:
+ clang_analyzer_eval(*p == 1); // expected-warning {{TRUE}}
+ // expected-warning@-1 {{UNKNOWN}}
+ clang_analyzer_eval(*q == 1); // expected-warning {{TRUE}}
+ // expected-warning@-1 {{UNKNOWN}}
+}
+
+// A label before the asm goto makes a loop. The checks only run on the
+// second time round: the backward edge is followed, the output gets a new
+// value each time, and the path goes on behind the loop.
+void backward_jump(void) {
+ int x = 0;
+ int prev;
+ int count = 0;
+
+again:
+ prev = x;
+ count++;
+ asm goto("" : "=r"(x) : : : again);
+ if (count == 2) {
+ clang_analyzer_eval(prev == x); // expected-warning {{UNKNOWN}}
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ }
+}
+
+// No operands, so the block has no elements and there is nothing to
+// invalidate. Both ways are still reachable.
+void no_operands(void) {
+ asm goto("jmp %l[out]" : : : : out);
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ return;
+out:
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
diff --git a/clang/test/Analysis/egraph-asm-goto-no-crash.cpp
b/clang/test/Analysis/egraph-asm-goto-no-crash.cpp
index 37f8fc533abe3..a5b5729c7c83f 100644
--- a/clang/test/Analysis/egraph-asm-goto-no-crash.cpp
+++ b/clang/test/Analysis/egraph-asm-goto-no-crash.cpp
@@ -1,7 +1,5 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify
%s
-// expected-no-diagnostics
-
void clang_analyzer_warnIfReached();
void testAsmGoto() {
@@ -11,16 +9,13 @@ void testAsmGoto() {
: /* clobbers */
: label1, label2 /* any labels used */);
- // FIXME: Should be reachable.
- clang_analyzer_warnIfReached();
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
label1:
- // FIXME: Should be reachable.
- clang_analyzer_warnIfReached();
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
return;
label2:
- // FIXME: Should be reachable.
- clang_analyzer_warnIfReached();
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
return;
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
