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 <benicsbal...@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@-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@+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@-1 {{(reg_${{[0-9]+}}<ptr_size p>) & 1U}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits