This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc2bb68078eb9: [dataflow] Disallow implicit copy of
Environment, use fork() instead (authored by sammccall).
Changed prior to commit:
https://reviews.llvm.org/D153674?vs=534138&id=534519#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153674/new/
https://reviews.llvm.org/D153674
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -363,7 +363,7 @@
if (It == StmtToAnnotations.end())
return;
auto [_, InsertSuccess] = AnnotationStates.insert(
- {It->second, StateT{State.Lattice, State.Env}});
+ {It->second, StateT{State.Lattice, State.Env.fork()}});
(void)_;
(void)InsertSuccess;
assert(InsertSuccess);
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -282,7 +282,7 @@
if (!MaybePredState)
continue;
- TypeErasedDataflowAnalysisState PredState = *MaybePredState;
+ TypeErasedDataflowAnalysisState PredState = MaybePredState->fork();
if (Analysis.builtinOptions()) {
if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
const StmtToEnvMap StmtToEnv(AC.CFCtx, AC.BlockStates);
@@ -309,7 +309,7 @@
// FIXME: Consider passing `Block` to `Analysis.typeErasedInitialElement()`
// to enable building analyses like computation of dominators that
// initialize the state of each basic block differently.
- MaybeState.emplace(Analysis.typeErasedInitialElement(), AC.InitEnv);
+ MaybeState.emplace(Analysis.typeErasedInitialElement(), AC.InitEnv.fork());
}
return std::move(*MaybeState);
}
@@ -447,12 +447,12 @@
ForwardDataflowWorklist Worklist(CFCtx.getCFG(), &POV);
std::vector<std::optional<TypeErasedDataflowAnalysisState>> BlockStates(
- CFCtx.getCFG().size(), std::nullopt);
+ CFCtx.getCFG().size());
// The entry basic block doesn't contain statements so it can be skipped.
const CFGBlock &Entry = CFCtx.getCFG().getEntry();
BlockStates[Entry.getBlockID()] = {Analysis.typeErasedInitialElement(),
- InitEnv};
+ InitEnv.fork()};
Worklist.enqueueSuccessors(&Entry);
AnalysisContext AC(CFCtx, Analysis, InitEnv, BlockStates);
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -865,7 +865,7 @@
assert(BlockToOutputState);
assert(ExitBlock < BlockToOutputState->size());
- auto ExitState = (*BlockToOutputState)[ExitBlock];
+ auto &ExitState = (*BlockToOutputState)[ExitBlock];
assert(ExitState);
Env.popCall(S, ExitState->Env);
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -265,19 +265,10 @@
: DACtx(&DACtx),
FlowConditionToken(&DACtx.arena().makeFlowConditionToken()) {}
-Environment::Environment(const Environment &Other)
- : DACtx(Other.DACtx), CallStack(Other.CallStack),
- ReturnVal(Other.ReturnVal), ReturnLoc(Other.ReturnLoc),
- ThisPointeeLoc(Other.ThisPointeeLoc), DeclToLoc(Other.DeclToLoc),
- ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
- MemberLocToStruct(Other.MemberLocToStruct),
- FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) {
-}
-
-Environment &Environment::operator=(const Environment &Other) {
- Environment Copy(Other);
- *this = std::move(Copy);
- return *this;
+Environment Environment::fork() const {
+ Environment Copy(*this);
+ Copy.FlowConditionToken = &DACtx->forkFlowCondition(*FlowConditionToken);
+ return Copy;
}
Environment::Environment(DataflowAnalysisContext &DACtx,
Index: clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
+++ clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
@@ -126,6 +126,10 @@
TypeErasedDataflowAnalysisState(TypeErasedLattice Lattice, Environment Env)
: Lattice(std::move(Lattice)), Env(std::move(Env)) {}
+
+ TypeErasedDataflowAnalysisState fork() const {
+ return TypeErasedDataflowAnalysisState(Lattice, Env.fork());
+ }
};
/// Transfers the state of a basic block by evaluating each of its elements in
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -160,8 +160,8 @@
/// the state of a program.
explicit Environment(DataflowAnalysisContext &DACtx);
- Environment(const Environment &Other);
- Environment &operator=(const Environment &Other);
+ // Copy-constructor is private, Environments should not be copied. See fork().
+ Environment &operator=(const Environment &Other) = delete;
Environment(Environment &&Other) = default;
Environment &operator=(Environment &&Other) = default;
@@ -176,6 +176,16 @@
/// with a symbolic representation of the `this` pointee.
Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx);
+ /// Returns a new environment that is a copy of this one.
+ ///
+ /// The state of the program is initially the same, but can be mutated without
+ /// affecting the original.
+ ///
+ /// However the original should not be further mutated, as this may interfere
+ /// with the fork. (In practice, values are stored independently, but the
+ /// forked flow condition references the original).
+ Environment fork() const;
+
/// Creates and returns an environment to use for an inline analysis of the
/// callee. Uses the storage location from each argument in the `Call` as the
/// storage location for the corresponding parameter in the callee.
@@ -540,6 +550,9 @@
LLVM_DUMP_METHOD void dump(raw_ostream &OS) const;
private:
+ // The copy-constructor is for use in fork() only.
+ Environment(const Environment &) = default;
+
/// Creates a value appropriate for `Type`, if `Type` is supported, otherwise
/// return null.
///
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -205,8 +205,10 @@
const TypeErasedDataflowAnalysisState &State) {
auto *Lattice =
llvm::any_cast<typename AnalysisT::Lattice>(&State.Lattice.Value);
+ // FIXME: we should not be copying the environment here!
+ // Ultimately the PostVisitCFG only gets a const reference anyway.
PostVisitCFG(Element, DataflowAnalysisState<typename AnalysisT::Lattice>{
- *Lattice, State.Env});
+ *Lattice, State.Env.fork()});
};
}
@@ -222,12 +224,13 @@
llvm::transform(
std::move(*TypeErasedBlockStates), std::back_inserter(BlockStates),
[](auto &OptState) {
- return llvm::transformOptional(std::move(OptState), [](auto &&State) {
- return DataflowAnalysisState<typename AnalysisT::Lattice>{
- llvm::any_cast<typename AnalysisT::Lattice>(
- std::move(State.Lattice.Value)),
- std::move(State.Env)};
- });
+ return llvm::transformOptional(
+ std::move(OptState), [](TypeErasedDataflowAnalysisState &&State) {
+ return DataflowAnalysisState<typename AnalysisT::Lattice>{
+ llvm::any_cast<typename AnalysisT::Lattice>(
+ std::move(State.Lattice.Value)),
+ std::move(State.Env)};
+ });
});
return BlockStates;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
