[clang] [analyzer] Model strchr/strrchr/memchr/strstr/strpbrk/strchrnul (PR #207267)
Balázs Benics via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 3 10:04:26 PDT 2026
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/207267
>From 1e145a23e7e9ce45f081f1c6c866d41c88bb5936 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Fri, 19 Jun 2026 17:07:57 +0100
Subject: [PATCH 1/3] [analyzer] Model
strchr/strrchr/memchr/strstr/strpbrk/strchrnul
CStringChecker did not model these buffer-search functions, so the engine
bound each call to a fresh conjured symbol unrelated to the source.
As a consequence, we could not track the origin of the returned pointer,
thus the fact that it shares provenance of the source pointer.
Fixes #203260
Assisted-by: Claude Opus 4.8
---
.../Checkers/CStringChecker.cpp | 78 +++++++-
clang/test/Analysis/string-search-modeling.c | 178 ++++++++++++++++++
2 files changed, 252 insertions(+), 4 deletions(-)
create mode 100644 clang/test/Analysis/string-search-modeling.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 32daa7045b12f..1a50e10b50ed6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -28,6 +28,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/STLForwardCompat.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/raw_ostream.h"
#include <functional>
@@ -82,7 +83,7 @@ class CStringChecker
: public CheckerFamily<eval::Call, check::PreStmt<DeclStmt>,
check::LiveSymbols, check::DeadSymbols,
check::RegionChanges> {
- mutable const char *CurrentFunctionDescription = nullptr;
+ mutable StringRef CurrentFunctionDescription;
public:
// FIXME: The bug types emitted by this checker family have confused garbage
@@ -162,6 +163,24 @@ class CStringChecker
{{CDM::CLibrary, {"strncasecmp"}, 3}, &CStringChecker::evalStrncasecmp},
{{CDM::CLibrary, {"strsep"}, 2}, &CStringChecker::evalStrsep},
{{CDM::CLibrary, {"strxfrm"}, 3}, &CStringChecker::evalStrxfrm},
+ {{CDM::CLibraryMaybeHardened, {"strchr"}, 2},
+ llvm::bind_back(&CStringChecker::evalStrchrCommon, "strchr()",
+ /*CanReturnNull=*/true)},
+ {{CDM::CLibraryMaybeHardened, {"strrchr"}, 2},
+ llvm::bind_back(&CStringChecker::evalStrchrCommon, "strrchr()",
+ /*CanReturnNull=*/true)},
+ {{CDM::CLibraryMaybeHardened, {"memchr"}, 3},
+ llvm::bind_back(&CStringChecker::evalStrchrCommon, "memchr()",
+ /*CanReturnNull=*/true)},
+ {{CDM::CLibrary, {"strstr"}, 2},
+ llvm::bind_back(&CStringChecker::evalStrchrCommon, "strstr()",
+ /*CanReturnNull=*/true)},
+ {{CDM::CLibrary, {"strpbrk"}, 2},
+ llvm::bind_back(&CStringChecker::evalStrchrCommon, "strpbrk()",
+ /*CanReturnNull=*/true)},
+ {{CDM::CLibrary, {"strchrnul"}, 2},
+ llvm::bind_back(&CStringChecker::evalStrchrCommon, "strchrnul()",
+ /*CanReturnNull=*/false)},
{{CDM::CLibrary, {"bcopy"}, 3}, &CStringChecker::evalBcopy},
{{CDM::CLibrary, {"bcmp"}, 3},
std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)},
@@ -225,6 +244,9 @@ class CStringChecker
void evalStrsep(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
+ StringRef FnName, bool CanReturnNull) const;
+
void evalStdCopy(CheckerContext &C, const CallEvent &Call) const;
void evalStdCopyBackward(CheckerContext &C, const CallEvent &Call) const;
void evalStdCopyCommon(CheckerContext &C, const CallEvent &Call) const;
@@ -380,7 +402,7 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C,
if (NullArg.isEnabled()) {
SmallString<80> buf;
llvm::raw_svector_ostream OS(buf);
- assert(CurrentFunctionDescription);
+ assert(!CurrentFunctionDescription.empty());
OS << "Null pointer passed as " << (Arg.ArgumentIndex + 1)
<< llvm::getOrdinalSuffix(Arg.ArgumentIndex + 1) << " argument to "
<< CurrentFunctionDescription;
@@ -1045,7 +1067,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state,
if (NotNullTerm.isEnabled()) {
SmallString<120> buf;
llvm::raw_svector_ostream os(buf);
- assert(CurrentFunctionDescription);
+ assert(!CurrentFunctionDescription.empty());
os << "Argument to " << CurrentFunctionDescription
<< " is the address of the label '" << Label->getLabel()->getName()
<< "', which is not a null-terminated string";
@@ -1115,7 +1137,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state,
SmallString<120> buf;
llvm::raw_svector_ostream os(buf);
- assert(CurrentFunctionDescription);
+ assert(!CurrentFunctionDescription.empty());
os << "Argument to " << CurrentFunctionDescription << " is ";
if (SummarizeRegion(os, C.getASTContext(), MR))
@@ -2617,6 +2639,54 @@ void CStringChecker::evalStrsep(CheckerContext &C,
C.addTransition(State);
}
+void CStringChecker::evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
+ StringRef FnName,
+ bool CanReturnNull) const {
+ CurrentFunctionDescription = FnName;
+ ProgramStateRef State = C.getState();
+ const StackFrame *SF = C.getStackFrame();
+ SValBuilder &SVB = C.getSValBuilder();
+ ASTContext &Ctx = C.getASTContext();
+ const Expr *CE = Call.getOriginExpr();
+ assert(CE);
+
+ // These functions always return a pointer.
+ if (!CE->getType()->isPointerType())
+ return;
+
+ // The first argument must be non-null for all functions in this family.
+ SourceArgExpr Src = {{Call.getArgExpr(0), 0}};
+ SVal SrcVal = State->getSVal(Src.Expression, SF);
+ State = checkNonNull(C, State, Src, SrcVal);
+ if (!State)
+ return;
+
+ // NULL (no-match) branch.
+ if (CanReturnNull) {
+ ProgramStateRef NullState =
+ State->BindExpr(CE, SF, SVB.makeNullWithType(CE->getType()));
+ C.addTransition(NullState);
+ }
+
+ // Found branch: a pointer within the source; needs a Loc for the arithmetic.
+ std::optional<Loc> SrcLoc = SrcVal.getAs<Loc>();
+ if (!SrcLoc) {
+ SVal Result = SVB.conjureSymbolVal(Call, C.blockCount());
+ State = State->BindExpr(CE, SF, Result);
+ C.addTransition(State);
+ return;
+ }
+
+ // The result is: Src + SymOffset
+ NonLoc SymOffset =
+ SVB.conjureSymbolVal(Call, Ctx.getSizeType(), C.blockCount())
+ .castAs<NonLoc>();
+ SVal Result = SVB.evalBinOpLN(State, BO_Add, *SrcLoc, SymOffset,
+ Src.Expression->getType());
+ State = State->BindExpr(CE, SF, Result);
+ C.addTransition(State);
+}
+
// These should probably be moved into a C++ standard library checker.
void CStringChecker::evalStdCopy(CheckerContext &C,
const CallEvent &Call) const {
diff --git a/clang/test/Analysis/string-search-modeling.c b/clang/test/Analysis/string-search-modeling.c
new file mode 100644
index 0000000000000..a50ec439731a3
--- /dev/null
+++ b/clang/test/Analysis/string-search-modeling.c
@@ -0,0 +1,178 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core,unix \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config eagerly-assume=false
+
+typedef __SIZE_TYPE__ size_t;
+void *malloc(size_t size);
+void free(void *p);
+void *memcpy(void *dest, const void *src, size_t n);
+char *strchr(const char *s, int c);
+char *strrchr(const char *s, int c);
+char *strstr(const char *haystack, const char *needle);
+char *strpbrk(const char *s, const char *accept);
+void *memchr(const void *s, int c, size_t n);
+char *strchrnul(const char *s, int c);
+
+void clang_analyzer_eval(int);
+
+//===----------------------------------------------------------------------===//
+// Check for stack address escapes.
+//===----------------------------------------------------------------------===//
+
+char *returns_stack_strchr(void) {
+ char buf[8] = "abc";
+ return strchr(buf, 'b');
+ // expected-warning at -1 {{Address of stack memory associated with local variable 'buf' returned to caller}}
+}
+
+char *returns_stack_strrchr(void) {
+ char buf[8] = "abc";
+ return strrchr(buf, 'b');
+ // expected-warning at -1 {{Address of stack memory associated with local variable 'buf' returned to caller}}
+}
+
+char *returns_stack_strstr(void) {
+ char buf[8] = "abc";
+ return strstr(buf, "b");
+ // expected-warning at -1 {{Address of stack memory associated with local variable 'buf' returned to caller}}
+}
+
+char *returns_stack_strpbrk(void) {
+ char buf[8] = "abc";
+ return strpbrk(buf, "b");
+ // expected-warning at -1 {{Address of stack memory associated with local variable 'buf' returned to caller}}
+}
+
+void *returns_stack_memchr(void) {
+ char buf[8] = "abc";
+ return memchr(buf, 'b', sizeof buf);
+ // expected-warning at -1 {{Address of stack memory associated with local variable 'buf' returned to caller}}
+}
+
+char *returns_stack_strchrnul(void) {
+ char buf[8] = "abc";
+ return strchrnul(buf, 'b');
+ // expected-warning at -1 {{Address of stack memory associated with local variable 'buf' returned to caller}}
+}
+
+char *forwards_param(char *p) {
+ return strchr(p, 'b'); // no-warning
+}
+
+char *returns_local_static(void) {
+ extern char g[8];
+ return strchr(g, 'b'); // no-warning
+}
+
+//===----------------------------------------------------------------------===//
+// unix.cstring.NullArg: the source pointer must be non-null.
+//===----------------------------------------------------------------------===//
+
+void null_source_strchr(int c) {
+ strchr(0, c);
+ // expected-warning at -1 {{Null pointer passed as 1st argument to strchr()}}
+}
+
+void null_source_strrchr(int c) {
+ strrchr(0, c);
+ // expected-warning at -1 {{Null pointer passed as 1st argument to strrchr()}}
+}
+
+void null_source_memchr(int c) {
+ memchr(0, c, 4);
+ // expected-warning at -1 {{Null pointer passed as 1st argument to memchr()}}
+}
+
+void null_source_strstr(void) {
+ strstr(0, "x");
+ // expected-warning at -1 {{Null pointer passed as 1st argument to strstr()}}
+}
+
+void null_source_strpbrk(void) {
+ strpbrk(0, "x");
+ // expected-warning at -1 {{Null pointer passed as 1st argument to strpbrk()}}
+}
+
+void null_source_strchrnul(int c) {
+ strchrnul(0, c);
+ // expected-warning at -1 {{Null pointer passed as 1st argument to strchrnul()}}
+}
+
+//===----------------------------------------------------------------------===//
+// State split: result == NULL on one branch, in the source on the other.
+//===----------------------------------------------------------------------===//
+
+// Both branches are reachable; the verifier matches the two values set-wise.
+void state_split(const char *p) {
+ clang_analyzer_eval(strchr(p, 'b') == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ clang_analyzer_eval(strrchr(p, 'b') == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ clang_analyzer_eval(strstr(p, "x") == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ clang_analyzer_eval(strpbrk(p, "x") == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ clang_analyzer_eval(memchr(p, 'b', 4) == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+// strchrnul does not split: it never returns NULL at runtime.
+void strchrnul_is_nonnull(const char *p) {
+ clang_analyzer_eval(strchrnul(p, 'b') == 0); // expected-warning {{FALSE}}
+}
+
+// On the "found" branch the result aliases the source region, but the offset
+// is opaque, so equality with in-source pointers is UNKNOWN.
+void found_branch_offset_is_opaque(const char *p) {
+ char *q = strchr(p, 'b');
+ if (!q) return; // constrain to "found" branch
+ clang_analyzer_eval(q == p); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(q == p + 1); // expected-warning {{UNKNOWN}}
+}
+
+void resulting_ptr_shares_provenance_with_src(int rng, char *opaque) {
+ if (rng == 10) {
+ char *q = strchr("abcd", 'b');
+ free(q); // expected-warning {{Argument to 'free()' is the address of a global variable, which is not memory allocated by 'malloc()'}}
+ return;
+ }
+
+ if (rng == 20) {
+ char *q = strchr(opaque, 'b');
+ free(q); // ok
+ return;
+ }
+
+ if (rng == 30) {
+ char *q = strchr(opaque, 'b');
+ free(q); // Notionally releases 'opaque'.
+ free(opaque); // expected-warning {{Attempt to release already released memory}}
+ return;
+ }
+}
+
+//===----------------------------------------------------------------------===//
+// core.NullDereference:
+// A returned pointer used without a NULL check is flagged on the NULL branch.
+//===----------------------------------------------------------------------===//
+
+void deref_unchecked(const char *s) {
+ char *p = strchr(s, 'b');
+ *p = 'X'; // expected-warning {{Dereference of null pointer}}
+}
+
+void deref_after_check(const char *s) {
+ char *p = strchr(s, 'b');
+ if (p) {
+ *p = 'X'; // no-warning
+ }
+}
+
+//===----------------------------------------------------------------------===//
+// Calling these functions does not invalidate unrelated memory.
+//===----------------------------------------------------------------------===//
+
+int global_unmodified;
+void no_invalidation_of_globals(const char *p) {
+ int local_unmodified = 10;
+ global_unmodified = 20;
+ (void)strchr(p, 'b');
+ clang_analyzer_eval(local_unmodified == 10); // expected-warning {{TRUE}}
+ clang_analyzer_eval(global_unmodified == 20); // expected-warning {{TRUE}}
+}
>From 0509a8ff9ae3bcf293e54aab8493611ceeed5cb4 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Fri, 3 Jul 2026 14:54:25 +0100
Subject: [PATCH 2/3] Move variables close to use-site
---
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 1a50e10b50ed6..babf974e5c1d7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2643,10 +2643,6 @@ void CStringChecker::evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
StringRef FnName,
bool CanReturnNull) const {
CurrentFunctionDescription = FnName;
- ProgramStateRef State = C.getState();
- const StackFrame *SF = C.getStackFrame();
- SValBuilder &SVB = C.getSValBuilder();
- ASTContext &Ctx = C.getASTContext();
const Expr *CE = Call.getOriginExpr();
assert(CE);
@@ -2654,6 +2650,11 @@ void CStringChecker::evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
if (!CE->getType()->isPointerType())
return;
+ ProgramStateRef State = C.getState();
+ const StackFrame *SF = C.getStackFrame();
+ SValBuilder &SVB = C.getSValBuilder();
+ ASTContext &Ctx = C.getASTContext();
+
// The first argument must be non-null for all functions in this family.
SourceArgExpr Src = {{Call.getArgExpr(0), 0}};
SVal SrcVal = State->getSVal(Src.Expression, SF);
>From 6a1e926e5ad44de0fa3ccca22d2c78e4bbfe32ef Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Fri, 3 Jul 2026 18:02:26 +0100
Subject: [PATCH 3/3] Add the dead-code about adding constraint to SymOffset
This is dead-code because we are trying to bind a constraint to
UnknownVal - which is a no-op.
Addresses:
https://github.com/llvm/llvm-project/pull/207267#discussion_r3520483401
---
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index babf974e5c1d7..745297dd1f057 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2679,9 +2679,13 @@ void CStringChecker::evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
}
// The result is: Src + SymOffset
+ auto RemainingExtentBytes =
+ getDynamicExtentWithOffset(State, *SrcLoc).castAs<DefinedOrUnknownSVal>();
NonLoc SymOffset =
SVB.conjureSymbolVal(Call, Ctx.getSizeType(), C.blockCount())
.castAs<NonLoc>();
+ State = State->assumeInBound(SymOffset, RemainingExtentBytes, true);
+
SVal Result = SVB.evalBinOpLN(State, BO_Add, *SrcLoc, SymOffset,
Src.Expression->getType());
State = State->BindExpr(CE, SF, Result);
More information about the cfe-commits
mailing list