diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h index 549c864dc91ef..4552df2a2a31e 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -690,6 +690,11 @@ class CXXInstanceCall : public AnyFunctionCall { ValueList &Values, RegionAndSymbolInvalidationTraits *ETraits) const override; + /// Returns the decl refered to by the "dynamic type" of the current object + /// and if the class can be a sub-class or not. + /// If the Pointer is null, the flag has no meaning. + std::pair getDeclForDynamicType() const; + public: /// Returns the expression representing the implicit 'this' object. virtual const Expr *getCXXThisExpr() const { return nullptr; } diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index 1efac6ac1c57f..270cce1d3a650 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -711,18 +711,17 @@ void CXXInstanceCall::getExtraInvalidatedValues( if (const auto *D = cast_or_null(getDecl())) { if (!D->isConst()) return; - // Get the record decl for the class of 'This'. D->getParent() may return a - // base class decl, rather than the class of the instance which needs to be - // checked for mutable fields. - // TODO: We might as well look at the dynamic type of the object. - const Expr *Ex = getCXXThisExpr()->IgnoreParenBaseCasts(); - QualType T = Ex->getType(); - if (T->isPointerType()) // Arrow or implicit-this syntax? - T = T->getPointeeType(); - const CXXRecordDecl *ParentRecord = T->getAsCXXRecordDecl(); - assert(ParentRecord); + + // Get the record decl for the class of 'This'. D->getParent() may return + // a base class decl, rather than the class of the instance which needs to + // be checked for mutable fields. + const CXXRecordDecl *ParentRecord = getDeclForDynamicType().first; + if (!ParentRecord || !ParentRecord->hasDefinition()) + return; + if (ParentRecord->hasMutableFields()) return; + // Preserve CXXThis. const MemRegion *ThisRegion = ThisVal.getAsRegion(); if (!ThisRegion) @@ -748,6 +747,21 @@ SVal CXXInstanceCall::getCXXThisVal() const { return ThisVal; } +std::pair +CXXInstanceCall::getDeclForDynamicType() const { + const MemRegion *R = getCXXThisVal().getAsRegion(); + if (!R) + return {}; + + DynamicTypeInfo DynType = getDynamicTypeInfo(getState(), R); + if (!DynType.isValid()) + return {}; + + assert(!DynType.getType()->getPointeeType().isNull()); + return {DynType.getType()->getPointeeCXXRecordDecl(), + DynType.canBeASubClass()}; +} + RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const { // Do we have a decl at all? const Decl *D = getDecl(); @@ -759,21 +773,7 @@ RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const { if (!MD->isVirtual()) return AnyFunctionCall::getRuntimeDefinition(); - // Do we know the implicit 'this' object being called? - const MemRegion *R = getCXXThisVal().getAsRegion(); - if (!R) - return {}; - - // Do we know anything about the type of 'this'? - DynamicTypeInfo DynType = getDynamicTypeInfo(getState(), R); - if (!DynType.isValid()) - return {}; - - // Is the type a C++ class? (This is mostly a defensive check.) - QualType RegionType = DynType.getType()->getPointeeType(); - assert(!RegionType.isNull() && "DynamicTypeInfo should always be a pointer."); - - const CXXRecordDecl *RD = RegionType->getAsCXXRecordDecl(); + auto [RD, CanBeSubClass] = getDeclForDynamicType(); if (!RD || !RD->hasDefinition()) return {}; @@ -800,7 +800,7 @@ RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const { // Does the decl that we found have an implementation? const FunctionDecl *Definition; if (!Result->hasBody(Definition)) { - if (!DynType.canBeASubClass()) + if (!CanBeSubClass) return AnyFunctionCall::getRuntimeDefinition(); return {}; } @@ -808,8 +808,9 @@ RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const { // We found a definition. If we're not sure that this devirtualization is // actually what will happen at runtime, make sure to provide the region so // that ExprEngine can decide what to do with it. - if (DynType.canBeASubClass()) - return RuntimeDefinition(Definition, R->StripCasts()); + if (CanBeSubClass) + return RuntimeDefinition(Definition, + getCXXThisVal().getAsRegion()->StripCasts()); return RuntimeDefinition(Definition, /*DispatchRegion=*/nullptr); } diff --git a/clang/test/Analysis/const-method-call.cpp b/clang/test/Analysis/const-method-call.cpp index 8e1fd3b125f0e..7da7ca5554a23 100644 --- a/clang/test/Analysis/const-method-call.cpp +++ b/clang/test/Analysis/const-method-call.cpp @@ -271,3 +271,49 @@ void checkThatConstMethodCallDoesInvalidateObjectForCircularReferences() { // FIXME: Should be UNKNOWN. clang_analyzer_eval(t.x); // expected-warning{{TRUE}} } + +namespace gh77378 { +template class callable; + +template class callable { + struct CallableType { + bool operator()(); + }; + using MethodType = R (CallableType::*)(); + CallableType *object_{nullptr}; + MethodType method_; + +public: + callable() = default; + + template + constexpr callable(const T &obj) + : object_{reinterpret_cast(&const_cast(obj))}, + method_{reinterpret_cast( + static_cast(&T::operator()))} {} + + constexpr bool const_method() const { + return (object_->*(method_))(); + } + + callable call() const & { + static const auto L = [this]() { + while (true) { + // This should not crash when conservative eval calling the member function + // when it unwinds the call stack due to exhausting the budget or reaching + // the inlining limit. + if (this->const_method()) { + break; + } + } + return true; + }; + return L; + } +}; + +void entry() { + callable{}.call().const_method(); + // expected-warning@-1 {{Address of stack memory associated with temporary object of type 'callable' is still referred to by the static variable 'L' upon returning to the caller. This will be a dangling reference}} +} +} // namespace gh77378