dcoughlin added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:843
@@ +842,3 @@
+  if (!Length)
+    return true;
+
----------------
There doesn't seem to be a test that cares about this returning true (as 
compared to false).

================
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:855
@@ +854,3 @@
+  if (!BufLoc)
+    return true;
+
----------------
There doesn't appear to be a test that cares about this returning true (as 
compared to false).

================
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:863
@@ +862,3 @@
+  if (!R)
+    return true;
+
----------------
What's the rationale for treating the array access as in-bounds if the BufEnd 
is unknown? Or if LengthValue is unknown? Should these branches return false? 
Either way, can you add a comment explaining why this is the right thing to do 
and also update the doc comment of IsFirstBufInBound to reflect this behavior 
(e.g., "Returns true if destination buffer of copy function must/may be in 
bound")

================
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1109
@@ +1108,3 @@
+      }
+      assert(RO.getOffset() >= 0 && "Offset should not be negative");
+      uint64_t LowerOffset = RO.getOffset();
----------------
This assertion is triggering on our internal build bots. I'm working to get a 
reduced test case.

================
Comment at: test/Analysis/pr22954.c:739
@@ +738,3 @@
+// Test tainted values.
+struct yy {
+  char s1[4];
----------------
A question: how does this test tainted values?


http://reviews.llvm.org/D12571



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to