Skip to content

Commit d3e8f1f

Browse files
committed
[analyzer] Use dynamic type when invalidating by a member function call
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 #77378
1 parent df6822f commit d3e8f1f

File tree

3 files changed

+80
-25
lines changed

3 files changed

+80
-25
lines changed

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,10 @@ class CXXInstanceCall : public AnyFunctionCall {
690690
ValueList &Values,
691691
RegionAndSymbolInvalidationTraits *ETraits) const override;
692692

693+
/// Returns the decl refered to by the "dynamic type" of the current object.
694+
/// This may be null.
695+
const CXXRecordDecl *getDeclForDynamicType() const;
696+
693697
public:
694698
/// Returns the expression representing the implicit 'this' object.
695699
virtual const Expr *getCXXThisExpr() const { return nullptr; }

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -711,18 +711,17 @@ void CXXInstanceCall::getExtraInvalidatedValues(
711711
if (const auto *D = cast_or_null<CXXMethodDecl>(getDecl())) {
712712
if (!D->isConst())
713713
return;
714-
// Get the record decl for the class of 'This'. D->getParent() may return a
715-
// base class decl, rather than the class of the instance which needs to be
716-
// checked for mutable fields.
717-
// TODO: We might as well look at the dynamic type of the object.
718-
const Expr *Ex = getCXXThisExpr()->IgnoreParenBaseCasts();
719-
QualType T = Ex->getType();
720-
if (T->isPointerType()) // Arrow or implicit-this syntax?
721-
T = T->getPointeeType();
722-
const CXXRecordDecl *ParentRecord = T->getAsCXXRecordDecl();
723-
assert(ParentRecord);
714+
715+
// Get the record decl for the class of 'This'. D->getParent() may return
716+
// a base class decl, rather than the class of the instance which needs to
717+
// be checked for mutable fields.
718+
const CXXRecordDecl *ParentRecord = getDeclForDynamicType();
719+
if (!ParentRecord || !ParentRecord->hasDefinition())
720+
return;
721+
724722
if (ParentRecord->hasMutableFields())
725723
return;
724+
726725
// Preserve CXXThis.
727726
const MemRegion *ThisRegion = ThisVal.getAsRegion();
728727
if (!ThisRegion)
@@ -748,6 +747,22 @@ SVal CXXInstanceCall::getCXXThisVal() const {
748747
return ThisVal;
749748
}
750749

750+
const CXXRecordDecl *CXXInstanceCall::getDeclForDynamicType() const {
751+
const MemRegion *R = getCXXThisVal().getAsRegion();
752+
if (!R)
753+
return nullptr;
754+
755+
DynamicTypeInfo DynType = getDynamicTypeInfo(getState(), R);
756+
if (!DynType.isValid())
757+
return nullptr;
758+
759+
QualType Ty = DynType.getType()->getPointeeType();
760+
if (Ty.isNull())
761+
return nullptr;
762+
763+
return Ty->getAsCXXRecordDecl();
764+
}
765+
751766
RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const {
752767
// Do we have a decl at all?
753768
const Decl *D = getDecl();
@@ -759,21 +774,7 @@ RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const {
759774
if (!MD->isVirtual())
760775
return AnyFunctionCall::getRuntimeDefinition();
761776

762-
// Do we know the implicit 'this' object being called?
763-
const MemRegion *R = getCXXThisVal().getAsRegion();
764-
if (!R)
765-
return {};
766-
767-
// Do we know anything about the type of 'this'?
768-
DynamicTypeInfo DynType = getDynamicTypeInfo(getState(), R);
769-
if (!DynType.isValid())
770-
return {};
771-
772-
// Is the type a C++ class? (This is mostly a defensive check.)
773-
QualType RegionType = DynType.getType()->getPointeeType();
774-
assert(!RegionType.isNull() && "DynamicTypeInfo should always be a pointer.");
775-
776-
const CXXRecordDecl *RD = RegionType->getAsCXXRecordDecl();
777+
const CXXRecordDecl *RD = getDeclForDynamicType();
777778
if (!RD || !RD->hasDefinition())
778779
return {};
779780

@@ -797,6 +798,10 @@ RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const {
797798
return {};
798799
}
799800

801+
const MemRegion *R = getCXXThisVal().getAsRegion();
802+
DynamicTypeInfo DynType = getDynamicTypeInfo(getState(), R);
803+
assert(DynType.isValid() && "ensured by getDeclForDynamicType()");
804+
800805
// Does the decl that we found have an implementation?
801806
const FunctionDecl *Definition;
802807
if (!Result->hasBody(Definition)) {

clang/test/Analysis/const-method-call.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,3 +271,49 @@ void checkThatConstMethodCallDoesInvalidateObjectForCircularReferences() {
271271
// FIXME: Should be UNKNOWN.
272272
clang_analyzer_eval(t.x); // expected-warning{{TRUE}}
273273
}
274+
275+
namespace gh77378 {
276+
template <typename Signature> class callable;
277+
278+
template <typename R> class callable<R()> {
279+
struct CallableType {
280+
bool operator()();
281+
};
282+
using MethodType = R (CallableType::*)();
283+
CallableType *object_{nullptr};
284+
MethodType method_;
285+
286+
public:
287+
callable() = default;
288+
289+
template <typename T>
290+
constexpr callable(const T &obj)
291+
: object_{reinterpret_cast<CallableType *>(&const_cast<T &>(obj))},
292+
method_{reinterpret_cast<MethodType>(
293+
static_cast<bool (T::*)() const>(&T::operator()))} {}
294+
295+
constexpr bool const_method() const {
296+
return (object_->*(method_))();
297+
}
298+
299+
callable call() const & {
300+
static const auto L = [this]() {
301+
while (true) {
302+
// This should not crash when conservative eval calling the member function
303+
// when it unwinds the call stack due to exhausting the budget or reaching
304+
// the inlining limit.
305+
if (this->const_method()) {
306+
break;
307+
}
308+
}
309+
return true;
310+
};
311+
return L;
312+
}
313+
};
314+
315+
void entry() {
316+
callable<bool()>{}.call().const_method();
317+
// 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}}
318+
}
319+
} // namespace gh77378

0 commit comments

Comments
 (0)