MTC added inline comments.

================
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
----------------
NoQ wrote:
> 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.
Yea, thanks! 


================
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1055
+    std::tie(StateNullChar, StateNonNullChar) =
+        assumeZero(C, State, CharVal, Ctx.UnsignedCharTy);
+
----------------
NoQ wrote:
> I think this should use `IntTy` here. Because that's the type of the 
> `memset`'s argument, and that's what `assumeZero()` expects.
I confirmed again that `memset()` will convert the value to `unsigned char` 
first, see http://en.cppreference.com/w/c/string/byte/memset. 

In the next update, I will `evalCast(value, UnsignedCharTy, IntTy)` first, 
therefore, I will continue to use `UnsignedCharTy`.


================
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}}
----------------
NoQ wrote:
> Typo: `should` :)
thanks, will do!


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

Reply via email to