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

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

balazske added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:114-115
     // Convert the array length to size_t.
     NonLoc IndexLength =
         SVB.evalCast(SizeD, SizeTy, SizeE->getType()).castAs<NonLoc>();
     // Multiply the array length by the element size.
----------------
vabridgers wrote:
> NoQ wrote:
> > Do i understand correctly that this cast is the only difference between the value that has been checked and the value on which the assertion is asserted?
> Yes, looks that way to me. Let's see if Balasz, Gabor, Adam or Kristof responds in the next day or two? Thanks Artem!
Yes the cast is the difference. Even if this problem is fixed (cast included in `checkVLAIndexSize`) the same problem happens.
The following code (in `checkVLAIndexSize`) prints `(reg_$0<int a>) + 1 0` before the crash. So there is some difference between `assume` and `getKnownValue`. It can be better to include check of `getKnownValue` in `checkVLAIndexSize` (at least for zero value).
```lang=c++
  // Convert the array length to size_t.
  NonLoc SizeNL =
      SVB.evalCast(SizeD, SizeTy, SizeE->getType()).castAs<NonLoc>();

  // Check if the size is zero.
  ProgramStateRef StateNotZero, StateZero;
  std::tie(StateNotZero, StateZero) = State->assume(SizeNL);

  if (StateZero && !StateNotZero) {
    reportBug(VLA_Zero, SizeE, StateZero, C);
    return nullptr;
  }

  // From this point on, assume that the size is not zero.
  State = StateNotZero;

  if (const llvm::APSInt *IndexLVal = SVB.getKnownValue(State, SizeNL)) {
    uint64_t L = IndexLVal->getZExtValue();
    llvm::errs() << SizeNL << " " << L << "\n";
    assert(L != 0);
  }

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80903