samestep updated this revision to Diff 448438.
samestep added a comment.
Address some of Yitzie's comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130726/new/
https://reviews.llvm.org/D130726
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -3960,4 +3960,45 @@
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
}
+TEST(TransferTest, ContextSensitiveSetBothTrueAndFalse) {
+ std::string Code = R"(
+ bool GiveBool();
+ void SetBool(bool &Var, bool Val) { Var = Val; }
+
+ void target() {
+ bool Foo = GiveBool();
+ bool Bar = GiveBool();
+ SetBool(Foo, true);
+ SetBool(Bar, false);
+ // [[p]]
+ }
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ const Environment &Env = Results[0].second.Env;
+
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
+
+ const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ ASSERT_THAT(BarDecl, NotNull());
+
+ auto &FooVal =
+ *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
+ EXPECT_TRUE(Env.flowConditionImplies(FooVal));
+ EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
+
+ auto &BarVal =
+ *cast<BoolValue>(Env.getValue(*BarDecl, SkipPast::None));
+ EXPECT_FALSE(Env.flowConditionImplies(BarVal));
+ EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BarVal)));
+ },
+ {/*.ApplyBuiltinTransfer=*/true,
+ /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
} // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -534,7 +534,7 @@
auto ExitState = (*BlockToOutputState)[ExitBlock];
assert(ExitState);
- Env = ExitState->Env;
+ Env.popCall(ExitState->Env);
}
}
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -203,14 +203,6 @@
Environment Environment::pushCall(const CallExpr *Call) const {
Environment Env(*this);
- // FIXME: Currently this only works if the callee is never a method and the
- // same callee is never analyzed from multiple separate callsites. To
- // generalize this, we'll need to store a "context" field (probably a stack of
- // `const CallExpr *`s) in the `Environment`, and then change the
- // `DataflowAnalysisContext` class to hold a map from contexts to "frames",
- // where each frame stores its own version of what are currently the
- // `DeclToLoc`, `ExprToLoc`, and `ThisPointeeLoc` fields.
-
const auto *FuncDecl = Call->getDirectCallee();
assert(FuncDecl != nullptr);
const auto *Body = FuncDecl->getBody();
@@ -228,16 +220,40 @@
for (; ArgIt != ArgEnd; ++ParamIt, ++ArgIt) {
assert(ParamIt != ParamEnd);
- const VarDecl *Param = *ParamIt;
const Expr *Arg = *ArgIt;
auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
assert(ArgLoc != nullptr);
- Env.setStorageLocation(*Param, *ArgLoc);
+
+ const VarDecl *Param = *ParamIt;
+ auto &Loc = Env.createStorageLocation(*Param);
+ Env.setStorageLocation(*Param, Loc);
+
+ QualType ParamType = Param->getType();
+ if (ParamType->isReferenceType()) {
+ auto &Val = Env.takeOwnership(std::make_unique<ReferenceValue>(*ArgLoc));
+ Env.setValue(Loc, Val);
+ } else if (auto *ArgVal = Env.getValue(*ArgLoc)) {
+ Env.setValue(Loc, *ArgVal);
+ } else if (Value *Val = Env.createValue(ParamType)) {
+ Env.setValue(Loc, *Val);
+ }
}
return Env;
}
+void Environment::popCall(const Environment &CalleeEnv) {
+ // We ignore `DACtx` because it's already the same in both. We don't bring
+ // back `DeclToLoc` and `ExprToLoc` because we want to be able to later
+ // analyze the same callee in a different context, and `setStorageLocation`
+ // requires there to not already be a storage location assigned. Conceptually,
+ // these maps capture information from the local scope, so when popping that
+ // scope, we do not propagate the maps.
+ this->LocToVal = std::move(CalleeEnv.LocToVal);
+ this->MemberLocToStruct = std::move(CalleeEnv.MemberLocToStruct);
+ this->FlowConditionToken = std::move(CalleeEnv.FlowConditionToken);
+}
+
bool Environment::equivalentTo(const Environment &Other,
Environment::ValueModel &Model) const {
assert(DACtx == Other.DACtx);
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -143,6 +143,10 @@
/// Each argument of `Call` must already have a `StorageLocation`.
Environment pushCall(const CallExpr *Call) const;
+ /// Moves gathered information back into `this` from a `CalleeEnv` created via
+ /// `pushCall`.
+ void popCall(const Environment &CalleeEnv);
+
/// Returns true if and only if the environment is equivalent to `Other`, i.e
/// the two environments:
/// - have the same mappings from declarations to storage locations,
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
