Skip to content

Commit f6971bf

Browse files
authored
[FlowSensitive] [StatusOr] [12/N] Add support for smart pointers (#170943)
1 parent e3905a4 commit f6971bf

File tree

2 files changed

+122
-0
lines changed

2 files changed

+122
-0
lines changed

clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
2626
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
2727
#include "clang/Analysis/FlowSensitive/RecordOps.h"
28+
#include "clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h"
2829
#include "clang/Analysis/FlowSensitive/StorageLocation.h"
2930
#include "clang/Analysis/FlowSensitive/Value.h"
3031
#include "clang/Basic/LLVM.h"
@@ -849,6 +850,16 @@ transferNonConstMemberOperatorCall(const CXXOperatorCallExpr *Expr,
849850
handleNonConstMemberCall(Expr, RecordLoc, Result, State);
850851
}
851852

853+
static RecordStorageLocation *
854+
getSmartPtrLikeStorageLocation(const Expr &E, const Environment &Env) {
855+
if (!E.isPRValue())
856+
return dyn_cast_or_null<RecordStorageLocation>(Env.getStorageLocation(E));
857+
if (auto *PointerVal = dyn_cast_or_null<PointerValue>(Env.getValue(E)))
858+
return dyn_cast_or_null<RecordStorageLocation>(
859+
&PointerVal->getPointeeLoc());
860+
return nullptr;
861+
}
862+
852863
CFGMatchSwitch<LatticeTransferState>
853864
buildTransferMatchSwitch(ASTContext &Ctx,
854865
CFGMatchSwitchBuilder<LatticeTransferState> Builder) {
@@ -906,6 +917,43 @@ buildTransferMatchSwitch(ASTContext &Ctx,
906917
transferLoggingGetReferenceableValueCall)
907918
.CaseOfCFGStmt<CallExpr>(isLoggingCheckEqImpl(),
908919
transferLoggingCheckEqImpl)
920+
// This needs to go before the const accessor call matcher, because these
921+
// look like them, but we model `operator`* and `get` to return the same
922+
// object. Also, we model them for non-const cases.
923+
.CaseOfCFGStmt<CXXOperatorCallExpr>(
924+
isPointerLikeOperatorStar(),
925+
[](const CXXOperatorCallExpr *E,
926+
const MatchFinder::MatchResult &Result,
927+
LatticeTransferState &State) {
928+
transferSmartPointerLikeCachedDeref(
929+
E, getSmartPtrLikeStorageLocation(*E->getArg(0), State.Env),
930+
State, [](StorageLocation &Loc) {});
931+
})
932+
.CaseOfCFGStmt<CXXOperatorCallExpr>(
933+
isPointerLikeOperatorArrow(),
934+
[](const CXXOperatorCallExpr *E,
935+
const MatchFinder::MatchResult &Result,
936+
LatticeTransferState &State) {
937+
transferSmartPointerLikeCachedGet(
938+
E, getSmartPtrLikeStorageLocation(*E->getArg(0), State.Env),
939+
State, [](StorageLocation &Loc) {});
940+
})
941+
.CaseOfCFGStmt<CXXMemberCallExpr>(
942+
isSmartPointerLikeValueMethodCall(),
943+
[](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result,
944+
LatticeTransferState &State) {
945+
transferSmartPointerLikeCachedDeref(
946+
E, getImplicitObjectLocation(*E, State.Env), State,
947+
[](StorageLocation &Loc) {});
948+
})
949+
.CaseOfCFGStmt<CXXMemberCallExpr>(
950+
isSmartPointerLikeGetMethodCall(),
951+
[](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result,
952+
LatticeTransferState &State) {
953+
transferSmartPointerLikeCachedGet(
954+
E, getImplicitObjectLocation(*E, State.Env), State,
955+
[](StorageLocation &Loc) {});
956+
})
909957
// const accessor calls
910958
.CaseOfCFGStmt<CXXMemberCallExpr>(isConstStatusOrAccessorMemberCall(),
911959
transferConstStatusOrAccessorMemberCall)

clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3443,6 +3443,79 @@ TEST_P(UncheckedStatusOrAccessModelTest, AccessorCall) {
34433443
)cc");
34443444
}
34453445

3446+
TEST_P(UncheckedStatusOrAccessModelTest, PointerLike) {
3447+
ExpectDiagnosticsFor(R"cc(
3448+
#include "unchecked_statusor_access_test_defs.h"
3449+
3450+
class Foo {
3451+
public:
3452+
std::pair<int, STATUSOR_VOIDPTR>& operator*() const;
3453+
std::pair<int, STATUSOR_VOIDPTR>* operator->() const;
3454+
bool operator!=(const Foo& other) const;
3455+
};
3456+
3457+
void target() {
3458+
Foo foo;
3459+
if (foo->second.ok() && *foo->second != nullptr) {
3460+
*foo->second;
3461+
(*foo).second.value();
3462+
}
3463+
}
3464+
)cc");
3465+
ExpectDiagnosticsFor(R"cc(
3466+
#include "unchecked_statusor_access_test_defs.h"
3467+
3468+
class Foo {
3469+
public:
3470+
std::pair<int, STATUSOR_INT>& operator*() const;
3471+
std::pair<int, STATUSOR_INT>* operator->() const;
3472+
};
3473+
void target() {
3474+
Foo foo;
3475+
if (!foo->second.ok()) return;
3476+
foo->second.value();
3477+
(*foo).second.value();
3478+
}
3479+
)cc");
3480+
ExpectDiagnosticsFor(R"cc(
3481+
#include "unchecked_statusor_access_test_defs.h"
3482+
3483+
void target(std::pair<int, STATUSOR_VOIDPTR>* foo) {
3484+
if (foo->second.ok() && *foo->second != nullptr) {
3485+
*foo->second;
3486+
(*foo).second.value();
3487+
}
3488+
}
3489+
)cc");
3490+
}
3491+
3492+
TEST_P(UncheckedStatusOrAccessModelTest, UniquePtr) {
3493+
ExpectDiagnosticsFor(
3494+
R"cc(
3495+
#include "unchecked_statusor_access_test_defs.h"
3496+
3497+
void target() {
3498+
auto sor_up = Make<std::unique_ptr<STATUSOR_INT>>();
3499+
if (sor_up->ok()) sor_up->value();
3500+
}
3501+
)cc");
3502+
}
3503+
3504+
TEST_P(UncheckedStatusOrAccessModelTest, UniquePtrReset) {
3505+
ExpectDiagnosticsFor(
3506+
R"cc(
3507+
#include "unchecked_statusor_access_test_defs.h"
3508+
3509+
void target() {
3510+
auto sor_up = Make<std::unique_ptr<STATUSOR_INT>>();
3511+
if (sor_up->ok()) {
3512+
sor_up.reset(Make<STATUSOR_INT*>());
3513+
sor_up->value(); // [[unsafe]]
3514+
}
3515+
}
3516+
)cc");
3517+
}
3518+
34463519
} // namespace
34473520

34483521
std::string
@@ -3492,6 +3565,7 @@ GetHeaders(UncheckedStatusOrAccessModelTestAliasKind AliasKind) {
34923565
#include "std_pair.h"
34933566
#include "absl_log.h"
34943567
#include "testing_defs.h"
3568+
#include "std_unique_ptr.h"
34953569
34963570
template <typename T>
34973571
T Make();

0 commit comments

Comments
 (0)