[clang] [analyzer] Fix false positive in strchr/strrchr with constant args (PR #210154)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 17 05:28:22 PDT 2026
https://github.com/earnol updated https://github.com/llvm/llvm-project/pull/210154
>From 89a8ff8c898f06fe5c13951ed3db4e05d6501ef0 Mon Sep 17 00:00:00 2001
From: Vladislav Aranov <vladislav.aranov at ericsson.com>
Date: Thu, 16 Jul 2026 21:46:43 +0200
Subject: [PATCH] [analyzer] Fix false positive in strchr/strrchr with constant
args
When both the source string and the search target are compile-time
constants, determine the outcome precisely and only emit the feasible
branch (found or not-found). This eliminates false positives from
core.NullPointerArithm when e.g. strrchr is used in the common
FILE_BASENAME macro pattern.
Handles strchr, strrchr, memchr, strstr, and strpbrk.
Fixes: https://github.com/llvm/llvm-project/issues/209905
---
.../Checkers/CStringChecker.cpp | 176 +++++++++++--
clang/test/Analysis/string-search-modeling.c | 242 ++++++++++++++++++
2 files changed, 397 insertions(+), 21 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 745297dd1f057..9f8d7b524bed2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -30,6 +30,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLForwardCompat.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/Support/raw_ostream.h"
#include <functional>
#include <optional>
@@ -2662,34 +2663,167 @@ void CStringChecker::evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
if (!State)
return;
- // NULL (no-match) branch.
- if (CanReturnNull) {
+ // If both args are constant, only emit the feasible branch (llvm#209905).
+ // When a match is found, compute the exact offset for precise pointer
+ // arithmetic downstream.
+ enum MatchKind { Unknown, MustMatch, MustNotMatch };
+ struct ConstMatchResult {
+ MatchKind Kind = Unknown;
+ std::optional<size_t> Offset; // Concrete offset when Kind == MustMatch.
+ };
+ ConstMatchResult ConstResult = [&]() -> ConstMatchResult {
+ const MemRegion *SrcRegion = SrcVal.getAsRegion();
+ if (!SrcRegion)
+ return {};
+ const StringLiteral *SrcLit =
+ getStringLiteralFromRegion(SrcRegion->StripCasts());
+ if (!SrcLit)
+ return {};
+ StringRef Haystack = SrcLit->getString();
+
+ SVal Arg1Val = State->getSVal(Call.getArgExpr(1), SF);
+
+ // strchr: find first occurrence of character.
+ auto matchStrchr = [&]() -> ConstMatchResult {
+ const llvm::APSInt *CharInt = SVB.getKnownValue(State, Arg1Val);
+ if (!CharInt)
+ return {};
+ char Ch = static_cast<char>(CharInt->getExtValue());
+ // The C string ends at the first null (embedded or at Haystack.size()).
+ size_t NulPos = Haystack.find('\0');
+ size_t TermPos = (NulPos != StringRef::npos) ? NulPos : Haystack.size();
+ if (Ch == '\0')
+ return {MustMatch, TermPos};
+ // Search only up to the null terminator.
+ size_t Pos = Haystack.substr(0, TermPos).find(Ch);
+ if (Pos == StringRef::npos)
+ return {MustNotMatch, std::nullopt};
+ return {MustMatch, Pos};
+ };
+
+ // strrchr: find last occurrence of character.
+ auto matchStrrchr = [&]() -> ConstMatchResult {
+ const llvm::APSInt *CharInt = SVB.getKnownValue(State, Arg1Val);
+ if (!CharInt)
+ return {};
+ char Ch = static_cast<char>(CharInt->getExtValue());
+ size_t NulPos = Haystack.find('\0');
+ size_t TermPos = (NulPos != StringRef::npos) ? NulPos : Haystack.size();
+ if (Ch == '\0')
+ return {MustMatch, TermPos};
+ size_t Pos = Haystack.substr(0, TermPos).rfind(Ch);
+ if (Pos == StringRef::npos)
+ return {MustNotMatch, std::nullopt};
+ return {MustMatch, Pos};
+ };
+
+ // memchr: find character within the first N bytes.
+ auto matchMemchr = [&]() -> ConstMatchResult {
+ const llvm::APSInt *CharInt = SVB.getKnownValue(State, Arg1Val);
+ if (!CharInt)
+ return {};
+ if (Call.getNumArgs() < 3)
+ return {};
+ const llvm::APSInt *Len =
+ SVB.getKnownValue(State, State->getSVal(Call.getArgExpr(2), SF));
+ if (!Len)
+ return {};
+ char Ch = static_cast<char>(CharInt->getExtValue());
+ uint64_t N = Len->getZExtValue();
+ // Include the implicit null terminator in the searchable region.
+ SmallString<64> Buf(Haystack);
+ Buf.push_back('\0');
+ StringRef SearchRegion = StringRef(Buf.data(), Buf.size()).substr(0, N);
+ size_t Pos = SearchRegion.find(Ch);
+ if (Pos == StringRef::npos)
+ return {MustNotMatch, std::nullopt};
+ return {MustMatch, Pos};
+ };
+
+ // strstr: find first occurrence of substring.
+ auto matchSubstring = [&]() -> ConstMatchResult {
+ const MemRegion *R = Arg1Val.getAsRegion();
+ if (!R)
+ return {};
+ const StringLiteral *Lit = getStringLiteralFromRegion(R->StripCasts());
+ if (!Lit)
+ return {};
+ StringRef Needle = Lit->getString();
+ if (Needle.empty())
+ return {MustMatch, size_t{0}};
+ size_t Pos = Haystack.find(Needle);
+ if (Pos == StringRef::npos)
+ return {MustNotMatch, std::nullopt};
+ return {MustMatch, Pos};
+ };
+
+ // strpbrk: find first character from accept set.
+ auto matchAnyChar = [&]() -> ConstMatchResult {
+ const MemRegion *R = Arg1Val.getAsRegion();
+ if (!R)
+ return {};
+ const StringLiteral *Lit = getStringLiteralFromRegion(R->StripCasts());
+ if (!Lit)
+ return {};
+ StringRef Accept = Lit->getString();
+ size_t Pos = Haystack.find_first_of(Accept);
+ if (Pos == StringRef::npos)
+ return {MustNotMatch, std::nullopt};
+ return {MustMatch, Pos};
+ };
+
+ return llvm::StringSwitch<std::function<ConstMatchResult()>>(FnName)
+ .Case("strchr()", matchStrchr)
+ .Case("strrchr()", matchStrrchr)
+ .Case("memchr()", matchMemchr)
+ .Case("strstr()", matchSubstring)
+ .Case("strpbrk()", matchAnyChar)
+ .Default([&] { return ConstMatchResult{}; })();
+ }();
+
+ // NULL (no-match) branch — skip when the match is guaranteed.
+ if (CanReturnNull && ConstResult.Kind != MustMatch) {
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;
- }
+ // Found branch — skip when the match is impossible.
+ if (ConstResult.Kind != MustNotMatch) {
+ 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);
+ // If we know the exact offset, use a concrete value.
+ if (ConstResult.Offset) {
+ NonLoc ConcreteOffset =
+ SVB.makeIntVal(*ConstResult.Offset, Ctx.getSizeType())
+ .castAs<NonLoc>();
+ SVal Result = SVB.evalBinOpLN(State, BO_Add, *SrcLoc, ConcreteOffset,
+ Src.Expression->getType());
+ State = State->BindExpr(CE, SF, Result);
+ C.addTransition(State);
+ } else {
+ // Unknown match: use a symbolic offset constrained to be in bounds.
+ auto RemainingExtentBytes = getDynamicExtentWithOffset(State, *SrcLoc)
+ .castAs<DefinedOrUnknownSVal>();
+ NonLoc SymOffset =
+ SVB.conjureSymbolVal(Call, Ctx.getSizeType(), C.blockCount())
+ .castAs<NonLoc>();
+ State = State->assumeInBound(SymOffset, RemainingExtentBytes, true);
+ if (!State)
+ return;
- SVal Result = SVB.evalBinOpLN(State, BO_Add, *SrcLoc, SymOffset,
- Src.Expression->getType());
- State = State->BindExpr(CE, SF, Result);
- C.addTransition(State);
+ 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.
diff --git a/clang/test/Analysis/string-search-modeling.c b/clang/test/Analysis/string-search-modeling.c
index a50ec439731a3..17fbf06bfcc02 100644
--- a/clang/test/Analysis/string-search-modeling.c
+++ b/clang/test/Analysis/string-search-modeling.c
@@ -176,3 +176,245 @@ void no_invalidation_of_globals(const char *p) {
clang_analyzer_eval(local_unmodified == 10); // expected-warning {{TRUE}}
clang_analyzer_eval(global_unmodified == 20); // expected-warning {{TRUE}}
}
+
+//===----------------------------------------------------------------------===//
+// When both arguments are compile-time constants, only the correct branch is
+// taken: found when the target exists, null when it does not.
+// See: https://github.com/llvm/llvm-project/issues/209905
+//===----------------------------------------------------------------------===//
+
+// --- strchr / strrchr: target character IS in the literal ---
+const char *test_strrchr_const_no_fp(void) {
+ // This is the original reproducer from #209905.
+ return strrchr("/foo/bar.c", '/') ? strrchr("/foo/bar.c", '/') + 1 : "/foo/bar.c"; // no-warning
+}
+
+void test_strchr_const_found(void) {
+ clang_analyzer_eval(strchr("/foo/bar.c", '/') == 0); // expected-warning {{FALSE}}
+}
+
+void test_strrchr_const_found(void) {
+ clang_analyzer_eval(strrchr("/foo/bar.c", '/') == 0); // expected-warning {{FALSE}}
+}
+
+// --- strchr / strrchr: target character is NOT in the literal ---
+void test_strchr_const_not_found(void) {
+ clang_analyzer_eval(strchr("hello", 'z') == 0); // expected-warning {{TRUE}}
+}
+
+void test_strrchr_const_not_found(void) {
+ clang_analyzer_eval(strrchr("hello", 'z') == 0); // expected-warning {{TRUE}}
+}
+
+// --- memchr: character within bounds ---
+void test_memchr_const_found(void) {
+ clang_analyzer_eval(memchr("abcdef", 'c', 6) == 0); // expected-warning {{FALSE}}
+}
+
+// --- memchr: character beyond the specified length ---
+void test_memchr_const_not_in_range(void) {
+ clang_analyzer_eval(memchr("abcdef", 'f', 3) == 0); // expected-warning {{TRUE}}
+}
+
+// --- strstr: needle IS a substring ---
+void test_strstr_const_found(void) {
+ clang_analyzer_eval(strstr("hello world", "world") == 0); // expected-warning {{FALSE}}
+}
+
+// --- strstr: needle is NOT a substring ---
+void test_strstr_const_not_found(void) {
+ clang_analyzer_eval(strstr("hello world", "xyz") == 0); // expected-warning {{TRUE}}
+}
+
+// --- strpbrk: accept set has a match ---
+void test_strpbrk_const_found(void) {
+ clang_analyzer_eval(strpbrk("hello", "aeiou") == 0); // expected-warning {{FALSE}}
+}
+
+// --- strpbrk: no character from accept set in source ---
+void test_strpbrk_const_not_found(void) {
+ clang_analyzer_eval(strpbrk("hello", "xyz") == 0); // expected-warning {{TRUE}}
+}
+
+// --- Non-constant source: both branches must still exist ---
+void test_strchr_non_const_source(const char *p) {
+ clang_analyzer_eval(strchr(p, '/') == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+// --- Various constant source forms: const, static const, #define, __FILE__ ---
+static const char static_const_path[] = "/usr/local/bin/tool";
+
+void test_strchr_static_const(void) {
+ clang_analyzer_eval(strchr(static_const_path, '/') == 0); // expected-warning {{FALSE}}
+}
+
+const char global_const_path[] = "/etc/config";
+
+void test_strrchr_global_const(void) {
+ clang_analyzer_eval(strrchr(global_const_path, '/') == 0); // expected-warning {{FALSE}}
+}
+
+#define FIXED_PATH "/home/user/project/file.c"
+
+void test_strchr_define(void) {
+ clang_analyzer_eval(strchr(FIXED_PATH, '/') == 0); // expected-warning {{FALSE}}
+}
+
+#define MY_FILE_BASENAME (strrchr(__FILE__, '/') ? strrchr(__FILE__, '/') + 1 : __FILE__)
+
+void test_file_basename_macro(void) {
+ const char *base = MY_FILE_BASENAME; // no-warning
+ (void)base;
+}
+
+#define PREFIX "module"
+#define SUFFIX "_handler"
+// Adjacent string literal concatenation (the realistic preprocessor pattern):
+#define MODULE_PATH "/opt/" PREFIX "/" SUFFIX ".so"
+
+void test_strchr_concatenated_define(void) {
+ clang_analyzer_eval(strchr(MODULE_PATH, '/') == 0); // expected-warning {{FALSE}}
+}
+
+// --- Character argument via #define ---
+#define SEPARATOR '/'
+
+void test_strchr_define_char(void) {
+ clang_analyzer_eval(strchr("/foo/bar", SEPARATOR) == 0); // expected-warning {{FALSE}}
+}
+
+#define SEARCH_CHAR 'x'
+
+void test_strchr_define_char_not_found(void) {
+ clang_analyzer_eval(strchr("/foo/bar", SEARCH_CHAR) == 0); // expected-warning {{TRUE}}
+}
+
+// --- Edge cases: null character '\0' ---
+void test_strchr_null_char_always_found(void) {
+ // strchr(s, '\0') always finds the terminator.
+ clang_analyzer_eval(strchr("hello", '\0') == 0); // expected-warning {{FALSE}}
+}
+
+void test_strrchr_null_char_always_found(void) {
+ clang_analyzer_eval(strrchr("hello", '\0') == 0); // expected-warning {{FALSE}}
+}
+
+void test_memchr_null_char_within_bounds(void) {
+ // "abc" has terminator at index 3; searching 4 bytes includes it.
+ clang_analyzer_eval(memchr("abc", '\0', 4) == 0); // expected-warning {{FALSE}}
+}
+
+void test_memchr_null_char_out_of_bounds(void) {
+ // "abc" has terminator at index 3; searching only 3 bytes misses it.
+ clang_analyzer_eval(memchr("abc", '\0', 3) == 0); // expected-warning {{TRUE}}
+}
+
+// --- Edge cases: empty strings ---
+void test_strchr_empty_haystack(void) {
+ // Empty string only contains '\0'; '/' is not there.
+ clang_analyzer_eval(strchr("", '/') == 0); // expected-warning {{TRUE}}
+}
+
+void test_strchr_empty_haystack_null_char(void) {
+ // strchr("", '\0') finds the terminator.
+ clang_analyzer_eval(strchr("", '\0') == 0); // expected-warning {{FALSE}}
+}
+
+void test_strstr_empty_needle(void) {
+ // strstr(s, "") always returns s.
+ clang_analyzer_eval(strstr("hello", "") == 0); // expected-warning {{FALSE}}
+}
+
+void test_strpbrk_empty_accept(void) {
+ // strpbrk(s, "") never matches.
+ clang_analyzer_eval(strpbrk("hello", "") == 0); // expected-warning {{TRUE}}
+}
+
+//===----------------------------------------------------------------------===//
+// Verify exact pointer offsets when both arguments are compile-time constants.
+// The enhanced modeling returns Src + concrete_offset rather than a symbolic
+// offset, enabling precise pointer arithmetic downstream.
+//===----------------------------------------------------------------------===//
+
+// --- strchr: returns pointer to first occurrence ---
+void test_strchr_exact_offset(void) {
+ const char *s = "/foo/bar.c";
+ // '/' first appears at index 0.
+ clang_analyzer_eval(strchr(s, '/') == s); // expected-warning {{TRUE}}
+}
+
+// --- strrchr: returns pointer to last occurrence ---
+void test_strrchr_exact_offset(void) {
+ const char *s = "/foo/bar.c";
+ // '/' last appears at index 4.
+ clang_analyzer_eval(strrchr(s, '/') == s + 4); // expected-warning {{TRUE}}
+}
+
+// --- strrchr + 1: the FILE_BASENAME pattern ---
+void test_strrchr_plus_one(void) {
+ const char *s = "/foo/bar.c";
+ const char *base = strrchr(s, '/') + 1;
+ // Should point to 'b' at index 5.
+ clang_analyzer_eval(base == s + 5); // expected-warning {{TRUE}}
+}
+
+// --- strchr with null terminator: points to end of string ---
+void test_strchr_null_terminator_offset(void) {
+ const char *s = "hello";
+ // strchr(s, '\0') returns pointer to the null terminator at index 5.
+ clang_analyzer_eval(strchr(s, '\0') == s + 5); // expected-warning {{TRUE}}
+}
+
+// --- strstr: returns pointer to first substring match ---
+void test_strstr_exact_offset(void) {
+ const char *s = "hello world";
+ // "world" starts at index 6.
+ clang_analyzer_eval(strstr(s, "world") == s + 6); // expected-warning {{TRUE}}
+}
+
+// --- strstr with empty needle: returns the source pointer ---
+void test_strstr_empty_needle_offset(void) {
+ const char *s = "hello";
+ clang_analyzer_eval(strstr(s, "") == s); // expected-warning {{TRUE}}
+}
+
+// --- strpbrk: returns pointer to first matching character ---
+void test_strpbrk_exact_offset(void) {
+ const char *s = "hello";
+ // First vowel 'e' is at index 1.
+ clang_analyzer_eval(strpbrk(s, "aeiou") == s + 1); // expected-warning {{TRUE}}
+}
+
+// --- memchr: returns pointer to character within bounds ---
+void test_memchr_exact_offset(void) {
+ const char *s = "abcdef";
+ // 'c' is at index 2.
+ clang_analyzer_eval(memchr(s, 'c', 6) == s + 2); // expected-warning {{TRUE}}
+}
+
+// --- memchr with embedded null characters ---
+void test_memchr_embedded_null(void) {
+ // String literal "ab\0cd" has a null at index 2, then 'c' at 3, 'd' at 4,
+ // and the implicit terminator at index 5.
+ const char *s = "ab\0cd";
+ // memchr searching 5 bytes finds the first '\0' at index 2.
+ clang_analyzer_eval(memchr(s, '\0', 5) == s + 2); // expected-warning {{TRUE}}
+}
+
+void test_memchr_second_segment_after_null(void) {
+ const char *s = "ab\0cd";
+ // 'c' is at index 3; searching 5 bytes should find it.
+ clang_analyzer_eval(memchr(s, 'c', 5) == s + 3); // expected-warning {{TRUE}}
+}
+
+void test_memchr_char_before_null_boundary(void) {
+ const char *s = "ab\0cd";
+ // 'b' is at index 1; searching only 2 bytes still finds it.
+ clang_analyzer_eval(memchr(s, 'b', 2) == s + 1); // expected-warning {{TRUE}}
+}
+
+void test_memchr_char_hidden_by_short_len(void) {
+ const char *s = "ab\0cd";
+ // 'c' is at index 3 but searching only 3 bytes (indices 0-2) misses it.
+ clang_analyzer_eval(memchr(s, 'c', 3) == 0); // expected-warning {{TRUE}}
+}
More information about the cfe-commits
mailing list