[llvm-branch-commits] [clang] 86b9476 - Revert "[analyzer] Model strchr/strrchr/memchr/strstr/strpbrk/strchrnul (#207267)"
Tobias Hieta via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Jul 26 11:21:04 PDT 2026
Author: Balazs Benics
Date: 2026-07-26T20:20:53+02:00
New Revision: 86b9476d9d2b8782d32007b61dac0cdaac56d1f9
URL: https://github.com/llvm/llvm-project/commit/86b9476d9d2b8782d32007b61dac0cdaac56d1f9
DIFF: https://github.com/llvm/llvm-project/commit/86b9476d9d2b8782d32007b61dac0cdaac56d1f9.diff
LOG: Revert "[analyzer] Model strchr/strrchr/memchr/strstr/strpbrk/strchrnul (#207267)"
This reverts commit a34cb573eae65f48f0e51147289e042a86b55d16.
This feature caused some issues (#209905), so the best course of
action is to postpone it to clang-24 and revert from this branch.
The #210154 tried to fix the surfaced issue, and considered to nominate
it for backport in #211832, but ultimately we had to revert it from
main in #211857 - this confirms that probably the best action is to
revert the half-baked feature from the release branch.
This means that #203260 won't be fixed in clang-23, and that's fine.
Added:
Modified:
clang/docs/ReleaseNotes.md
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
Removed:
clang/test/Analysis/string-search-modeling.c
################################################################################
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index f4043d6c64a71..2ea954b442678 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -1270,7 +1270,6 @@ latest release, please see the [Clang Web Site](https://clang.llvm.org) or the
- Added a new `check::LifetimeEnd` callback that fires for each `CFGLifetimeEnds` element, which is useful for detecting dangling pointers. (#GH201123)
- The `unix.StdCLibraryFunctions` standard-library summaries were optimized for binary size. (#GH202662)
- Fixed the alignment of entries printed by `clang -cc1 -analyzer-print-analyzer-options` / `-analyzer-help`. (#GH190570)
-- Improved the models of `strchr`/`strrchr`/`memchr`/`strstr`/`strpbrk`/`strchrnul`, enabling `core.StackAddressEscape` to catch dangling pointers returned by these functions. (#GH203260)
- Improved the modeling of symbolic ranges in the engine when calculating the largest and smallest possible values for range sets involving the `+`, `-`, and `*` binary operators. (#GH173113)
#### Moved checkers
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 745297dd1f057..32daa7045b12f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -28,7 +28,6 @@
#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>
@@ -83,7 +82,7 @@ class CStringChecker
: public CheckerFamily<eval::Call, check::PreStmt<DeclStmt>,
check::LiveSymbols, check::DeadSymbols,
check::RegionChanges> {
- mutable StringRef CurrentFunctionDescription;
+ mutable const char *CurrentFunctionDescription = nullptr;
public:
// FIXME: The bug types emitted by this checker family have confused garbage
@@ -163,24 +162,6 @@ 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)},
@@ -244,9 +225,6 @@ 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;
@@ -402,7 +380,7 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C,
if (NullArg.isEnabled()) {
SmallString<80> buf;
llvm::raw_svector_ostream OS(buf);
- assert(!CurrentFunctionDescription.empty());
+ assert(CurrentFunctionDescription);
OS << "Null pointer passed as " << (Arg.ArgumentIndex + 1)
<< llvm::getOrdinalSuffix(Arg.ArgumentIndex + 1) << " argument to "
<< CurrentFunctionDescription;
@@ -1067,7 +1045,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state,
if (NotNullTerm.isEnabled()) {
SmallString<120> buf;
llvm::raw_svector_ostream os(buf);
- assert(!CurrentFunctionDescription.empty());
+ assert(CurrentFunctionDescription);
os << "Argument to " << CurrentFunctionDescription
<< " is the address of the label '" << Label->getLabel()->getName()
<< "', which is not a null-terminated string";
@@ -1137,7 +1115,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state,
SmallString<120> buf;
llvm::raw_svector_ostream os(buf);
- assert(!CurrentFunctionDescription.empty());
+ assert(CurrentFunctionDescription);
os << "Argument to " << CurrentFunctionDescription << " is ";
if (SummarizeRegion(os, C.getASTContext(), MR))
@@ -2639,59 +2617,6 @@ void CStringChecker::evalStrsep(CheckerContext &C,
C.addTransition(State);
}
-void CStringChecker::evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
- StringRef FnName,
- bool CanReturnNull) const {
- CurrentFunctionDescription = FnName;
- const Expr *CE = Call.getOriginExpr();
- assert(CE);
-
- // These functions always return a pointer.
- 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);
- 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
- 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);
- 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
deleted file mode 100644
index a50ec439731a3..0000000000000
--- a/clang/test/Analysis/string-search-modeling.c
+++ /dev/null
@@ -1,178 +0,0 @@
-// 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}}
-}
More information about the llvm-branch-commits
mailing list