[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Yes, @NoQ is right. If the constraint manager cannot reason about a value, then then `ProgramState::assume()` will return the same state with the new assumption in the constraints. Whenever `ProgramState::assume()` returns `nullptr` it means that the assumption is impossible. Thus in this case `DynSize` and `*ArraySizeNL` cannot be equal. To me this really seems to be a similar issue to Bug 28450 <https://bugs.llvm.org/show_bug.cgi?id=28450>. The last comment for that bug is D69726 <https://reviews.llvm.org/D69726>, but the bug is not closed to it seems to me that D69726 <https://reviews.llvm.org/D69726> does not solve it, just takes a single step towards the solution.

I see that proper solution is somewhat more complicated and requires deep knowledge and takes probably much time. However, this one is a core checker, and not even //alpha//. Even //alpha// checkers should not crash at all (for me //alpha// means many false positives), but a crashing core checker, that is no even alpha completely undermines the users' trust towards the Analyzer. Bug 28450 <https://bugs.llvm.org/show_bug.cgi?id=28450> is open for almost one and half years. Thus something quick should be done even before someone implements the proper solution (from @NOQ's suggestions the second one is implemented by D69726 <https://reviews.llvm.org/D69726>, but not the first one). A //FIXME// must be added in either case, and the misleading comment deleted. Or if this solution is completely unacceptable for the community then `VLASizeChecker` should be degraded to //alpha//.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743