Skip to content

Commit ddc638c

Browse files
fmayerfmayer
authored
[FlowSensitive] [StatusOr] [14/N] Support nested StatusOrs
Reviewers: jvoung Reviewed By: jvoung Pull Request: #170950
1 parent 6ae0b9f commit ddc638c

File tree

2 files changed

+207
-0
lines changed

2 files changed

+207
-0
lines changed

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,6 +1044,36 @@ transferAssertionResultOperatorBoolCall(const CXXMemberCallExpr *Expr,
10441044
State.Env.setValue(*Expr, Res);
10451045
}
10461046

1047+
static void transferDerefCall(const CXXOperatorCallExpr *Expr,
1048+
const MatchFinder::MatchResult &,
1049+
LatticeTransferState &State) {
1050+
auto *StatusOrLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0));
1051+
1052+
if (StatusOrLoc && State.Env.getStorageLocation(*Expr) == nullptr)
1053+
State.Env.setStorageLocation(*Expr,
1054+
StatusOrLoc->getSyntheticField("value"));
1055+
}
1056+
1057+
static void transferArrowCall(const CXXOperatorCallExpr *Expr,
1058+
const MatchFinder::MatchResult &,
1059+
LatticeTransferState &State) {
1060+
auto *StatusOrLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0));
1061+
if (!StatusOrLoc)
1062+
return;
1063+
State.Env.setValue(*Expr, State.Env.create<PointerValue>(
1064+
StatusOrLoc->getSyntheticField("value")));
1065+
}
1066+
1067+
static void transferValueCall(const CXXMemberCallExpr *Expr,
1068+
const MatchFinder::MatchResult &,
1069+
LatticeTransferState &State) {
1070+
auto *StatusOrLoc = getImplicitObjectLocation(*Expr, State.Env);
1071+
1072+
if (StatusOrLoc && State.Env.getStorageLocation(*Expr) == nullptr)
1073+
State.Env.setStorageLocation(*Expr,
1074+
StatusOrLoc->getSyntheticField("value"));
1075+
}
1076+
10471077
static RecordStorageLocation *
10481078
getSmartPtrLikeStorageLocation(const Expr &E, const Environment &Env) {
10491079
if (!E.isPRValue())
@@ -1130,6 +1160,12 @@ buildTransferMatchSwitch(ASTContext &Ctx,
11301160
transferValueAssignmentCall)
11311161
.CaseOfCFGStmt<CXXConstructExpr>(isStatusOrValueConstructor(),
11321162
transferValueConstructor)
1163+
.CaseOfCFGStmt<CXXOperatorCallExpr>(isStatusOrOperatorCallWithName("->"),
1164+
transferArrowCall)
1165+
.CaseOfCFGStmt<CXXOperatorCallExpr>(isStatusOrOperatorCallWithName("*"),
1166+
transferDerefCall)
1167+
.CaseOfCFGStmt<CXXMemberCallExpr>(isStatusOrMemberCallWithName("value"),
1168+
transferValueCall)
11331169
.CaseOfCFGStmt<CallExpr>(isAsStatusCallWithStatus(),
11341170
transferAsStatusCallWithStatus)
11351171
.CaseOfCFGStmt<CallExpr>(isAsStatusCallWithStatusOr(),

‎clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3419,6 +3419,15 @@ TEST_P(UncheckedStatusOrAccessModelTest, NestedStatusOr) {
34193419
result.value();
34203420
}
34213421
)cc");
3422+
ExpectDiagnosticsFor(R"cc(
3423+
#include "unchecked_statusor_access_test_defs.h"
3424+
3425+
void target() {
3426+
absl::StatusOr<STATUSOR_INT> result = Make<STATUSOR_INT>();
3427+
if (result.value().ok())
3428+
result.value().value();
3429+
}
3430+
)cc");
34223431
}
34233432

34243433
TEST_P(UncheckedStatusOrAccessModelTest, PtrConstruct) {
@@ -3739,6 +3748,168 @@ TEST_P(UncheckedStatusOrAccessModelTest, UniquePtrReset) {
37393748
)cc");
37403749
}
37413750

3751+
TEST_P(UncheckedStatusOrAccessModelTest, NestedStatusOrInStatusOrStruct) {
3752+
// Non-standard assignment with a nested StatusOr.
3753+
ExpectDiagnosticsFor(
3754+
R"cc(
3755+
#include "unchecked_statusor_access_test_defs.h"
3756+
3757+
struct Inner {
3758+
absl::StatusOr<std::string> sor;
3759+
};
3760+
3761+
struct Outer {
3762+
absl::StatusOr<Inner> inner;
3763+
};
3764+
3765+
void target() {
3766+
Outer foo = Make<Outer>();
3767+
foo.inner->sor = "a"; // [[unsafe]]
3768+
}
3769+
)cc");
3770+
3771+
ExpectDiagnosticsFor(
3772+
R"cc(
3773+
#include "unchecked_statusor_access_test_defs.h"
3774+
3775+
struct Foo {
3776+
absl::StatusOr<std::string> sor;
3777+
};
3778+
3779+
void target(const absl::StatusOr<Foo>& foo) {
3780+
if (foo.ok() && foo->sor.ok()) foo->sor.value();
3781+
}
3782+
)cc");
3783+
3784+
ExpectDiagnosticsFor(
3785+
R"cc(
3786+
#include "unchecked_statusor_access_test_defs.h"
3787+
3788+
struct Foo {
3789+
absl::StatusOr<std::string> sor;
3790+
};
3791+
3792+
void target(const absl::StatusOr<Foo>& foo) {
3793+
if (foo.ok() && foo.value().sor.ok()) foo->sor.value();
3794+
}
3795+
)cc");
3796+
3797+
ExpectDiagnosticsFor(
3798+
R"cc(
3799+
#include "unchecked_statusor_access_test_defs.h"
3800+
3801+
struct Foo {
3802+
absl::StatusOr<std::string> sor;
3803+
};
3804+
3805+
void target(const absl::StatusOr<Foo>& foo) {
3806+
if (foo.ok() && (*foo).sor.ok()) (*foo).sor.value();
3807+
}
3808+
)cc");
3809+
3810+
ExpectDiagnosticsFor(
3811+
R"cc(
3812+
#include "unchecked_statusor_access_test_defs.h"
3813+
3814+
struct Foo {
3815+
absl::StatusOr<std::string> sor;
3816+
};
3817+
3818+
void target(absl::StatusOr<Foo>& foo) {
3819+
if (foo.ok() && foo->sor.ok()) *foo->sor;
3820+
}
3821+
)cc");
3822+
// With assignment.
3823+
ExpectDiagnosticsFor(
3824+
R"cc(
3825+
#include "unchecked_statusor_access_test_defs.h"
3826+
3827+
struct Foo {
3828+
absl::StatusOr<std::string> sor;
3829+
};
3830+
3831+
void target(absl::StatusOr<Foo>& foo) {
3832+
if (foo.ok() && foo->sor.ok()) {
3833+
foo->sor = Make<absl::StatusOr<std::string>>();
3834+
foo->sor.value(); // [[unsafe]]
3835+
}
3836+
}
3837+
)cc");
3838+
3839+
ExpectDiagnosticsFor(
3840+
R"cc(
3841+
#include "unchecked_statusor_access_test_defs.h"
3842+
3843+
struct Foo {
3844+
absl::StatusOr<std::string> sor;
3845+
};
3846+
3847+
void target(absl::StatusOr<Foo>& foo) {
3848+
if (foo.ok() && foo->sor.ok()) {
3849+
auto n = Make<absl::StatusOr<std::string>>();
3850+
if (n.ok()) foo->sor = n;
3851+
foo->sor.value();
3852+
}
3853+
}
3854+
)cc");
3855+
3856+
ExpectDiagnosticsFor(
3857+
R"cc(
3858+
#include "unchecked_statusor_access_test_defs.h"
3859+
3860+
struct Foo {
3861+
absl::StatusOr<std::string> sor;
3862+
};
3863+
3864+
void target(absl::StatusOr<Foo>& foo) {
3865+
if (foo.ok() && foo->sor.ok()) {
3866+
auto n = Make<absl::StatusOr<std::string>>();
3867+
if (n.ok()) foo->sor = std::move(n);
3868+
foo->sor.value();
3869+
}
3870+
}
3871+
)cc");
3872+
3873+
// More complicated conditionals.
3874+
ExpectDiagnosticsFor(
3875+
R"cc(
3876+
#include "unchecked_statusor_access_test_defs.h"
3877+
3878+
struct Foo {
3879+
absl::StatusOr<std::string> sor;
3880+
};
3881+
3882+
void target(absl::StatusOr<Foo>& foo) {
3883+
if (!foo.ok()) return;
3884+
if (!foo->sor.ok())
3885+
foo->sor.value(); // [[unsafe]]
3886+
else
3887+
foo->sor.value();
3888+
}
3889+
)cc");
3890+
3891+
ExpectDiagnosticsFor(
3892+
R"cc(
3893+
#include "unchecked_statusor_access_test_defs.h"
3894+
3895+
struct Foo {
3896+
absl::StatusOr<std::string> sor;
3897+
};
3898+
3899+
void target(absl::StatusOr<Foo>& foo, bool b) {
3900+
if (!foo.ok()) return;
3901+
if (b) {
3902+
if (!foo->sor.ok()) return;
3903+
foo->sor.value();
3904+
} else {
3905+
if (!foo->sor.ok()) return;
3906+
foo->sor.value();
3907+
}
3908+
foo->sor.value();
3909+
}
3910+
)cc");
3911+
}
3912+
37423913
} // namespace
37433914

37443915
std::string

0 commit comments

Comments
 (0)