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 <[email protected]> Date: Fri, 19 Jun 2026 17:07:57 +0100 Subject: [PATCH 1/2] [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@-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@-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@-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@-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@-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@-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@-1 {{Null pointer passed as 1st argument to strchr()}} +} + +void null_source_strrchr(int c) { + strrchr(0, c); + // expected-warning@-1 {{Null pointer passed as 1st argument to strrchr()}} +} + +void null_source_memchr(int c) { + memchr(0, c, 4); + // expected-warning@-1 {{Null pointer passed as 1st argument to memchr()}} +} + +void null_source_strstr(void) { + strstr(0, "x"); + // expected-warning@-1 {{Null pointer passed as 1st argument to strstr()}} +} + +void null_source_strpbrk(void) { + strpbrk(0, "x"); + // expected-warning@-1 {{Null pointer passed as 1st argument to strpbrk()}} +} + +void null_source_strchrnul(int c) { + strchrnul(0, c); + // expected-warning@-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 <[email protected]> Date: Fri, 3 Jul 2026 14:54:25 +0100 Subject: [PATCH 2/2] 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); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
