Author: Balazs Benics Date: 2023-11-04T11:11:24+01:00 New Revision: 51d15d13dea4325d1f76353af847d9de0b532e87
URL: https://github.com/llvm/llvm-project/commit/51d15d13dea4325d1f76353af847d9de0b532e87 DIFF: https://github.com/llvm/llvm-project/commit/51d15d13dea4325d1f76353af847d9de0b532e87.diff LOG: [analyzer] Fix assertion failure in `CXXInstanceCall::getCXXThisVal` (#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. As this patch presents, I'm evaluating a cast to transform the `NonLoc`. However, given that `evalCast()` can't be cast from `NonLoc` to a pointer type thingy (`Loc`), we end up with `Unknown`. It is because `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 `SymExpr`s, as stated: > // Because pointer arithmetic is represented by ElementRegion layers, > // the base symbol here should not contain any arithmetic. 3) We can't use `ElementRegion`s 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 left a FIXME instead, while still returning `Unknown` on that path. IMO this is still better than having a crash. Fixes #69922 Added: Modified: clang/lib/StaticAnalyzer/Core/CallEvent.cpp clang/lib/StaticAnalyzer/Core/SValBuilder.cpp clang/test/Analysis/builtin_bitcast.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index ad5bb66c4fff3c8..76fb7481f65194b 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -715,10 +715,14 @@ void CXXInstanceCall::getExtraInvalidatedValues( SVal CXXInstanceCall::getCXXThisVal() const { const Expr *Base = getCXXThisExpr(); // FIXME: This doesn't handle an overloaded ->* operator. - if (!Base) - return UnknownVal(); + SVal ThisVal = Base ? getSVal(Base) : UnknownVal(); + + if (isa<NonLoc>(ThisVal)) { + SValBuilder &SVB = getState()->getStateManager().getSValBuilder(); + QualType OriginalTy = ThisVal.getType(SVB.getContext()); + return SVB.evalCast(ThisVal, Base->getType(), OriginalTy); + } - SVal ThisVal = getSVal(Base); assert(ThisVal.isUnknownOrUndef() || isa<Loc>(ThisVal)); return ThisVal; } diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp index d89d82626f72694..9375f39b2d71dd4 100644 --- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -980,6 +980,11 @@ class EvalCastVisitor : public SValVisitor<EvalCastVisitor, SVal> { return VB.makeNonLoc(SE, T, CastTy); } + // FIXME: We should be able to cast NonLoc -> Loc + // (when Loc::isLocType(CastTy) is true) + // But it's hard to do as SymbolicRegions can't refer to SymbolCasts holding + // generic SymExprs. Check the commit message for the details. + // Symbol to pointer and whatever else. return UnknownVal(); } diff --git a/clang/test/Analysis/builtin_bitcast.cpp b/clang/test/Analysis/builtin_bitcast.cpp index 396e7caa45f6acd..5a0d9e7189b8edd 100644 --- a/clang/test/Analysis/builtin_bitcast.cpp +++ b/clang/test/Analysis/builtin_bitcast.cpp @@ -2,6 +2,7 @@ // RUN: -analyzer-checker=core,debug.ExprInspection template <typename T> void clang_analyzer_dump(T); +using size_t = decltype(sizeof(int)); __attribute__((always_inline)) static inline constexpr unsigned int _castf32_u32(float __A) { return __builtin_bit_cast(unsigned int, __A); // no-warning @@ -30,3 +31,23 @@ 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; + } +}; +void gh_69922(size_t p) { + // expected-warning-re@+1 {{(reg_${{[0-9]+}}<size_t 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]+}}<size_t p>) & 1U}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits