[clang] 8361877 - Revert "[clang][dataflow] Use NoopLattice in optional model"
Sam Estep via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 29 12:34:54 PDT 2022
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
More information about the cfe-commits
mailing list