Skip to content

[analyzer] Use dynamic type when invalidating by a member function call #111138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<const CXXRecordDecl *, bool> getDeclForDynamicType() const;

public:
/// Returns the expression representing the implicit 'this' object.
virtual const Expr *getCXXThisExpr() const { return nullptr; }
Expand Down
57 changes: 29 additions & 28 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().first;
if (!ParentRecord || !ParentRecord->hasDefinition())
return;

if (ParentRecord->hasMutableFields())
return;

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

std::pair<const CXXRecordDecl *, bool>
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();
Expand All @@ -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 {};

Expand All @@ -800,16 +800,17 @@ 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 {};
}

// 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);
}

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
Loading