[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

It is no problem to return instead of the assert. I could fix the problem by using

  SVal LessThanZeroVal = SVB.evalBinOp(State, BO_LE, SizeD, Zero, SizeTy);

in `checkVLAIndexSize` (`BO_LT` is used before). Still the proposed return makes the code more safe.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:130
+      // constraints are not solved for expressions with multiple
+      // symbols, so just bail on invalid indices at this point.
+      if (IndexL <= 0)
----------------
I do not know if this is accurate reason for the problem, a more general text is better? And should start with uppercase.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:131
+      // symbols, so just bail on invalid indices at this point.
+      if (IndexL <= 0)
+        return nullptr;
----------------
Can be `==`.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:134
+
       assert(IndexL > 0 && "Index length should have been checked for zero.");
       if (KnownSize <= SizeMax / IndexL) {
----------------
The assert does not look good if there is already the check before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80903