Commit Graph

1 Commits

Author SHA1 Message Date
Balazs Benics a6816b957d [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
2021-12-06 18:38:58 +01:00