[FlowSensitive] [StatusOr] [6/N] Support pointer comparison

[fmayer]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Author: Florian Mayer (fmayer)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/163875.diff

2 Files Affected:

• (modified) clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp (+80)
• (modified) clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp (+57)

diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index 76f93d5a88c3e..dc0a8f7e4cfd6 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -146,6 +146,20 @@ static auto isNotOkStatusCall() { "::absl::UnimplementedError", "::absl::UnknownError")))); } +static auto isPointerComparisonOperatorCall(std::string operator_name) { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return binaryOperator( + hasOperatorName(operator_name), + hasLHS( + anyOf(hasType(hasCanonicalType(pointerType(pointee(statusOrType())))), + hasType(hasCanonicalType( + pointerType(pointee(possiblyAliasedStatusType())))))), + hasRHS( + anyOf(hasType(hasCanonicalType(pointerType(pointee(statusOrType())))), + hasType(hasCanonicalType( + pointerType(pointee(possiblyAliasedStatusType()))))))); +} + static auto buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) { return CFGMatchSwitchBuilder<const Environment, @@ -433,6 +447,58 @@ static void transferComparisonOperator(const CXXOperatorCallExpr *Expr, State.Env.setValue(*Expr, *LhsAndRhsVal); } +static RecordStorageLocation *getPointeeLocation(const Expr &Expr, + Environment &Env) { + if (auto *PointerVal = Env.get<PointerValue>(Expr)) + return dyn_cast<RecordStorageLocation>(&PointerVal->getPointeeLoc()); + return nullptr; +} + +static BoolValue *evaluatePointerEquality(const Expr *LhsExpr, + const Expr *RhsExpr, + Environment &Env) { + assert(LhsExpr->getType()->isPointerType()); + assert(RhsExpr->getType()->isPointerType()); + RecordStorageLocation *LhsStatusLoc = nullptr; + RecordStorageLocation *RhsStatusLoc = nullptr; + if (isStatusOrType(LhsExpr->getType()->getPointeeType()) && + isStatusOrType(RhsExpr->getType()->getPointeeType())) { + auto *LhsStatusOrLoc = getPointeeLocation(*LhsExpr, Env); + auto *RhsStatusOrLoc = getPointeeLocation(*RhsExpr, Env); + if (LhsStatusOrLoc == nullptr || RhsStatusOrLoc == nullptr) + return nullptr; + LhsStatusLoc = &locForStatus(*LhsStatusOrLoc); + RhsStatusLoc = &locForStatus(*RhsStatusOrLoc); + } else if (isStatusType(LhsExpr->getType()->getPointeeType()) && + isStatusType(RhsExpr->getType()->getPointeeType())) { + LhsStatusLoc = getPointeeLocation(*LhsExpr, Env); + RhsStatusLoc = getPointeeLocation(*RhsExpr, Env); + } + if (LhsStatusLoc == nullptr || RhsStatusLoc == nullptr) + return nullptr; + auto &LhsOkVal = valForOk(*LhsStatusLoc, Env); + auto &RhsOkVal = valForOk(*RhsStatusLoc, Env); + auto &Res = Env.makeAtomicBoolValue(); + auto &A = Env.arena(); + Env.assume(A.makeImplies( + Res.formula(), A.makeEquals(LhsOkVal.formula(), RhsOkVal.formula()))); + return &Res; +} + +static void transferPointerComparisonOperator(const BinaryOperator *Expr, + LatticeTransferState &State, + bool IsNegative) { + auto *LhsAndRhsVal = + evaluatePointerEquality(Expr->getLHS(), Expr->getRHS(), State.Env); + if (LhsAndRhsVal == nullptr) + return; + + if (IsNegative) + State.Env.setValue(*Expr, State.Env.makeNot(*LhsAndRhsVal)); + else + State.Env.setValue(*Expr, *LhsAndRhsVal); +} + static void transferOkStatusCall(const CallExpr *Expr, const MatchFinder::MatchResult &, LatticeTransferState &State) { @@ -477,6 +543,20 @@ buildTransferMatchSwitch(ASTContext &Ctx, transferComparisonOperator(Expr, State, /*IsNegative=*/true); }) + .CaseOfCFGStmt<BinaryOperator>( + isPointerComparisonOperatorCall("=="), + [](const BinaryOperator *Expr, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + transferPointerComparisonOperator(Expr, State, + /*IsNegative=*/false); + }) + .CaseOfCFGStmt<BinaryOperator>( + isPointerComparisonOperatorCall("!="), + [](const BinaryOperator *Expr, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + transferPointerComparisonOperator(Expr, State, + /*IsNegative=*/true); + }) .CaseOfCFGStmt<CallExpr>(isOkStatusCall(), transferOkStatusCall) .CaseOfCFGStmt<CallExpr>(isNotOkStatusCall(), transferNotOkStatusCall) .Build(); diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp index 2676dab7fd904..62e456ad07bdb 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -2858,6 +2858,63 @@ TEST_P(UncheckedStatusOrAccessModelTest, EqualityCheck) { )cc"); } +TEST_P(UncheckedStatusOrAccessModelTest, PointerEqualityCheck) { + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT* x, STATUSOR_INT* y) { + if (x->ok()) { + if (x == y) + y->value(); + else + y->value(); // [[unsafe]] + } + } + )cc"); + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT* x, STATUSOR_INT* y) { + if (x->ok()) { + if (x != y) + y->value(); // [[unsafe]] + else + y->value(); + } + } + )cc"); + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUS* x, STATUS* y) { + auto sor = Make<STATUSOR_INT>(); + if (x->ok()) { + if (x == y && sor.status() == *y) + sor.value(); + else + sor.value(); // [[unsafe]] + } + } + )cc"); + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUS* x, STATUS* y) { + auto sor = Make<STATUSOR_INT>(); + if (x->ok()) { + if (x != y) + sor.value(); // [[unsafe]] + else if (sor.status() == *y) + sor.value(); + } + } + )cc"); +} + } // namespace std::string 

[fmayer]

@BaLiKfromUA CC