Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,16 @@ static auto isStatusOrValueConstructor() {
"std::in_place_t"))))));
}

static auto isStatusOrConstructor() {
using namespace ::clang::ast_matchers; // NOLINT: Too many names
return cxxConstructExpr(hasType(statusOrType()));
}

static auto isStatusConstructor() {
using namespace ::clang::ast_matchers; // NOLINT: Too many names
return cxxConstructExpr(hasType(statusType()));
}

static auto
buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) {
return CFGMatchSwitchBuilder<const Environment,
Expand Down Expand Up @@ -574,6 +584,25 @@ static void transferValueConstructor(const CXXConstructExpr *Expr,
State.Env.assume(OkVal.formula());
}

static void transferStatusOrConstructor(const CXXConstructExpr *Expr,
const MatchFinder::MatchResult &,
LatticeTransferState &State) {
RecordStorageLocation &StatusOrLoc = State.Env.getResultObjectLocation(*Expr);
RecordStorageLocation &StatusLoc = locForStatus(StatusOrLoc);

if (State.Env.getValue(locForOk(StatusLoc)) == nullptr)
initializeStatusOr(StatusOrLoc, State.Env);
}

static void transferStatusConstructor(const CXXConstructExpr *Expr,
const MatchFinder::MatchResult &,
LatticeTransferState &State) {
RecordStorageLocation &StatusLoc = State.Env.getResultObjectLocation(*Expr);

if (State.Env.getValue(locForOk(StatusLoc)) == nullptr)
initializeStatus(StatusLoc, State.Env);
}

CFGMatchSwitch<LatticeTransferState>
buildTransferMatchSwitch(ASTContext &Ctx,
CFGMatchSwitchBuilder<LatticeTransferState> Builder) {
Expand Down Expand Up @@ -623,6 +652,16 @@ buildTransferMatchSwitch(ASTContext &Ctx,
transferValueAssignmentCall)
.CaseOfCFGStmt<CXXConstructExpr>(isStatusOrValueConstructor(),
transferValueConstructor)
// N.B. These need to come after all other CXXConstructExpr.
// These are there to make sure that every Status and StatusOr object
// have their ok boolean initialized when constructed. If we were to
// lazily initialize them when we first access them, we can produce
// false positives if that first access is in a control flow statement.
// You can comment out these two constructors and see tests fail.
.CaseOfCFGStmt<CXXConstructExpr>(isStatusOrConstructor(),
transferStatusOrConstructor)
.CaseOfCFGStmt<CXXConstructExpr>(isStatusConstructor(),
transferStatusConstructor)
.Build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3147,6 +3147,45 @@ TEST_P(UncheckedStatusOrAccessModelTest, InPlaceConstruct) {
)cc");
}

TEST_P(UncheckedStatusOrAccessModelTest, ConstructStatusOrFromReference) {
ExpectDiagnosticsFor(R"cc(
#include "unchecked_statusor_access_test_defs.h"
void target() {
const auto sor1 = Make<STATUSOR_INT&>();
const auto sor2 = Make<STATUSOR_INT&>();
if (!sor1.ok() && !sor2.ok()) return;
if (sor1.ok() && !sor2.ok()) {
} else if (!sor1.ok() && sor2.ok()) {
} else {
sor1.value();
sor2.value();
}
}
)cc");
}

TEST_P(UncheckedStatusOrAccessModelTest, ConstructStatusFromReference) {
ExpectDiagnosticsFor(R"cc(
#include "unchecked_statusor_access_test_defs.h"

void target() {
const auto sor1 = Make<STATUSOR_INT&>();
const auto sor2 = Make<STATUSOR_INT&>();
const auto s1 = Make<STATUS&>();
const auto s2 = Make<STATUS&>();

if (!s1.ok() && !s2.ok()) return;
if (s1.ok() && !s2.ok()) {
} else if (!s1.ok() && s2.ok()) {
} else {
if (s1 != sor1.status() || s2 != sor2.status()) return;
sor1.value();
sor2.value();
}
}
)cc");
}

} // namespace

std::string
Expand Down
Loading