NoQ added a comment.
Thank you! Looks good overall.
================
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1013-1014
+
+ // Get the offset and the base region to figure out whether the offset of
+ // buffer is 0.
+ RegionOffset Offset = MR->getAsOffset();
----------------
Please say something here (or above) about why do we want our offset to be 0:
> We're about to model memset by producing a "default binding" in the Store.
> Our current implementation - RegionStore - doesn't support default bindings
> that don't cover the whole base region.
================
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1037
+
+ if (StateWholeReg && !StateNotWholeReg && CharVal.isZeroConstant()) {
+ // If the 'memset()' acts on the whole region of destination buffer and
----------------
I think we should use `StateNonNullChar` (that's computed below) instead of
`CharVal.isZeroConstant()` because the Environment doesn't provide a guarantee
that all symbols within it are collapsed to constants where applicable.
================
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1055
+ std::tie(StateNullChar, StateNonNullChar) =
+ assumeZero(C, State, CharVal, Ctx.UnsignedCharTy);
+
----------------
I think this should use `IntTy` here. Because that's the type of the `memset`'s
argument, and that's what `assumeZero()` expects.
================
Comment at: test/Analysis/string.c:1412
+ clang_analyzer_eval(strlen(str) >= 10); // expected-warning{{TRUE}}
+ // FIXME: This shoule be TRUE.
+ clang_analyzer_eval(str[1] == '0'); // expected-warning{{UNKNOWN}}
----------------
Typo: `should` :)
Repository:
rC Clang
https://reviews.llvm.org/D44934
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
