[PATCH] D120992: [analyzer] ReverseNull: New checker to warn for pointer value conditions, if the pointer value is unconditionally non-null

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I guess there's the usual direction that I occasionally suggest: develop a way to verify that all possible paths were explored during symbolic execution (`CoreEngine::hasWorkRemaining()` on steroids), then do most of the work in `checkEndAnalysis`.

We also already have `alpha.core.TestAfterDivZero` that implements a somewhat similar check-after-use strategy.

> Do you think there is any point in turning this into an optin checker? Or the philosophy should be that this is just way too wild?

We made such trade-offs in the past, the most obvious one being our strategy to always split paths on every if-statement and never merge. In particular, our core null dereference checker suffers a lot from this, and it's a major point of criticism we regularly receive from our users.

What I'm trying to say here is, I honestly think re-doing it as CFG-based should be relatively easy. We couldn't make any progress at all without those state splits but in this case it's much less of a fundamental issue so I think it's not worth it to hard-commit to a flawed solution from the start.

In fact you already have one of the building blocks:

> What if we demand that the the CFGBlock of the dereference must dominate the CFGBlock of the condition point?

I mean, yes, this is a good strategy, you could implement that //and then remove symbolic execution//. Another building block could be "there aren't any writes into (or escapes of) that local variable between the use and the check". So it looks like this is entirely about statements of certain kind appearing in certain order in the CFG.

Another bonus from CFG-based checkers is that they can be turned into compiler warnings - which may affect a lot more users. I actually don't know why dead stores checker isn't a compiler warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120992