Skip to content

Commit cd0ea93

Browse files
fmayerfmayer
committed
[𝘀𝗽𝗿] changes to main this commit is based on
Created using spr 1.3.7 [skip ci]
1 parent 0e0ec4c commit cd0ea93

File tree

3 files changed

+337
-1
lines changed

3 files changed

+337
-1
lines changed

‎clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "clang/ASTMatchers/ASTMatchers.h"
1414
#include "clang/Analysis/CFG.h"
1515
#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
16+
#include "clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h"
1617
#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
1718
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
1819
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
@@ -69,7 +70,8 @@ struct UncheckedStatusOrAccessModelOptions {};
6970

7071
// Dataflow analysis that discovers unsafe uses of StatusOr values.
7172
class UncheckedStatusOrAccessModel
72-
: public DataflowAnalysis<UncheckedStatusOrAccessModel, NoopLattice> {
73+
: public DataflowAnalysis<UncheckedStatusOrAccessModel,
74+
CachedConstAccessorsLattice<NoopLattice>> {
7375
public:
7476
explicit UncheckedStatusOrAccessModel(ASTContext &Ctx, Environment &Env);
7577

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

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,49 @@ static auto isAsStatusCallWithStatusOr() {
237237
hasArgument(0, hasType(statusOrType())));
238238
}
239239

240+
static auto possiblyReferencedStatusOrType() {
241+
using namespace ::clang::ast_matchers; // NOLINT: Too many names
242+
return anyOf(statusOrType(), referenceType(pointee(statusOrType())));
243+
}
244+
245+
static auto isConstStatusOrAccessorMemberCall() {
246+
using namespace ::clang::ast_matchers; // NOLINT: Too many names
247+
return cxxMemberCallExpr(callee(
248+
cxxMethodDecl(parameterCountIs(0), isConst(),
249+
returns(qualType(possiblyReferencedStatusOrType())))));
250+
}
251+
252+
static auto isConstStatusOrAccessorMemberOperatorCall() {
253+
using namespace ::clang::ast_matchers; // NOLINT: Too many names
254+
return cxxOperatorCallExpr(
255+
callee(cxxMethodDecl(parameterCountIs(0), isConst(),
256+
returns(possiblyReferencedStatusOrType()))));
257+
}
258+
259+
static auto isConstStatusOrPointerAccessorMemberCall() {
260+
using namespace ::clang::ast_matchers; // NOLINT: Too many names
261+
return cxxMemberCallExpr(callee(cxxMethodDecl(
262+
parameterCountIs(0), isConst(),
263+
returns(pointerType(pointee(possiblyReferencedStatusOrType()))))));
264+
}
265+
266+
static auto isConstStatusOrPointerAccessorMemberOperatorCall() {
267+
using namespace ::clang::ast_matchers; // NOLINT: Too many names
268+
return cxxOperatorCallExpr(callee(cxxMethodDecl(
269+
parameterCountIs(0), isConst(),
270+
returns(pointerType(pointee(possiblyReferencedStatusOrType()))))));
271+
}
272+
273+
static auto isNonConstMemberCall() {
274+
using namespace ::clang::ast_matchers; // NOLINT: Too many names
275+
return cxxMemberCallExpr(callee(cxxMethodDecl(unless(isConst()))));
276+
}
277+
278+
static auto isNonConstMemberOperatorCall() {
279+
using namespace ::clang::ast_matchers; // NOLINT: Too many names
280+
return cxxOperatorCallExpr(callee(cxxMethodDecl(unless(isConst()))));
281+
}
282+
240283
static auto
241284
buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) {
242285
return CFGMatchSwitchBuilder<const Environment,
@@ -697,6 +740,107 @@ static void transferPointerToBoolean(const ImplicitCastExpr *Expr,
697740
dyn_cast_or_null<BoolValue>(State.Env.getValue(*Expr->getSubExpr())))
698741
State.Env.setValue(*Expr, *SubExprVal);
699742
}
743+
static void handleConstStatusOrAccessorMemberCall(
744+
const CallExpr *Expr, RecordStorageLocation *RecordLoc,
745+
const MatchFinder::MatchResult &Result, LatticeTransferState &State) {
746+
assert(isStatusOrType(Expr->getType()));
747+
if (RecordLoc == nullptr)
748+
return;
749+
const FunctionDecl *DirectCallee = Expr->getDirectCallee();
750+
if (DirectCallee == nullptr)
751+
return;
752+
StorageLocation &Loc =
753+
State.Lattice.getOrCreateConstMethodReturnStorageLocation(
754+
*RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) {
755+
initializeStatusOr(cast<RecordStorageLocation>(Loc), State.Env);
756+
});
757+
if (Expr->isPRValue()) {
758+
auto &ResultLoc = State.Env.getResultObjectLocation(*Expr);
759+
copyRecord(cast<RecordStorageLocation>(Loc), ResultLoc, State.Env);
760+
} else {
761+
State.Env.setStorageLocation(*Expr, Loc);
762+
}
763+
}
764+
765+
static void handleConstStatusOrPointerAccessorMemberCall(
766+
const CallExpr *Expr, RecordStorageLocation *RecordLoc,
767+
const MatchFinder::MatchResult &Result, LatticeTransferState &State) {
768+
if (RecordLoc == nullptr)
769+
return;
770+
auto *Val = State.Lattice.getOrCreateConstMethodReturnValue(*RecordLoc, Expr,
771+
State.Env);
772+
State.Env.setValue(*Expr, *Val);
773+
}
774+
775+
static void
776+
transferConstStatusOrAccessorMemberCall(const CXXMemberCallExpr *Expr,
777+
const MatchFinder::MatchResult &Result,
778+
LatticeTransferState &State) {
779+
handleConstStatusOrAccessorMemberCall(
780+
Expr, getImplicitObjectLocation(*Expr, State.Env), Result, State);
781+
}
782+
783+
static void transferConstStatusOrAccessorMemberOperatorCall(
784+
const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &Result,
785+
LatticeTransferState &State) {
786+
auto *RecordLoc = cast_or_null<RecordStorageLocation>(
787+
State.Env.getStorageLocation(*Expr->getArg(0)));
788+
handleConstStatusOrAccessorMemberCall(Expr, RecordLoc, Result, State);
789+
}
790+
791+
static void transferConstStatusOrPointerAccessorMemberCall(
792+
const CXXMemberCallExpr *Expr, const MatchFinder::MatchResult &Result,
793+
LatticeTransferState &State) {
794+
handleConstStatusOrPointerAccessorMemberCall(
795+
Expr, getImplicitObjectLocation(*Expr, State.Env), Result, State);
796+
}
797+
798+
static void transferConstStatusOrPointerAccessorMemberOperatorCall(
799+
const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &Result,
800+
LatticeTransferState &State) {
801+
auto *RecordLoc = cast_or_null<RecordStorageLocation>(
802+
State.Env.getStorageLocation(*Expr->getArg(0)));
803+
handleConstStatusOrPointerAccessorMemberCall(Expr, RecordLoc, Result, State);
804+
}
805+
806+
static void transferStatusOrReturningCall(const CallExpr *Expr,
807+
LatticeTransferState &State) {
808+
RecordStorageLocation *StatusOrLoc =
809+
Expr->isPRValue() ? &State.Env.getResultObjectLocation(*Expr)
810+
: State.Env.get<RecordStorageLocation>(*Expr);
811+
if (StatusOrLoc != nullptr &&
812+
State.Env.getValue(locForOk(locForStatus(*StatusOrLoc))) == nullptr)
813+
initializeStatusOr(*StatusOrLoc, State.Env);
814+
}
815+
816+
static void handleNonConstMemberCall(const CallExpr *Expr,
817+
RecordStorageLocation *RecordLoc,
818+
const MatchFinder::MatchResult &Result,
819+
LatticeTransferState &State) {
820+
if (RecordLoc == nullptr)
821+
return;
822+
State.Lattice.clearConstMethodReturnValues(*RecordLoc);
823+
State.Lattice.clearConstMethodReturnStorageLocations(*RecordLoc);
824+
825+
if (isStatusOrType(Expr->getType()))
826+
transferStatusOrReturningCall(Expr, State);
827+
}
828+
829+
static void transferNonConstMemberCall(const CXXMemberCallExpr *Expr,
830+
const MatchFinder::MatchResult &Result,
831+
LatticeTransferState &State) {
832+
handleNonConstMemberCall(Expr, getImplicitObjectLocation(*Expr, State.Env),
833+
Result, State);
834+
}
835+
836+
static void
837+
transferNonConstMemberOperatorCall(const CXXOperatorCallExpr *Expr,
838+
const MatchFinder::MatchResult &Result,
839+
LatticeTransferState &State) {
840+
auto *RecordLoc = cast_or_null<RecordStorageLocation>(
841+
State.Env.getStorageLocation(*Expr->getArg(0)));
842+
handleNonConstMemberCall(Expr, RecordLoc, Result, State);
843+
}
700844

701845
CFGMatchSwitch<LatticeTransferState>
702846
buildTransferMatchSwitch(ASTContext &Ctx,
@@ -755,6 +899,23 @@ buildTransferMatchSwitch(ASTContext &Ctx,
755899
transferLoggingGetReferenceableValueCall)
756900
.CaseOfCFGStmt<CallExpr>(isLoggingCheckEqImpl(),
757901
transferLoggingCheckEqImpl)
902+
// const accessor calls
903+
.CaseOfCFGStmt<CXXMemberCallExpr>(isConstStatusOrAccessorMemberCall(),
904+
transferConstStatusOrAccessorMemberCall)
905+
.CaseOfCFGStmt<CXXOperatorCallExpr>(
906+
isConstStatusOrAccessorMemberOperatorCall(),
907+
transferConstStatusOrAccessorMemberOperatorCall)
908+
.CaseOfCFGStmt<CXXMemberCallExpr>(
909+
isConstStatusOrPointerAccessorMemberCall(),
910+
transferConstStatusOrPointerAccessorMemberCall)
911+
.CaseOfCFGStmt<CXXOperatorCallExpr>(
912+
isConstStatusOrPointerAccessorMemberOperatorCall(),
913+
transferConstStatusOrPointerAccessorMemberOperatorCall)
914+
// non-const member calls that may modify the state of an object.
915+
.CaseOfCFGStmt<CXXMemberCallExpr>(isNonConstMemberCall(),
916+
transferNonConstMemberCall)
917+
.CaseOfCFGStmt<CXXOperatorCallExpr>(isNonConstMemberOperatorCall(),
918+
transferNonConstMemberOperatorCall)
758919
// N.B. These need to come after all other CXXConstructExpr.
759920
// These are there to make sure that every Status and StatusOr object
760921
// have their ok boolean initialized when constructed. If we were to

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

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3270,6 +3270,179 @@ TEST_P(UncheckedStatusOrAccessModelTest, ConstructStatusFromReference) {
32703270
)cc");
32713271
}
32723272

3273+
TEST_P(UncheckedStatusOrAccessModelTest, AccessorCall) {
3274+
// Accessor returns reference.
3275+
ExpectDiagnosticsFor(
3276+
R"cc(
3277+
#include "unchecked_statusor_access_test_defs.h"
3278+
3279+
struct Foo {
3280+
STATUSOR_INT sor_;
3281+
3282+
const STATUSOR_INT& sor() const { return sor_; }
3283+
};
3284+
3285+
void target(Foo foo) {
3286+
if (foo.sor().ok()) foo.sor().value();
3287+
}
3288+
)cc");
3289+
3290+
// Uses an operator
3291+
ExpectDiagnosticsFor(
3292+
R"cc(
3293+
#include "unchecked_statusor_access_test_defs.h"
3294+
3295+
struct Foo {
3296+
STATUSOR_INT sor_;
3297+
3298+
const STATUSOR_INT& operator()() const { return sor_; }
3299+
};
3300+
3301+
void target(Foo foo) {
3302+
if (foo().ok()) foo().value();
3303+
}
3304+
)cc");
3305+
3306+
// Calls nonconst method inbetween.
3307+
ExpectDiagnosticsFor(
3308+
R"cc(
3309+
#include "unchecked_statusor_access_test_defs.h"
3310+
3311+
struct Foo {
3312+
STATUSOR_INT sor_;
3313+
3314+
void invalidate() {}
3315+
3316+
const STATUSOR_INT& sor() const { return sor_; }
3317+
};
3318+
3319+
void target(Foo foo) {
3320+
if (foo.sor().ok()) {
3321+
foo.invalidate();
3322+
foo.sor().value(); // [[unsafe]]
3323+
}
3324+
}
3325+
)cc");
3326+
3327+
// Calls nonconst operator inbetween.
3328+
ExpectDiagnosticsFor(
3329+
R"cc(
3330+
#include "unchecked_statusor_access_test_defs.h"
3331+
3332+
struct Foo {
3333+
STATUSOR_INT sor_;
3334+
3335+
void operator()() {}
3336+
3337+
const STATUSOR_INT& sor() const { return sor_; }
3338+
};
3339+
3340+
void target(Foo foo) {
3341+
if (foo.sor().ok()) {
3342+
foo();
3343+
foo.sor().value(); // [[unsafe]]
3344+
}
3345+
}
3346+
)cc");
3347+
3348+
// Accessor returns copy.
3349+
ExpectDiagnosticsFor(
3350+
R"cc(
3351+
#include "unchecked_statusor_access_test_defs.h"
3352+
3353+
struct Foo {
3354+
STATUSOR_INT sor_;
3355+
3356+
STATUSOR_INT sor() const { return sor_; }
3357+
};
3358+
3359+
void target(Foo foo) {
3360+
if (foo.sor().ok()) foo.sor().value();
3361+
}
3362+
)cc");
3363+
3364+
// Non-const accessor.
3365+
ExpectDiagnosticsFor(
3366+
R"cc(
3367+
#include "unchecked_statusor_access_test_defs.h"
3368+
3369+
struct Foo {
3370+
STATUSOR_INT sor_;
3371+
3372+
const STATUSOR_INT& sor() { return sor_; }
3373+
};
3374+
3375+
void target(Foo foo) {
3376+
if (foo.sor().ok()) foo.sor().value(); // [[unsafe]]
3377+
}
3378+
)cc");
3379+
3380+
// Non-const rvalue accessor.
3381+
ExpectDiagnosticsFor(
3382+
R"cc(
3383+
#include "unchecked_statusor_access_test_defs.h"
3384+
3385+
struct Foo {
3386+
STATUSOR_INT sor_;
3387+
3388+
STATUSOR_INT&& sor() { return std::move(sor_); }
3389+
};
3390+
3391+
void target(Foo foo) {
3392+
if (foo.sor().ok()) foo.sor().value(); // [[unsafe]]
3393+
}
3394+
)cc");
3395+
3396+
// const pointer accessor.
3397+
ExpectDiagnosticsFor(
3398+
R"cc(
3399+
#include "unchecked_statusor_access_test_defs.h"
3400+
3401+
struct Foo {
3402+
STATUSOR_INT sor_;
3403+
3404+
const STATUSOR_INT* sor() const { return &sor_; }
3405+
};
3406+
3407+
void target(Foo foo) {
3408+
if (foo.sor()->ok()) foo.sor()->value();
3409+
}
3410+
)cc");
3411+
3412+
// const pointer operator.
3413+
ExpectDiagnosticsFor(
3414+
R"cc(
3415+
#include "unchecked_statusor_access_test_defs.h"
3416+
3417+
struct Foo {
3418+
STATUSOR_INT sor_;
3419+
3420+
const STATUSOR_INT* operator->() const { return &sor_; }
3421+
};
3422+
3423+
void target(Foo foo) {
3424+
if (foo->ok()) foo->value();
3425+
}
3426+
)cc");
3427+
3428+
// We copy the result of the accessor.
3429+
ExpectDiagnosticsFor(R"cc(
3430+
#include "unchecked_statusor_access_test_defs.h"
3431+
3432+
struct Foo {
3433+
STATUSOR_INT sor_;
3434+
3435+
const STATUSOR_INT& sor() const { return sor_; }
3436+
};
3437+
void target() {
3438+
Foo foo;
3439+
if (!foo.sor().ok()) return;
3440+
const auto sor = foo.sor();
3441+
sor.value();
3442+
}
3443+
)cc");
3444+
}
3445+
32733446
} // namespace
32743447

32753448
std::string

0 commit comments

Comments
 (0)