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

Wed Sep 30 09:51:17 PDT 2020

baloghadamsoftware requested changes to this revision.
baloghadamsoftware added a comment.
This revision now requires changes to proceed.

In D86743#2303922 <https://reviews.llvm.org/D86743#2303922>, @NoQ wrote:

> One slightly redeeming thing about this crash is that it's assertion-only. When built without assertions clang doesn't crash and this patch doesn't really change its behavior (adding transition to a null state is equivalent to adding no transitions at all). This means that the assertion did its job and notified us of the serious issue but simply removing the assertion doesn't bring *that* much benefit and we can probably afford to wait for a more solid fix.

Yes, that is right. `CheckerContext::addTransition()` accepts `nullptr`, thus with assertions disabled it does exactly what this patch does. Thus I think that this patch could be abandoned now, and if the bug is not exactly the same as 28450 <https://bugs.llvm.org/show_bug.cgi?id=28450> then a new bug report should be filed in //BugZilla//. And maybe the review on D69726 <https://reviews.llvm.org/D69726> could be continued if that is part of the proper solution you suggested.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743