[clang] [analyzer] Fix assertion failure in CXXInstanceCall::getCXXThisVal (PR #70837)

[Balazs Benics via cfe-commits]

https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/70837

Workaround the case when the `this` pointer is actually a `NonLoc`, by returning `Unknown` instead.
The solution isn't ideal, as `this` should be really a `Loc`, but due to how casts work, I feel this is our easiest and best option.

I've considered using `SVB.evalCast(ThisVal, Base->getType(), QualType())`, but it doesn't work as `EvalCastVisitor::VisitNonLocSymbolVal()` only evaluates casts that happen from NonLoc to NonLocs.

When I tried to actually implement that case, I figured:
1) Create a SymbolicRegion from that nonloc::SymbolVal; but SymbolRegion ctor expects a pointer type for the symbol.
2) Okay, just have a SymbolCast, getting us the pointer type; but SymbolRegion expects SymbolData symbols, not generic SymExprs, as stated:

> // Because pointer arithmetic is represented by ElementRegion layers,
> // the base symbol here should not contain any arithmetic.

3) We can't use ElementRegions to perform this cast because to have an ElementRegion, you already have to have a SubRegion that you want to cast, but the point is that we don't have that.

At this point, I gave up, and just returned `Unknown` xD IMO this is still better than having a crash.

Fixes #69922

>From a7f64815f4986fad597b9cb2d1acce2de9ac20bf Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Mon, 23 Oct 2023 18:10:29 +0200
Subject: [PATCH] [analyzer] Fix assertion failure in
 CXXInstanceCall::getCXXThisVal

Workaround the case when the `this` pointer is actually a `NonLoc`, by
returning `Unknown` instead.
The solution isn't ideal, as `this` should be really a `Loc`, but due to
how casts work, I feel this is our easiest and best option.

I've considered using `SVB.evalCast(ThisVal, Base->getType(), QualType())`,
but it doesn't work as `EvalCastVisitor::VisitNonLocSymbolVal()` only
evaluates casts that happen from NonLoc to NonLocs.

When I tried to actually implement that case, I figured:
1) Create a SymbolicRegion from that nonloc::SymbolVal; but SymbolRegion
   ctor expects a pointer type for the symbol.
2) Okay, just have a SymbolCast, getting us the pointer type; but
   SymbolRegion expects SymbolData symbols, not generic SymExprs, as stated:

> // Because pointer arithmetic is represented by ElementRegion layers,
> // the base symbol here should not contain any arithmetic.

3) We can't use ElementRegions to perform this cast because to have an
   ElementRegion, you already have to have a SubRegion that you want to
   cast, but the point is that we don't have that.

At this point, I gave up, and just returned `Unknown` xD
IMO this is still better than having a crash.

Fixes #69922
---
 clang/lib/StaticAnalyzer/Core/CallEvent.cpp |  5 ++---
 clang/test/Analysis/builtin_bitcast.cpp     | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index ad5bb66c4fff3c8..20bc64dc2631cec 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -715,10 +715,9 @@ void CXXInstanceCall::getExtraInvalidatedValues(
 SVal CXXInstanceCall::getCXXThisVal() const {
   const Expr *Base = getCXXThisExpr();
   // FIXME: This doesn't handle an overloaded ->* operator.
-  if (!Base)
+  SVal ThisVal = Base ? getSVal(Base) : UnknownVal();
+  if (isa<NonLoc>(ThisVal))
     return UnknownVal();
-
-  SVal ThisVal = getSVal(Base);
   assert(ThisVal.isUnknownOrUndef() || isa<Loc>(ThisVal));
   return ThisVal;
 }
diff --git a/clang/test/Analysis/builtin_bitcast.cpp b/clang/test/Analysis/builtin_bitcast.cpp
index 396e7caa45f6acd..13475694d287a22 100644
--- a/clang/test/Analysis/builtin_bitcast.cpp
+++ b/clang/test/Analysis/builtin_bitcast.cpp
@@ -30,3 +30,24 @@ void test(int i) {
   clang_analyzer_dump(g4);
   // expected-warning at -1 {{&i [as 64 bit integer]}}
 }
+
+struct A {
+  int n;
+  void set(int x) {
+    n = x;
+  }
+};
+using ptr_size = decltype(sizeof(void *));
+void gh_69922(ptr_size p) {
+  // expected-warning-re at +1 {{(reg_${{[0-9]+}}<ptr_size p>) & 1U}}
+  clang_analyzer_dump(__builtin_bit_cast(A*, p & 1));
+
+  __builtin_bit_cast(A*, p & 1)->set(2); // no-crash
+  // However, since the `this` pointer is expected to be a Loc, but we have
+  // NonLoc there, we simply give up and resolve it as `Unknown`.
+  // Then, inside the `set()` member function call we can't evaluate the
+  // store to the member variable `n`.
+
+  clang_analyzer_dump(__builtin_bit_cast(A*, p & 1)->n); // Ideally, this should print "2".
+  // expected-warning-re at -1 {{(reg_${{[0-9]+}}<ptr_size p>) & 1U}}
+}