llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Florian Mayer (fmayer) <details> <summary>Changes</summary> This allows comparison which these status codes --- Full diff: https://github.com/llvm/llvm-project/pull/163872.diff 3 Files Affected: - (modified) clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp (+40-4) - (modified) clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp (+1-1) - (modified) clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp (+30) ``````````diff diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index 4ebf3e4251dd6..11dd0e73f7089 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -129,6 +129,24 @@ static auto isComparisonOperatorCall(llvm::StringRef operator_name) { hasArgument(1, anyOf(hasType(statusType()), hasType(statusOrType())))); } +static auto isOkStatusCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr(callee(functionDecl(hasName("::absl::OkStatus")))); +} + +static auto isNotOkStatusCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr(callee(functionDecl(hasAnyName( + "::absl::AbortedError", "::absl::AlreadyExistsError", + "::absl::CancelledError", "::absl::DataLossError", + "::absl::DeadlineExceededError", "::absl::FailedPreconditionError", + "::absl::InternalError", "::absl::InvalidArgumentError", + "::absl::NotFoundError", "::absl::OutOfRangeError", + "::absl::PermissionDeniedError", "::absl::ResourceExhaustedError", + "::absl::UnauthenticatedError", "::absl::UnavailableError", + "::absl::UnimplementedError", "::absl::UnknownError")))); +} + static auto buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) { return CFGMatchSwitchBuilder<const Environment, @@ -411,6 +429,22 @@ static void transferComparisonOperator(const CXXOperatorCallExpr* Expr, State.Env.setValue(*Expr, *LhsAndRhsVal); } +static void transferOkStatusCall(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + auto &OkVal = + initializeStatus(State.Env.getResultObjectLocation(*Expr), State.Env); + State.Env.assume(OkVal.formula()); +} + +static void transferNotOkStatusCall(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + auto &OkVal = + initializeStatus(State.Env.getResultObjectLocation(*Expr), State.Env); + auto &A = State.Env.arena(); + State.Env.assume(A.makeNot(OkVal.formula())); +} CFGMatchSwitch<LatticeTransferState> buildTransferMatchSwitch(ASTContext &Ctx, @@ -427,18 +461,20 @@ buildTransferMatchSwitch(ASTContext &Ctx, transferStatusUpdateCall) .CaseOfCFGStmt<CXXOperatorCallExpr>( isComparisonOperatorCall("=="), - [](const CXXOperatorCallExpr* Expr, const MatchFinder::MatchResult&, - LatticeTransferState& State) { + [](const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &, + LatticeTransferState &State) { transferComparisonOperator(Expr, State, /*IsNegative=*/false); }) .CaseOfCFGStmt<CXXOperatorCallExpr>( isComparisonOperatorCall("!="), - [](const CXXOperatorCallExpr* Expr, const MatchFinder::MatchResult&, - LatticeTransferState& State) { + [](const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &, + LatticeTransferState &State) { transferComparisonOperator(Expr, State, /*IsNegative=*/true); }) + .CaseOfCFGStmt<CallExpr>(isOkStatusCall(), transferOkStatusCall) + .CaseOfCFGStmt<CallExpr>(isNotOkStatusCall(), transferNotOkStatusCall) .Build(); } diff --git a/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp b/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp index d3dee58651396..461af73ea6c01 100644 --- a/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp +++ b/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp @@ -1356,7 +1356,7 @@ bool operator==(const Status &lhs, const Status &rhs); bool operator!=(const Status &lhs, const Status &rhs); Status OkStatus(); -Status InvalidArgumentError(char *); +Status InvalidArgumentError(const char *); #endif // STATUS_H )cc"; diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp index 99f04cc8fe7e7..e7fe378a6973d 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -2710,6 +2710,36 @@ TEST_P(UncheckedStatusOrAccessModelTest, EqualityCheck) { } } )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + if (sor.status() == absl::OkStatus()) + sor.value(); + else + sor.value(); // [[unsafe]] + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + if (sor.status() != absl::OkStatus()) + sor.value(); // [[unsafe]] + else + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + if (sor.status() != absl::InvalidArgumentError("oh no")) + sor.value(); // [[unsafe]] + else + sor.value(); // [[unsafe]] + } + )cc"); ExpectDiagnosticsFor( R"cc( #include "unchecked_statusor_access_test_defs.h" `````````` </details> https://github.com/llvm/llvm-project/pull/163872 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
