llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis Author: Florian Mayer (fmayer) <details> <summary>Changes</summary> Reverts llvm/llvm-project#<!-- -->162932 ``` Failed Tests (1): Clang-Unit :: ./AllClangUnitTests/failed_to_discover_tests_from_gtest ``` --- Patch is 88.11 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/164040.diff 9 Files Affected: - (removed) clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h (-112) - (modified) clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt (-1) - (removed) clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp (-295) - (modified) clang/unittests/Analysis/FlowSensitive/CMakeLists.txt (-2) - (removed) clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTest.cpp (-34) - (removed) clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp (-2518) - (removed) clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.h (-157) - (modified) llvm/utils/gn/secondary/clang/lib/Analysis/FlowSensitive/Models/BUILD.gn (-1) - (modified) llvm/utils/gn/secondary/clang/unittests/Analysis/FlowSensitive/BUILD.gn (-2) ``````````diff diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h deleted file mode 100644 index cb619fb0cb5bb..0000000000000 --- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h +++ /dev/null @@ -1,112 +0,0 @@ -//===- UncheckedStatusOrAccessModel.h -------------------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDSTATUSORACCESSMODEL_H -#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDSTATUSORACCESSMODEL_H - -#include "clang/AST/Type.h" -#include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/Analysis/CFG.h" -#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h" -#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h" -#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" -#include "clang/Analysis/FlowSensitive/MatchSwitch.h" -#include "clang/Analysis/FlowSensitive/NoopLattice.h" -#include "clang/Analysis/FlowSensitive/StorageLocation.h" -#include "clang/Analysis/FlowSensitive/Value.h" -#include "clang/Basic/SourceLocation.h" -#include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/StringMap.h" -#include "llvm/ADT/StringRef.h" - -namespace clang::dataflow::statusor_model { - -// The helper functions exported here are for use of downstream vendor -// extensions of this model. - -// Match declaration of `absl::StatusOr<T>` and bind `T` to "T". -clang::ast_matchers::DeclarationMatcher statusOrClass(); -// Match declaration of `absl::Status`. -clang::ast_matchers::DeclarationMatcher statusClass(); -// Match declaration of `absl::internal_statusor::OperatorBase`. -clang::ast_matchers::DeclarationMatcher statusOrOperatorBaseClass(); -clang::ast_matchers::TypeMatcher statusOrType(); - -// Get RecordStorageLocation for the `Status` contained in the `StatusOr` -RecordStorageLocation &locForStatus(RecordStorageLocation &StatusOrLoc); -// Get the StorageLocation for the OK boolean in the `Status` -StorageLocation &locForOk(RecordStorageLocation &StatusLoc); -// Get the OK boolean in the `Status`, and initialize it if necessary. -BoolValue &valForOk(RecordStorageLocation &StatusLoc, Environment &Env); -// Get synthetic fields for the types modelled by -// `UncheckedStatusOrAccessModel`. -llvm::StringMap<QualType> getSyntheticFields(QualType Ty, QualType StatusType, - const CXXRecordDecl &RD); - -// Initialize the synthetic fields of the `StatusOr`. -// N.B. if it is already initialized, the value gets reset. -BoolValue &initializeStatusOr(RecordStorageLocation &StatusOrLoc, - Environment &Env); -// Initialize the synthetic fields of the `Status`. -// N.B. if it is already initialized, the value gets reset. -BoolValue &initializeStatus(RecordStorageLocation &StatusLoc, Environment &Env); - -bool isRecordTypeWithName(QualType Type, llvm::StringRef TypeName); -// Return true if `Type` is instantiation of `absl::StatusOr<T>` -bool isStatusOrType(QualType Type); -// Return true if `Type` is `absl::Status` -bool isStatusType(QualType Type); - -// Get `QualType` for `absl::Status`, or default-constructed -// QualType if it does not exist. -QualType findStatusType(const ASTContext &Ctx); - -struct UncheckedStatusOrAccessModelOptions {}; - -// Dataflow analysis that discovers unsafe uses of StatusOr values. -class UncheckedStatusOrAccessModel - : public DataflowAnalysis<UncheckedStatusOrAccessModel, NoopLattice> { -public: - explicit UncheckedStatusOrAccessModel(ASTContext &Ctx, Environment &Env); - - static Lattice initialElement() { return {}; } - - void transfer(const CFGElement &Elt, Lattice &L, Environment &Env); - -private: - CFGMatchSwitch<TransferState<Lattice>> TransferMatchSwitch; -}; - -using LatticeTransferState = - TransferState<UncheckedStatusOrAccessModel::Lattice>; - -// Extend the Builder with the transfer functions for -// `UncheckedStatusOrAccessModel`. This is useful to write downstream models -// that extend the model. -CFGMatchSwitch<LatticeTransferState> -buildTransferMatchSwitch(ASTContext &Ctx, - CFGMatchSwitchBuilder<LatticeTransferState> Builder); - -class UncheckedStatusOrAccessDiagnoser { -public: - explicit UncheckedStatusOrAccessDiagnoser( - UncheckedStatusOrAccessModelOptions Options = {}); - - llvm::SmallVector<SourceLocation> operator()( - const CFGElement &Elt, ASTContext &Ctx, - const TransferStateForDiagnostics<UncheckedStatusOrAccessModel::Lattice> - &State); - -private: - CFGMatchSwitch<const Environment, llvm::SmallVector<SourceLocation>> - DiagnoseMatchSwitch; -}; - -} // namespace clang::dataflow::statusor_model - -#endif // CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDSTATUSORACCESSMODEL_H diff --git a/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt b/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt index d1236f5714881..89bbe8791eb2c 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt +++ b/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt @@ -1,7 +1,6 @@ add_clang_library(clangAnalysisFlowSensitiveModels ChromiumCheckModel.cpp UncheckedOptionalAccessModel.cpp - UncheckedStatusOrAccessModel.cpp LINK_LIBS clangAnalysis diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp deleted file mode 100644 index 75b0959491c22..0000000000000 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ /dev/null @@ -1,295 +0,0 @@ -//===- UncheckedStatusOrAccessModel.cpp -----------------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h" - -#include <cassert> -#include <utility> - -#include "clang/AST/DeclCXX.h" -#include "clang/AST/DeclTemplate.h" -#include "clang/AST/Expr.h" -#include "clang/AST/ExprCXX.h" -#include "clang/AST/TypeBase.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/ASTMatchers/ASTMatchersInternal.h" -#include "clang/Analysis/CFG.h" -#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h" -#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h" -#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" -#include "clang/Analysis/FlowSensitive/MatchSwitch.h" -#include "clang/Analysis/FlowSensitive/StorageLocation.h" -#include "clang/Analysis/FlowSensitive/Value.h" -#include "clang/Basic/LLVM.h" -#include "clang/Basic/SourceLocation.h" -#include "llvm/ADT/StringMap.h" - -namespace clang::dataflow::statusor_model { -namespace { - -using ::clang::ast_matchers::MatchFinder; -using ::clang::ast_matchers::StatementMatcher; - -} // namespace - -static bool namespaceEquals(const NamespaceDecl *NS, - clang::ArrayRef<clang::StringRef> NamespaceNames) { - while (!NamespaceNames.empty() && NS) { - if (NS->getName() != NamespaceNames.consume_back()) - return false; - NS = dyn_cast_or_null<NamespaceDecl>(NS->getParent()); - } - return NamespaceNames.empty() && !NS; -} - -// TODO: move this to a proper place to share with the rest of clang -static bool isTypeNamed(QualType Type, clang::ArrayRef<clang::StringRef> NS, - StringRef Name) { - if (Type.isNull()) - return false; - if (auto *RD = Type->getAsRecordDecl()) - if (RD->getName() == Name) - if (const auto *N = dyn_cast_or_null<NamespaceDecl>(RD->getDeclContext())) - return namespaceEquals(N, NS); - return false; -} - -static bool isStatusOrOperatorBaseType(QualType Type) { - return isTypeNamed(Type, {"absl", "internal_statusor"}, "OperatorBase"); -} - -static bool isSafeUnwrap(RecordStorageLocation *StatusOrLoc, - const Environment &Env) { - if (!StatusOrLoc) - return false; - auto &StatusLoc = locForStatus(*StatusOrLoc); - auto *OkVal = Env.get<BoolValue>(locForOk(StatusLoc)); - return OkVal != nullptr && Env.proves(OkVal->formula()); -} - -static ClassTemplateSpecializationDecl * -getStatusOrBaseClass(const QualType &Ty) { - auto *RD = Ty->getAsCXXRecordDecl(); - if (RD == nullptr) - return nullptr; - if (isStatusOrType(Ty) || - // In case we are analyzing code under OperatorBase itself that uses - // operator* (e.g. to implement operator->). - isStatusOrOperatorBaseType(Ty)) - return cast<ClassTemplateSpecializationDecl>(RD); - if (!RD->hasDefinition()) - return nullptr; - for (const auto &Base : RD->bases()) - if (auto *QT = getStatusOrBaseClass(Base.getType())) - return QT; - return nullptr; -} - -static QualType getStatusOrValueType(ClassTemplateSpecializationDecl *TRD) { - return TRD->getTemplateArgs().get(0).getAsType(); -} - -static auto isStatusOrMemberCallWithName(llvm::StringRef member_name) { - using namespace ::clang::ast_matchers; // NOLINT: Too many names - return cxxMemberCallExpr( - on(expr(unless(cxxThisExpr()))), - callee(cxxMethodDecl( - hasName(member_name), - ofClass(anyOf(statusOrClass(), statusOrOperatorBaseClass()))))); -} - -static auto isStatusOrOperatorCallWithName(llvm::StringRef operator_name) { - using namespace ::clang::ast_matchers; // NOLINT: Too many names - return cxxOperatorCallExpr( - hasOverloadedOperatorName(operator_name), - callee(cxxMethodDecl( - ofClass(anyOf(statusOrClass(), statusOrOperatorBaseClass()))))); -} - -static auto valueCall() { - using namespace ::clang::ast_matchers; // NOLINT: Too many names - return anyOf(isStatusOrMemberCallWithName("value"), - isStatusOrMemberCallWithName("ValueOrDie")); -} - -static auto valueOperatorCall() { - using namespace ::clang::ast_matchers; // NOLINT: Too many names - return expr(anyOf(isStatusOrOperatorCallWithName("*"), - isStatusOrOperatorCallWithName("->"))); -} - -static auto -buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) { - return CFGMatchSwitchBuilder<const Environment, - llvm::SmallVector<SourceLocation>>() - // StatusOr::value, StatusOr::ValueOrDie - .CaseOfCFGStmt<CXXMemberCallExpr>( - valueCall(), - [](const CXXMemberCallExpr *E, - const ast_matchers::MatchFinder::MatchResult &, - const Environment &Env) { - if (!isSafeUnwrap(getImplicitObjectLocation(*E, Env), Env)) - return llvm::SmallVector<SourceLocation>({E->getExprLoc()}); - return llvm::SmallVector<SourceLocation>(); - }) - - // StatusOr::operator*, StatusOr::operator-> - .CaseOfCFGStmt<CXXOperatorCallExpr>( - valueOperatorCall(), - [](const CXXOperatorCallExpr *E, - const ast_matchers::MatchFinder::MatchResult &, - const Environment &Env) { - RecordStorageLocation *StatusOrLoc = - Env.get<RecordStorageLocation>(*E->getArg(0)); - if (!isSafeUnwrap(StatusOrLoc, Env)) - return llvm::SmallVector<SourceLocation>({E->getOperatorLoc()}); - return llvm::SmallVector<SourceLocation>(); - }) - .Build(); -} - -UncheckedStatusOrAccessDiagnoser::UncheckedStatusOrAccessDiagnoser( - UncheckedStatusOrAccessModelOptions Options) - : DiagnoseMatchSwitch(buildDiagnoseMatchSwitch(Options)) {} - -llvm::SmallVector<SourceLocation> UncheckedStatusOrAccessDiagnoser::operator()( - const CFGElement &Elt, ASTContext &Ctx, - const TransferStateForDiagnostics<UncheckedStatusOrAccessModel::Lattice> - &State) { - return DiagnoseMatchSwitch(Elt, Ctx, State.Env); -} - -BoolValue &initializeStatus(RecordStorageLocation &StatusLoc, - Environment &Env) { - auto &OkVal = Env.makeAtomicBoolValue(); - Env.setValue(locForOk(StatusLoc), OkVal); - return OkVal; -} - -BoolValue &initializeStatusOr(RecordStorageLocation &StatusOrLoc, - Environment &Env) { - return initializeStatus(locForStatus(StatusOrLoc), Env); -} - -clang::ast_matchers::DeclarationMatcher statusOrClass() { - using namespace ::clang::ast_matchers; // NOLINT: Too many names - return classTemplateSpecializationDecl( - hasName("absl::StatusOr"), - hasTemplateArgument(0, refersToType(type().bind("T")))); -} - -clang::ast_matchers::DeclarationMatcher statusClass() { - using namespace ::clang::ast_matchers; // NOLINT: Too many names - return cxxRecordDecl(hasName("absl::Status")); -} - -clang::ast_matchers::DeclarationMatcher statusOrOperatorBaseClass() { - using namespace ::clang::ast_matchers; // NOLINT: Too many names - return classTemplateSpecializationDecl( - hasName("absl::internal_statusor::OperatorBase")); -} - -clang::ast_matchers::TypeMatcher statusOrType() { - using namespace ::clang::ast_matchers; // NOLINT: Too many names - return hasCanonicalType(qualType(hasDeclaration(statusOrClass()))); -} - -bool isRecordTypeWithName(QualType Type, llvm::StringRef TypeName) { - return Type->isRecordType() && - Type->getAsCXXRecordDecl()->getQualifiedNameAsString() == TypeName; -} - -bool isStatusOrType(QualType Type) { - return isTypeNamed(Type, {"absl"}, "StatusOr"); -} - -bool isStatusType(QualType Type) { - return isTypeNamed(Type, {"absl"}, "Status"); -} - -llvm::StringMap<QualType> getSyntheticFields(QualType Ty, QualType StatusType, - const CXXRecordDecl &RD) { - if (auto *TRD = getStatusOrBaseClass(Ty)) - return {{"status", StatusType}, {"value", getStatusOrValueType(TRD)}}; - if (isStatusType(Ty) || (RD.hasDefinition() && - RD.isDerivedFrom(StatusType->getAsCXXRecordDecl()))) - return {{"ok", RD.getASTContext().BoolTy}}; - return {}; -} - -RecordStorageLocation &locForStatus(RecordStorageLocation &StatusOrLoc) { - return cast<RecordStorageLocation>(StatusOrLoc.getSyntheticField("status")); -} - -StorageLocation &locForOk(RecordStorageLocation &StatusLoc) { - return StatusLoc.getSyntheticField("ok"); -} - -BoolValue &valForOk(RecordStorageLocation &StatusLoc, Environment &Env) { - if (auto *Val = Env.get<BoolValue>(locForOk(StatusLoc))) - return *Val; - return initializeStatus(StatusLoc, Env); -} - -static void transferStatusOrOkCall(const CXXMemberCallExpr *Expr, - const MatchFinder::MatchResult &, - LatticeTransferState &State) { - RecordStorageLocation *StatusOrLoc = - getImplicitObjectLocation(*Expr, State.Env); - if (StatusOrLoc == nullptr) - return; - - auto &OkVal = valForOk(locForStatus(*StatusOrLoc), State.Env); - State.Env.setValue(*Expr, OkVal); -} - -CFGMatchSwitch<LatticeTransferState> -buildTransferMatchSwitch(ASTContext &Ctx, - CFGMatchSwitchBuilder<LatticeTransferState> Builder) { - using namespace ::clang::ast_matchers; // NOLINT: Too many names - return std::move(Builder) - .CaseOfCFGStmt<CXXMemberCallExpr>(isStatusOrMemberCallWithName("ok"), - transferStatusOrOkCall) - .Build(); -} - -QualType findStatusType(const ASTContext &Ctx) { - for (Type *Ty : Ctx.getTypes()) - if (isStatusType(QualType(Ty, 0))) - return QualType(Ty, 0); - - return QualType(); -} - -UncheckedStatusOrAccessModel::UncheckedStatusOrAccessModel(ASTContext &Ctx, - Environment &Env) - : DataflowAnalysis<UncheckedStatusOrAccessModel, - UncheckedStatusOrAccessModel::Lattice>(Ctx), - TransferMatchSwitch(buildTransferMatchSwitch(Ctx, {})) { - QualType StatusType = findStatusType(Ctx); - Env.getDataflowAnalysisContext().setSyntheticFieldCallback( - [StatusType](QualType Ty) -> llvm::StringMap<QualType> { - CXXRecordDecl *RD = Ty->getAsCXXRecordDecl(); - if (RD == nullptr) - return {}; - - if (auto Fields = getSyntheticFields(Ty, StatusType, *RD); - !Fields.empty()) - return Fields; - return {}; - }); -} - -void UncheckedStatusOrAccessModel::transfer(const CFGElement &Elt, Lattice &L, - Environment &Env) { - LatticeTransferState State(L, Env); - TransferMatchSwitch(Elt, getASTContext(), State); -} - -} // namespace clang::dataflow::statusor_model diff --git a/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt b/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt index 1d932ec6e8a96..35082387b46e9 100644 --- a/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt +++ b/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt @@ -25,8 +25,6 @@ add_clang_unittest(ClangAnalysisFlowSensitiveTests TransferTest.cpp TypeErasedDataflowAnalysisTest.cpp UncheckedOptionalAccessModelTest.cpp - UncheckedStatusOrAccessModelTest.cpp - UncheckedStatusOrAccessModelTestFixture.cpp ValueTest.cpp WatchedLiteralsSolverTest.cpp CLANG_LIBS diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTest.cpp deleted file mode 100644 index 07d3f2412e842..0000000000000 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTest.cpp +++ /dev/null @@ -1,34 +0,0 @@ -//===- UncheckedStatusOrAccessModelTest.cpp -------------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include <utility> - -#include "UncheckedStatusOrAccessModelTestFixture.h" -#include "clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h" -#include "gtest/gtest.h" - -namespace clang::dataflow::statusor_model { -namespace { - -INSTANTIATE_TEST_SUITE_P( - UncheckedStatusOrAccessModelTest, UncheckedStatusOrAccessModelTest, - testing::Values( - std::make_pair(new UncheckedStatusOrAccessModelTestExecutor< - UncheckedStatusOrAccessModel>(), - UncheckedStatusOrAccessModelTestAliasKind::kUnaliased), - std::make_pair( - new UncheckedStatusOrAccessModelTestExecutor< - UncheckedStatusOrAccessModel>(), - UncheckedStatusOrAccessModelTestAliasKind::kPartiallyAliased), - std::make_pair( - new UncheckedStatusOrAccessModelTestExecutor< - UncheckedStatusOrAccessModel>(), - UncheckedStatusOrAccessModelTestAliasKind::kFullyAliased))); -} // namespace - -} // namespace clang::dataflow::statusor_model diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp deleted file mode 100644 index 4827cc1d0a7e9..0000000000000 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ /dev/null @@ -1,2518 +0,0 @@ -//===- UncheckedStatusOrAccessModelTestFixture.cpp ------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "UncheckedStatusOrAccessModelTestFixture.h" -#include "MockHeaders.h" -#include "llvm/Support/ErrorHandling.h" - -#include <string> -#include <utility> -#include <vector> - -#include "gtest/gtest.h" - -namespace clang::dataflow::statusor_model { -namespace { - -TE... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/164040 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
