https://github.com/martinboehme created
https://github.com/llvm/llvm-project/pull/77895
Reverts llvm/llvm-project#77750
>From 44fbb43480c8779e8164a21dacb4d524d69b0e07 Mon Sep 17 00:00:00 2001
From: martinboehme <mboe...@google.com>
Date: Fri, 12 Jan 2024 09:53:53 +0100
Subject: [PATCH] Revert "[clang][dataflow] Process terminator condition within
`transferCFGBlock()`."
---
.../TypeErasedDataflowAnalysis.cpp | 42 ++++++-------------
.../Analysis/FlowSensitive/LoggerTest.cpp | 1 -
.../Analysis/FlowSensitive/TransferTest.cpp | 31 --------------
3 files changed, 12 insertions(+), 62 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 24d1447f09794a..e0a3552a9cde17 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -126,12 +126,19 @@ class TerminatorVisitor
private:
TerminatorVisitorRetTy extendFlowCondition(const Expr &Cond) {
+ // The terminator sub-expression might not be evaluated.
+ if (Env.getValue(Cond) == nullptr)
+ transfer(StmtToEnv, Cond, Env);
+
auto *Val = Env.get<BoolValue>(Cond);
- // In transferCFGBlock(), we ensure that we always have a `Value` for the
- // terminator condition, so assert this.
- // We consciously assert ourselves instead of asserting via `cast()` so
- // that we get a more meaningful line number if the assertion fails.
- assert(Val != nullptr);
+ // Value merging depends on flow conditions from different environments
+ // being mutually exclusive -- that is, they cannot both be true in their
+ // entirety (even if they may share some clauses). So, we need *some* value
+ // for the condition expression, even if just an atom.
+ if (Val == nullptr) {
+ Val = &Env.makeAtomicBoolValue();
+ Env.setValue(Cond, *Val);
+ }
bool ConditionValue = true;
// The condition must be inverted for the successor that encompasses the
@@ -481,31 +488,6 @@ transferCFGBlock(const CFGBlock &Block, AnalysisContext
&AC,
}
AC.Log.recordState(State);
}
-
- // If we have a terminator, evaluate its condition.
- // This `Expr` may not appear as a `CFGElement` anywhere else, and it's
- // important that we evaluate it here (rather than while processing the
- // terminator) so that we put the corresponding value in the right
- // environment.
- if (const Expr *TerminatorCond =
- dyn_cast_or_null<Expr>(Block.getTerminatorCondition())) {
- if (State.Env.getValue(*TerminatorCond) == nullptr)
- // FIXME: This only runs the builtin transfer, not the analysis-specific
- // transfer. Fixing this isn't trivial, as the analysis-specific transfer
- // takes a `CFGElement` as input, but some expressions only show up as a
- // terminator condition, but not as a `CFGElement`. The condition of an
if
- // statement is one such example.
- transfer(StmtToEnvMap(AC.CFCtx, AC.BlockStates), *TerminatorCond,
- State.Env);
-
- // If the transfer function didn't produce a value, create an atom so that
- // we have *some* value for the condition expression. This ensures that
- // when we extend the flow condition, it actually changes.
- if (State.Env.getValue(*TerminatorCond) == nullptr)
- State.Env.setValue(*TerminatorCond, State.Env.makeAtomicBoolValue());
- AC.Log.recordState(State);
- }
-
return State;
}
diff --git a/clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
b/clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
index c5594aa3fe9d1f..a60dbe1f61f6d6 100644
--- a/clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
@@ -123,7 +123,6 @@ recordState(Elements=1, Branches=0, Joins=0)
enterElement(b (ImplicitCastExpr, LValueToRValue, _Bool))
transfer()
recordState(Elements=2, Branches=0, Joins=0)
-recordState(Elements=2, Branches=0, Joins=0)
enterBlock(3, false)
transferBranch(0)
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 6d88e25f77c807..056c4f3383d832 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -6408,35 +6408,4 @@ TEST(TransferTest, DifferentReferenceLocInJoin) {
});
}
-// This test verifies correct modeling of a relational dependency that goes
-// through unmodeled functions (the simple `cond()` in this case).
-TEST(TransferTest, ConditionalRelation) {
- std::string Code = R"(
- bool cond();
- void target() {
- bool a = true;
- bool b = true;
- if (cond()) {
- a = false;
- if (cond()) {
- b = false;
- }
- }
- (void)0;
- // [[p]]
- }
- )";
- runDataflow(
- Code,
- [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
- ASTContext &ASTCtx) {
- const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
- auto &A = Env.arena();
- auto &VarA = getValueForDecl<BoolValue>(ASTCtx, Env, "a").formula();
- auto &VarB = getValueForDecl<BoolValue>(ASTCtx, Env, "b").formula();
-
- EXPECT_FALSE(Env.allows(A.makeAnd(VarA, A.makeNot(VarB))));
- });
-}
-
} // namespace
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
