[analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions
authorBalazs Benics <balazs.benics@sigmatechnology.se>
Mon, 6 Dec 2021 17:38:58 +0000 (18:38 +0100)
committerBalazs Benics <balazs.benics@sigmatechnology.se>
Mon, 6 Dec 2021 17:38:58 +0000 (18:38 +0100)
commita6816b957d28ab7855f2af1277c72a6d65b600b4
tree5b18a968d59bcfbd8b6f3cfed665af40e2001895
parent63a6348cad6caccf285c1661bc60d8ba5a40c972
[analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

Previously, the `SValBuilder` could not encounter expressions of the
following kind:

  NonLoc OP Loc
  Loc OP NonLoc

Where the `Op` is other than `BO_Add`.

As of now, due to the smarter simplification and the fixedpoint
iteration, it turns out we can.
It can happen if the `Loc` was perfectly constrained to a concrete
value (`nonloc::ConcreteInt`), thus the simplifier can do
constant-folding in these cases as well.

Unfortunately, this could cause assertion failures, since we assumed
that the operator must be `BO_Add`, causing a crash.

---

In the patch, I decided to preserve the original behavior (aka. swap the
operands (if the operator is commutative), but if the `RHS` was a
`loc::ConcreteInt` call `evalBinOpNN()`.

I think this interpretation of the arithmetic expression is closer to
reality.

I also tried naively introducing a separate handler for
`loc::ConcreteInt` RHS, before doing handling the more generic `Loc` RHS
case. However, it broke the `zoo1backwards()` test in the `nullptr.cpp`
file. This highlighted for me the importance to preserve the original
behavior for the `BO_Add` at least.

PS: Sorry for introducing yet another branch into this `evalBinOpXX`
madness. I've got a couple of ideas about refactoring these.
We'll see if I can get to it.

The test file demonstrates the issue and makes sure nothing similar
happens. The `no-crash` annotated lines show, where we crashed before
applying this patch.

Reviewed By: martong

Differential Revision: https://reviews.llvm.org/D115149
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
clang/test/Analysis/symbol-simplification-nonloc-loc.cpp [new file with mode: 0644]