[PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

[Devin Coughlin via cfe-commits]

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