diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index cf7c76ea216a2..7a698f276d6c1 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -201,6 +201,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 buildTransferMatchSwitch(ASTContext &Ctx, CFGMatchSwitchBuilder Builder) { @@ -628,6 +657,16 @@ buildTransferMatchSwitch(ASTContext &Ctx, transferValueAssignmentCall) .CaseOfCFGStmt(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(isStatusOrConstructor(), + transferStatusOrConstructor) + .CaseOfCFGStmt(isStatusConstructor(), + transferStatusConstructor) .Build(); } diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp index 1a7aba0aa6ca5..67c37e1e0f77f 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -3135,6 +3135,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(); + const auto sor2 = Make(); + 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(); + const auto sor2 = Make(); + const auto s1 = Make(); + const auto s2 = Make(); + + 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