NoQ 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.
----------------
balazske wrote:
> 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);
> }
>
> ```
It might be that `simplifySVal` isn't applied consistently. Like, `assume()`
fails to apply it so it fails to see that the value is in fact concrete zero.
Let's remove the assertion for now and add a FIXME to investigate why can't we
add it back. Like, there's nothing preventing us from investigating it now but
it will take some time whereas crashes are already there and we should fix them
to unblock other people.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80903/new/
https://reviews.llvm.org/D80903
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
