[PATCH] D66049: [analyzer] PR41729: Fix some false positives and improve strlcat and strlcpy modeling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Oh, these false positives! Thanks!!



================
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1627
+                                    svalBuilder.makeIntVal(1, sizeTy), sizeTy);
+          Optional<NonLoc> freeSpaceNL = freeSpace.getAs<NonLoc>();
+
----------------
Please handle the situation when `freeSpaceNL` is `None` before dereferencing. I suspect that this might actually happen when you overwhelm the symbol complexity by subtracting 1. I'm not really super sure this can happen, given that symbols we're working with are very specific as of now, but this is something that should be made possible if we ever go far enough in our attempts to improve this checker.


================
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1637
+          if (TrueState && !FalseState) {
+            // srcStrLength <=  size - dstStrLength -1
+            amountCopied = strLength;
----------------
Strange whitespace here and in the next comment (:


================
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1647
+          if (TrueState && FalseState) {
+            amountCopied = UnknownVal();
+          }
----------------
Great job not introducing the state split here!


================
Comment at: test/Analysis/cstring-syntax.c:57
+
+  //test for Bug 41729
+  char a[256], b[256];
----------------
I think this test doesn't really demonstrate that bug 41729 is fixed, as the offending checker isn't enabled on this test.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66049/new/

https://reviews.llvm.org/D66049