Skip to content

Commit

Permalink
[analyzer] Use dynamic type when invalidating by a member function call
Browse files Browse the repository at this point in the history
When instantiating "callable<T>", the "class CallableType" nested type
will only have a declaration in the copy for the instantiation - because
it's not refered to directly by any other code that would need a
complete definition.

However, in the past, when conservative eval calling member function,
we took the static type of the "this" expr, and looked up the
CXXRecordDecl it refered to to see if it has any mutable members (to
decide if it needs to refine invalidation or not).
Unfortunately, that query needs a definition, and it asserts otherwise,
thus we crashed.

To fix this, we should consult the dynamic type of the object, because
that will have the definition.
I anyways added a check for "hasDefinition" just to be on the safe side.

Fixes llvm#77378
  • Loading branch information
steakhal committed Oct 4, 2024
1 parent df6822f commit d3e8f1f
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,10 @@ class CXXInstanceCall : public AnyFunctionCall {
ValueList &Values,
RegionAndSymbolInvalidationTraits *ETraits) const override;

/// Returns the decl refered to by the "dynamic type" of the current object.
/// This may be null.
const CXXRecordDecl *getDeclForDynamicType() const;

public:
/// Returns the expression representing the implicit 'this' object.
virtual const Expr *getCXXThisExpr() const { return nullptr; }
Expand Down
55 changes: 30 additions & 25 deletions clang/lib/StaticAnalyzer/Core/CallEvent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -711,18 +711,17 @@ void CXXInstanceCall::getExtraInvalidatedValues(
if (const auto *D = cast_or_null<CXXMethodDecl>(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();
if (!ParentRecord || !ParentRecord->hasDefinition())
return;

if (ParentRecord->hasMutableFields())
return;

// Preserve CXXThis.
const MemRegion *ThisRegion = ThisVal.getAsRegion();
if (!ThisRegion)
Expand All @@ -748,6 +747,22 @@ SVal CXXInstanceCall::getCXXThisVal() const {
return ThisVal;
}

const CXXRecordDecl *CXXInstanceCall::getDeclForDynamicType() const {
const MemRegion *R = getCXXThisVal().getAsRegion();
if (!R)
return nullptr;

DynamicTypeInfo DynType = getDynamicTypeInfo(getState(), R);
if (!DynType.isValid())
return nullptr;

QualType Ty = DynType.getType()->getPointeeType();
if (Ty.isNull())
return nullptr;

return Ty->getAsCXXRecordDecl();
}

RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const {
// Do we have a decl at all?
const Decl *D = getDecl();
Expand All @@ -759,21 +774,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();
const CXXRecordDecl *RD = getDeclForDynamicType();
if (!RD || !RD->hasDefinition())
return {};

Expand All @@ -797,6 +798,10 @@ RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const {
return {};
}

const MemRegion *R = getCXXThisVal().getAsRegion();
DynamicTypeInfo DynType = getDynamicTypeInfo(getState(), R);
assert(DynType.isValid() && "ensured by getDeclForDynamicType()");

// Does the decl that we found have an implementation?
const FunctionDecl *Definition;
if (!Result->hasBody(Definition)) {
Expand Down
46 changes: 46 additions & 0 deletions clang/test/Analysis/const-method-call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,3 +271,49 @@ void checkThatConstMethodCallDoesInvalidateObjectForCircularReferences() {
// FIXME: Should be UNKNOWN.
clang_analyzer_eval(t.x); // expected-warning{{TRUE}}
}

namespace gh77378 {
template <typename Signature> class callable;

template <typename R> class callable<R()> {
struct CallableType {
bool operator()();
};
using MethodType = R (CallableType::*)();
CallableType *object_{nullptr};
MethodType method_;

public:
callable() = default;

template <typename T>
constexpr callable(const T &obj)
: object_{reinterpret_cast<CallableType *>(&const_cast<T &>(obj))},
method_{reinterpret_cast<MethodType>(
static_cast<bool (T::*)() const>(&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<bool()>{}.call().const_method();
// expected-warning@-1 {{Address of stack memory associated with temporary object of type 'callable<bool ()>' is still referred to by the static variable 'L' upon returning to the caller. This will be a dangling reference}}
}
} // namespace gh77378

0 comments on commit d3e8f1f

Please sign in to comment.