llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: None (guillem-bartrina-sonarsource) <details> <summary>Changes</summary> Upon running into `str(n)cmp` with unknown symbolic arguments, `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._ --- Full diff: https://github.com/llvm/llvm-project/pull/161644.diff 2 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (+2-5) - (modified) clang/test/Analysis/string.c (+16) ``````````diff 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() //===----------------------------------------------------------------------=== `````````` </details> https://github.com/llvm/llvm-project/pull/161644 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
