[clang] [analyzer] `SValBuilder::evalBinOpLN`: try simplifying the RHS first (PR #161537)

[Balazs Benics via cfe-commits]

================
@@ -1111,6 +1111,12 @@ SVal SimpleSValBuilder::evalBinOpLN(ProgramStateRef state,
   assert(!BinaryOperator::isComparisonOp(op) &&
          "arguments to comparison ops must be of the same type");
 
+  // Constraints may have changed since the creation of a bound SVal. Check if
+  // the value can be simplified based on those new constraints.
+  SVal simplifiedRhs = simplifySVal(state, rhs);
+  if (auto simplifiedRhsAsNonLoc = simplifiedRhs.getAs<NonLoc>())
+    rhs = *simplifiedRhsAsNonLoc;
+
----------------
steakhal wrote:

The comment suggests some spooky action in the distance that "may have changed" the constraints.
I think without explaining how and where that could happen this sentence only brings confusion.
If that would be the case, I'd rather just have this code without a comment.

BTW the contract of the evalBinOpXX functions is that they are never called from user code. the outermost evalBinOp should simplify and then dispatch to the suffixed implementation variants and also potentially recurse. This structure should give us guarantees that every symbol simplified exactly once (except if we end up recursing of course).
This contract in conjunction with this hunk makes me wonder if we overlooked this somehow.
Of course, that the checkers directly use evalBinOpXX fns doesn't really help establishing this contract - and that is likely the reason you stumbled on this bug in the first place.

I tried o refactor the code to ensure this contract but the change had a considerable slowdown when I measured. So I dropped the idea.

```suggestion
  // Constraints may have changed since the creation of a bound SVal.
  // Check if the value can be simplified based on those new constraints.
  SVal SimplifiedRHS = simplifySVal(state, rhs);
  if (auto SimplifiedRHSAsNonLoc = SimplifiedRHS.getAs<NonLoc>())
    rhs = *SimplifiedRHSAsNonLoc;

```

https://github.com/llvm/llvm-project/pull/161537