NoQ added a comment.
Let's separate `CStringChecker` improvements into a separate patch and have a
separate set of tests for it.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2097-2098
+void CStringChecker::evalCharPtrCommon(CheckerContext &C,
+ const CallExpr *CE) const {
+ CurrentFunctionDescription = "char pointer returning function";
----------------
Ok, so this whole thing is trying to evaluate `strchr`-like functions, right?
I've no idea what it does but the problem is much more trivial that what you're
trying to do here.
Branch 1:
1. Conjure the offset.
2. Add it to the original pointer (using `evalBinOp`).
3. Bind the result.
Branch 2:
1. Bind null.
And you probably should drop branch 2 completely because usually there's no
indication that the character may in fact be missing in the string, and i don't
want more null dereference false alarms. So it's just branch 1.
So the whole function should be 3 lines of code (maybe with a couple line
breaks).
Well, maybe you should also handle the case where the original pointer is null
(not sure if it's an immediate UB or not).
This could be improved by actually taking into account the contents of the
string, but you don't seem to be trying to do this here.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2101-2103
+ SValBuilder &SVB = C.getSValBuilder();
+ ProgramStateRef State, StateNull;
+ std::tie(StateNull, State) = C.getState()->assume(SVB.makeNull());
----------------
So, like, `StateNull` is the state in which it is assumed that `0` is non-zero
and `State` is the state in which it is assumed that `0` is zero?
I mean, apart from the naming flip-flop - i can tell you in advance that `0` is
zero, it's not a matter of assumptions.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2127-2128
+ if (const auto *SL = dyn_cast<StringLiteral>(SrcExpr->IgnoreImpCasts())) {
+ const StringRegion *ResultMR =
+ C.getStoreManager().getRegionManager().getStringRegion(SL);
+ SVal ResultV = loc::MemRegionVal(ResultMR);
----------------
This is guaranteed to return the string region that you already have as the
value of that expression.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71155/new/
https://reviews.llvm.org/D71155
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
