Skip to content

Commit f5de387

Browse files
fmayerLukacma
fmayer
authored and
Lukacma
committed
[FlowSensitive] [StatusOr] [8/N] Support value ctor and assignment
Reviewers: jvoung, Xazax-hun Reviewed By: jvoung Pull Request: llvm#163894
1 parent 52f534b commit f5de387

File tree

2 files changed

+222
-0
lines changed

2 files changed

+222
-0
lines changed

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,31 @@ static auto isPointerComparisonOperatorCall(std::string operator_name) {
177177
pointee(anyOf(statusOrType(), statusType())))))));
178178
}
179179

180+
// The nullPointerConstant in the two matchers below is to support
181+
// absl::StatusOr<void*> X = nullptr.
182+
// nullptr does not match the bound type.
183+
// TODO: be less restrictive around convertible types in general.
184+
static auto isStatusOrValueAssignmentCall() {
185+
using namespace ::clang::ast_matchers; // NOLINT: Too many names
186+
return cxxOperatorCallExpr(
187+
hasOverloadedOperatorName("="),
188+
callee(cxxMethodDecl(ofClass(statusOrClass()))),
189+
hasArgument(1, anyOf(hasType(hasUnqualifiedDesugaredType(
190+
type(equalsBoundNode("T")))),
191+
nullPointerConstant())));
192+
}
193+
194+
static auto isStatusOrValueConstructor() {
195+
using namespace ::clang::ast_matchers; // NOLINT: Too many names
196+
return cxxConstructExpr(
197+
hasType(statusOrType()),
198+
hasArgument(0,
199+
anyOf(hasType(hasCanonicalType(type(equalsBoundNode("T")))),
200+
nullPointerConstant(),
201+
hasType(namedDecl(hasAnyName("absl::in_place_t",
202+
"std::in_place_t"))))));
203+
}
204+
180205
static auto
181206
buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) {
182207
return CFGMatchSwitchBuilder<const Environment,
@@ -528,6 +553,27 @@ static void transferEmplaceCall(const CXXMemberCallExpr *Expr,
528553
State.Env.assume(OkVal.formula());
529554
}
530555

556+
static void transferValueAssignmentCall(const CXXOperatorCallExpr *Expr,
557+
const MatchFinder::MatchResult &,
558+
LatticeTransferState &State) {
559+
assert(Expr->getNumArgs() > 1);
560+
561+
auto *StatusOrLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0));
562+
if (StatusOrLoc == nullptr)
563+
return;
564+
565+
auto &OkVal = initializeStatusOr(*StatusOrLoc, State.Env);
566+
State.Env.assume(OkVal.formula());
567+
}
568+
569+
static void transferValueConstructor(const CXXConstructExpr *Expr,
570+
const MatchFinder::MatchResult &,
571+
LatticeTransferState &State) {
572+
auto &OkVal =
573+
initializeStatusOr(State.Env.getResultObjectLocation(*Expr), State.Env);
574+
State.Env.assume(OkVal.formula());
575+
}
576+
531577
CFGMatchSwitch<LatticeTransferState>
532578
buildTransferMatchSwitch(ASTContext &Ctx,
533579
CFGMatchSwitchBuilder<LatticeTransferState> Builder) {
@@ -573,6 +619,10 @@ buildTransferMatchSwitch(ASTContext &Ctx,
573619
.CaseOfCFGStmt<CallExpr>(isNotOkStatusCall(), transferNotOkStatusCall)
574620
.CaseOfCFGStmt<CXXMemberCallExpr>(isStatusOrMemberCallWithName("emplace"),
575621
transferEmplaceCall)
622+
.CaseOfCFGStmt<CXXOperatorCallExpr>(isStatusOrValueAssignmentCall(),
623+
transferValueAssignmentCall)
624+
.CaseOfCFGStmt<CXXConstructExpr>(isStatusOrValueConstructor(),
625+
transferValueConstructor)
576626
.Build();
577627
}
578628

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

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2975,6 +2975,178 @@ TEST_P(UncheckedStatusOrAccessModelTest, Emplace) {
29752975
)cc");
29762976
}
29772977

2978+
TEST_P(UncheckedStatusOrAccessModelTest, ValueConstruction) {
2979+
ExpectDiagnosticsFor(R"cc(
2980+
#include "unchecked_statusor_access_test_defs.h"
2981+
2982+
void target() {
2983+
STATUSOR_BOOL result = false;
2984+
result.value();
2985+
}
2986+
)cc");
2987+
ExpectDiagnosticsFor(R"cc(
2988+
#include "unchecked_statusor_access_test_defs.h"
2989+
2990+
void target() {
2991+
STATUSOR_INT result = 21;
2992+
result.value();
2993+
}
2994+
)cc");
2995+
ExpectDiagnosticsFor(R"cc(
2996+
#include "unchecked_statusor_access_test_defs.h"
2997+
2998+
void target() {
2999+
STATUSOR_INT result = Make<STATUSOR_INT>();
3000+
result.value(); // [[unsafe]]
3001+
}
3002+
)cc");
3003+
ExpectDiagnosticsFor(
3004+
R"cc(
3005+
#include "unchecked_statusor_access_test_defs.h"
3006+
3007+
void target() {
3008+
STATUSOR_BOOL result = false;
3009+
if (result.ok())
3010+
result.value();
3011+
else
3012+
result.value();
3013+
}
3014+
)cc");
3015+
3016+
ExpectDiagnosticsFor(R"cc(
3017+
#include "unchecked_statusor_access_test_defs.h"
3018+
3019+
void target() {
3020+
STATUSOR_BOOL result(false);
3021+
result.value();
3022+
}
3023+
)cc");
3024+
ExpectDiagnosticsFor(R"cc(
3025+
#include "unchecked_statusor_access_test_defs.h"
3026+
3027+
void target() {
3028+
STATUSOR_INT result(21);
3029+
result.value();
3030+
}
3031+
)cc");
3032+
ExpectDiagnosticsFor(R"cc(
3033+
#include "unchecked_statusor_access_test_defs.h"
3034+
3035+
void target() {
3036+
STATUSOR_INT result(Make<STATUSOR_INT>());
3037+
result.value(); // [[unsafe]]
3038+
}
3039+
)cc");
3040+
ExpectDiagnosticsFor(
3041+
R"cc(
3042+
#include "unchecked_statusor_access_test_defs.h"
3043+
3044+
void target() {
3045+
STATUSOR_BOOL result(false);
3046+
if (result.ok())
3047+
result.value();
3048+
else
3049+
result.value();
3050+
}
3051+
)cc");
3052+
}
3053+
3054+
TEST_P(UncheckedStatusOrAccessModelTest, ValueAssignment) {
3055+
ExpectDiagnosticsFor(R"cc(
3056+
#include "unchecked_statusor_access_test_defs.h"
3057+
3058+
void target() {
3059+
STATUSOR_BOOL result;
3060+
result = false;
3061+
result.value();
3062+
}
3063+
)cc");
3064+
ExpectDiagnosticsFor(R"cc(
3065+
#include "unchecked_statusor_access_test_defs.h"
3066+
3067+
void target() {
3068+
STATUSOR_INT result;
3069+
result = 21;
3070+
result.value();
3071+
}
3072+
)cc");
3073+
ExpectDiagnosticsFor(R"cc(
3074+
#include "unchecked_statusor_access_test_defs.h"
3075+
3076+
void target() {
3077+
STATUSOR_INT result;
3078+
result = Make<STATUSOR_INT>();
3079+
result.value(); // [[unsafe]]
3080+
}
3081+
)cc");
3082+
ExpectDiagnosticsFor(
3083+
R"cc(
3084+
#include "unchecked_statusor_access_test_defs.h"
3085+
3086+
void target() {
3087+
STATUSOR_BOOL result;
3088+
result = false;
3089+
if (result.ok())
3090+
result.value();
3091+
else
3092+
result.value();
3093+
}
3094+
)cc");
3095+
}
3096+
3097+
TEST_P(UncheckedStatusOrAccessModelTest, NestedStatusOr) {
3098+
ExpectDiagnosticsFor(R"cc(
3099+
#include "unchecked_statusor_access_test_defs.h"
3100+
3101+
void target() {
3102+
absl::StatusOr<STATUSOR_INT> result;
3103+
result = Make<STATUSOR_INT>();
3104+
result.value();
3105+
}
3106+
)cc");
3107+
ExpectDiagnosticsFor(R"cc(
3108+
#include "unchecked_statusor_access_test_defs.h"
3109+
3110+
void target() {
3111+
absl::StatusOr<STATUSOR_INT> result = Make<STATUSOR_INT>();
3112+
result.value();
3113+
}
3114+
)cc");
3115+
}
3116+
3117+
TEST_P(UncheckedStatusOrAccessModelTest, PtrConstruct) {
3118+
ExpectDiagnosticsFor(R"cc(
3119+
#include "unchecked_statusor_access_test_defs.h"
3120+
3121+
void target() {
3122+
STATUSOR_VOIDPTR sor = nullptr;
3123+
*sor;
3124+
}
3125+
)cc");
3126+
3127+
ExpectDiagnosticsFor(R"cc(
3128+
#include "unchecked_statusor_access_test_defs.h"
3129+
3130+
void target() {
3131+
STATUSOR_VOIDPTR sor(nullptr);
3132+
*sor;
3133+
}
3134+
)cc");
3135+
}
3136+
3137+
TEST_P(UncheckedStatusOrAccessModelTest, InPlaceConstruct) {
3138+
ExpectDiagnosticsFor(R"cc(
3139+
#include "unchecked_statusor_access_test_defs.h"
3140+
3141+
void target() {
3142+
STATUSOR_VOIDPTR absl_sor(absl::in_place, {nullptr});
3143+
*absl_sor;
3144+
STATUSOR_VOIDPTR std_sor(std::in_place, {nullptr});
3145+
*std_sor;
3146+
}
3147+
)cc");
3148+
}
3149+
29783150
} // namespace
29793151

29803152
std::string

0 commit comments

Comments
 (0)