https://github.com/guillem-bartrina-sonarsource created https://github.com/llvm/llvm-project/pull/161644
Upon running into `str(n)cmp` with unknown symbolic parameters, `CStringChecker` forks the analysis assuming that the two arguments are EQUAL in one branch. There is no explicit motivation for this behavior in the original commit message https://github.com/llvm/llvm-project/commit/c026370858e31e99a8190e4c945676fb2f3e941f, which leads me to believe it was not intentional. This behavior is quite counterintuitive, as well as noisy, and differs from that of the similar function `memcmp` ([here](https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp#L1590-L1601)). I suggest doing the same as in the latter case: not forking the analysis and proceeding to one branch or the other, whether we know for certain that the two arguments are EQUAL or not. _Note that the behavior I am modifying was not tested at all in the tests, as none broke due to the change._ >From 9a9e8fd3b9781de519c5df5c0df4e548e4d8966f Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Thu, 2 Oct 2025 11:32:50 +0200 Subject: [PATCH] [analyzer] CStringChecker: do not branch assuming the arguments of `str(n)cmp` are equal --- .../StaticAnalyzer/Checkers/CStringChecker.cpp | 7 ++----- clang/test/Analysis/string.c | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 0ae784c000f60..c15464e9a713f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -2418,15 +2418,12 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallEvent &Call, // If the two arguments might be the same buffer, we know the result is 0, // and we only need to check one size. - if (StSameBuf) { + if (StSameBuf && !StNotSameBuf) { StSameBuf = StSameBuf->BindExpr(Call.getOriginExpr(), LCtx, svalBuilder.makeZeroVal(Call.getResultType())); C.addTransition(StSameBuf); - - // If the two arguments are GUARANTEED to be the same, we're done! - if (!StNotSameBuf) - return; + return; } assert(StNotSameBuf); diff --git a/clang/test/Analysis/string.c b/clang/test/Analysis/string.c index cdd36275568e3..45b81297a026e 100644 --- a/clang/test/Analysis/string.c +++ b/clang/test/Analysis/string.c @@ -955,6 +955,14 @@ void strcmp_fn_l(char *x) { strcmp((char*)&strcmp_null_argument, x); // expected-warning{{Argument to string comparison function is the address of the function 'strcmp_null_argument', which is not a null-terminated string}} } +void strcmp_assumptions(char *a, char *b) { + if (strcmp(a, b) == 0) { + // ... + } + + clang_analyzer_eval(a == b); // expected-warning{{FALSE}} +} + //===----------------------------------------------------------------------=== // strncmp() //===----------------------------------------------------------------------=== @@ -1070,6 +1078,14 @@ int strncmp_null_argument(char *a, size_t n) { return strncmp(a, b, n); // expected-warning{{Null pointer passed as 2nd argument to string comparison function}} } +void strncmp_assumptions(char *a, char *b, size_t n) { + if (strncmp(a, b, n) == 0) { + // ... + } + + clang_analyzer_eval(a == b); // expected-warning{{FALSE}} +} + //===----------------------------------------------------------------------=== // strcasecmp() //===----------------------------------------------------------------------=== _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
