Author: Sam Estep Date: 2022-06-29T19:34:30Z New Revision: 8361877b10b8180ab12300b289da54a403ce7554
URL: https://github.com/llvm/llvm-project/commit/8361877b10b8180ab12300b289da54a403ce7554 DIFF: https://github.com/llvm/llvm-project/commit/8361877b10b8180ab12300b289da54a403ce7554.diff LOG: Revert "[clang][dataflow] Use NoopLattice in optional model" This reverts commit 335c05f5d19fecd5c0972ac801e58248d772a78e. Added: Modified: clang/docs/tools/clang-formatted-files.txt clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h Removed: clang/include/clang/Analysis/FlowSensitive/NoopLattice.h ################################################################################ diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt index 40d95411c3a0a..ed8b5e7e7ae7c 100644 --- a/clang/docs/tools/clang-formatted-files.txt +++ b/clang/docs/tools/clang-formatted-files.txt @@ -131,7 +131,6 @@ clang/include/clang/Analysis/FlowSensitive/DataflowLattice.h clang/include/clang/Analysis/FlowSensitive/DataflowWorklist.h clang/include/clang/Analysis/FlowSensitive/MapLattice.h clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h -clang/include/clang/Analysis/FlowSensitive/NoopLattice.h clang/include/clang/Analysis/FlowSensitive/Solver.h clang/include/clang/Analysis/FlowSensitive/SourceLocationsLattice.h clang/include/clang/Analysis/FlowSensitive/StorageLocation.h diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h index 25054deaf8afc..701db6470ac2f 100644 --- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h +++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h @@ -19,7 +19,7 @@ #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/SourceLocationsLattice.h" #include "clang/Basic/SourceLocation.h" #include <vector> @@ -38,11 +38,15 @@ struct UncheckedOptionalAccessModelOptions { bool IgnoreSmartPointerDereference = false; }; -/// Dataflow analysis that models whether optionals hold values or not. +/// Dataflow analysis that discovers unsafe accesses of optional values and +/// adds the respective source locations to the lattice. /// /// Models the `std::optional`, `absl::optional`, and `base::Optional` types. +/// +/// FIXME: Consider separating the models from the unchecked access analysis. class UncheckedOptionalAccessModel - : public DataflowAnalysis<UncheckedOptionalAccessModel, NoopLattice> { + : public DataflowAnalysis<UncheckedOptionalAccessModel, + SourceLocationsLattice> { public: UncheckedOptionalAccessModel( ASTContext &AstContext, UncheckedOptionalAccessModelOptions Options = {}); @@ -50,9 +54,12 @@ class UncheckedOptionalAccessModel /// Returns a matcher for the optional classes covered by this model. static ast_matchers::DeclarationMatcher optionalClassDecl(); - static NoopLattice initialElement() { return {}; } + static SourceLocationsLattice initialElement() { + return SourceLocationsLattice(); + } - void transfer(const Stmt *Stmt, NoopLattice &State, Environment &Env); + void transfer(const Stmt *Stmt, SourceLocationsLattice &State, + Environment &Env); bool compareEquivalent(QualType Type, const Value &Val1, const Environment &Env1, const Value &Val2, @@ -63,7 +70,7 @@ class UncheckedOptionalAccessModel Environment &MergedEnv) override; private: - MatchSwitch<TransferState<NoopLattice>> TransferMatchSwitch; + MatchSwitch<TransferState<SourceLocationsLattice>> TransferMatchSwitch; }; class UncheckedOptionalAccessDiagnoser { diff --git a/clang/include/clang/Analysis/FlowSensitive/NoopLattice.h b/clang/include/clang/Analysis/FlowSensitive/NoopLattice.h deleted file mode 100644 index 0192193281119..0000000000000 --- a/clang/include/clang/Analysis/FlowSensitive/NoopLattice.h +++ /dev/null @@ -1,41 +0,0 @@ -//===-- NoopLattice.h -------------------------------------------*- C++ -*-===// -// -// 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 -// -//===----------------------------------------------------------------------===// -// -// This file defines the lattice with exactly one element. -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOP_LATTICE_H -#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOP_LATTICE_H - -#include "clang/Analysis/FlowSensitive/DataflowLattice.h" -#include <ostream> - -namespace clang { -namespace dataflow { - -/// Trivial lattice for dataflow analysis with exactly one element. -/// -/// Useful for analyses that only need the Environment and nothing more. -class NoopLattice { -public: - bool operator==(const NoopLattice &Other) const { return true; } - - LatticeJoinEffect join(const NoopLattice &Other) { - return LatticeJoinEffect::Unchanged; - } -}; - -inline std::ostream &operator<<(std::ostream &OS, const NoopLattice &) { - return OS << "noop"; -} - -} // namespace dataflow -} // namespace clang - -#endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOP_LATTICE_H diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index eef3cc813a4ac..7d87d812dd575 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -20,7 +20,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/MatchSwitch.h" -#include "clang/Analysis/FlowSensitive/NoopLattice.h" +#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h" #include "clang/Analysis/FlowSensitive/Value.h" #include "clang/Basic/SourceLocation.h" #include "llvm/ADT/StringRef.h" @@ -35,7 +35,7 @@ namespace dataflow { namespace { using namespace ::clang::ast_matchers; -using LatticeTransferState = TransferState<NoopLattice>; +using LatticeTransferState = TransferState<SourceLocationsLattice>; DeclarationMatcher optionalClass() { return classTemplateSpecializationDecl( @@ -312,7 +312,18 @@ void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr, if (auto *Loc = maybeInitializeOptionalValueMember( UnwrapExpr->getType(), *OptionalVal, State.Env)) State.Env.setStorageLocation(*UnwrapExpr, *Loc); + + auto *Prop = OptionalVal->getProperty("has_value"); + if (auto *HasValueVal = cast_or_null<BoolValue>(Prop)) { + if (State.Env.flowConditionImplies(*HasValueVal)) + return; + } } + + // Record that this unwrap is *not* provably safe. + // FIXME: include either the name of the optional (if applicable) or a source + // range of the access for easier interpretation of the result. + State.Lattice.getSourceLocations().insert(ObjectExpr->getBeginLoc()); } void transferMakeOptionalCall(const CallExpr *E, @@ -705,10 +716,12 @@ UncheckedOptionalAccessModel::optionalClassDecl() { UncheckedOptionalAccessModel::UncheckedOptionalAccessModel( ASTContext &Ctx, UncheckedOptionalAccessModelOptions Options) - : DataflowAnalysis<UncheckedOptionalAccessModel, NoopLattice>(Ctx), + : DataflowAnalysis<UncheckedOptionalAccessModel, SourceLocationsLattice>( + Ctx), TransferMatchSwitch(buildTransferMatchSwitch(Options)) {} -void UncheckedOptionalAccessModel::transfer(const Stmt *S, NoopLattice &L, +void UncheckedOptionalAccessModel::transfer(const Stmt *S, + SourceLocationsLattice &L, Environment &Env) { LatticeTransferState State(L, Env); TransferMatchSwitch(*S, getASTContext(), State); diff --git a/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h b/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h index 45ed414406372..eab5782095bbc 100644 --- a/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h +++ b/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h @@ -18,11 +18,25 @@ #include "clang/AST/Stmt.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" -#include "clang/Analysis/FlowSensitive/NoopLattice.h" +#include "clang/Analysis/FlowSensitive/DataflowLattice.h" +#include <ostream> namespace clang { namespace dataflow { +class NoopLattice { +public: + bool operator==(const NoopLattice &) const { return true; } + + LatticeJoinEffect join(const NoopLattice &) { + return LatticeJoinEffect::Unchanged; + } +}; + +inline std::ostream &operator<<(std::ostream &OS, const NoopLattice &) { + return OS << "noop"; +} + class NoopAnalysis : public DataflowAnalysis<NoopAnalysis, NoopLattice> { public: /// `ApplyBuiltinTransfer` controls whether to run the built-in transfer _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits