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

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers created this revision.
vabridgers added reviewers: balazske, NoQ, martong, baloghadamsoftware, Szelethus, gamesh411.
Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun.
Herald added a project: clang.
vabridgers added a comment.

Not sure this is "correct", but it passes LITs with the new case, and it will at least get the discussion started :)


See https://bugs.llvm.org/show_bug.cgi?id=46128. The checker does not
yet comprehend constraints involving multiple symbols, so it's possible
to calculate a VLA size that's negative or 0. A LIT is added to catch
regressions, and this change simply bails if a VLA size of 0 or less is
calculated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80903

Files:
  clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
  clang/test/Analysis/vla.c


Index: clang/test/Analysis/vla.c
===================================================================
--- clang/test/Analysis/vla.c
+++ clang/test/Analysis/vla.c
@@ -137,3 +137,17 @@
   clang_analyzer_eval(clang_analyzer_getExtent(&vla3m) == 2 * x * 4 * sizeof(int));
   // expected-warning at -1{{TRUE}}
 }
+
+// https://bugs.llvm.org/show_bug.cgi?id=46128
+// analyzer doesn't handle more than simple symmmmbolic expressions.
+// Just don't crash.
+extern void foo(void);
+int a;
+void b() {
+  int c = a + 1;
+  for (;;) {
+    int d[c];
+    for (; 0 < c;)
+      foo();
+  }
+} // no-crash
Index: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -126,6 +126,11 @@
       // Size overflow check does not work with symbolic expressions because a
       // overflow situation can not be detected easily.
       uint64_t IndexL = IndexLVal->getZExtValue();
+      // constraints are not solved for expressions with multiple
+      // symbols, so just bail on invalid indices at this point.
+      if (IndexL <= 0)
+        return nullptr;
+
       assert(IndexL > 0 && "Index length should have been checked for zero.");
       if (KnownSize <= SizeMax / IndexL) {
         KnownSize *= IndexL;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80903.267532.patch
Type: text/x-patch
Size: 1381 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200601/0ac19c71/attachment.bin>