[all-commits] [llvm/llvm-project] a6816b: [analyzer][solver] Fix assertion on (NonLoc, Op, L...

[Balazs Benics via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: a6816b957d28ab7855f2af1277c72a6d65b600b4
      https://github.com/llvm/llvm-project/commit/a6816b957d28ab7855f2af1277c72a6d65b600b4
  Author: Balazs Benics <balazs.benics at sigmatechnology.se>
  Date:   2021-12-06 (Mon, 06 Dec 2021)

  Changed paths:
    M clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
    A clang/test/Analysis/symbol-simplification-nonloc-loc.cpp

  Log Message:
  -----------
  [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