samestep updated this revision to Diff 447906.
samestep added a comment.
Address Dmitri's review
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130600/new/
https://reviews.llvm.org/D130600
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
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
@@ -4001,4 +4001,60 @@
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
}
+TEST(TransferTest, ContextSensitiveReturnTrue) {
+ std::string Code = R"(
+ bool GiveBool() { return true; }
+
+ void target() {
+ bool Foo = GiveBool();
+ // [[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());
+
+ auto &FooVal =
+ *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
+ EXPECT_TRUE(Env.flowConditionImplies(FooVal));
+ },
+ {/*.ApplyBuiltinTransfer=*/true,
+ /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
+TEST(TransferTest, ContextSensitiveReturnFalse) {
+ std::string Code = R"(
+ bool GiveBool() { return false; }
+
+ void target() {
+ bool Foo = GiveBool();
+ // [[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());
+
+ auto &FooVal =
+ *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
+ EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(FooVal)));
+ },
+ {/*.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
@@ -332,6 +332,22 @@
std::make_unique<PointerValue>(*ThisPointeeLoc)));
}
+ void VisitReturnStmt(const ReturnStmt *S) {
+ auto *Loc = Env.getReturnStorageLocation();
+ if (Loc == nullptr) {
+ // The outermost context does not set a storage location for `return`, so
+ // in that case we just ignore `return` statements.
+ return;
+ }
+
+ auto *Ret = S->getRetValue();
+ assert(Ret != nullptr);
+ auto *Val = Env.getValue(*Ret, SkipPast::Reference);
+ assert(Val != nullptr);
+ // FIXME: Model NRVO.
+ Env.setValue(*Loc, *Val);
+ }
+
void VisitMemberExpr(const MemberExpr *S) {
ValueDecl *Member = S->getMemberDecl();
assert(Member != nullptr);
@@ -534,7 +550,11 @@
auto ExitState = (*BlockToOutputState)[ExitBlock];
assert(ExitState);
- Env.popCall(ExitState->Env);
+ auto &ExitEnv = ExitState->Env;
+ auto *ReturnLoc = ExitEnv.getReturnStorageLocation();
+ assert(ReturnLoc != nullptr);
+ Env.setStorageLocation(*S, *ReturnLoc);
+ Env.popCall(ExitEnv);
}
}
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -204,6 +204,16 @@
Environment Env(*this);
Env.CallStr.push_back(Call);
+ const auto *FuncDecl = Call->getDirectCallee();
+ assert(FuncDecl != nullptr);
+
+ auto *ReturnLoc = DACtx->getFrame(Env.CallStr).getReturnStorageLocation();
+ if (ReturnLoc == nullptr) {
+ QualType ReturnType = FuncDecl->getReturnType();
+ DACtx->getFrame(Env.CallStr)
+ .setReturnStorageLocation(Env.createStorageLocation(ReturnType));
+ }
+
// 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
@@ -212,8 +222,6 @@
// 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();
assert(Body != nullptr);
// FIXME: In order to allow the callee to reference globals, we probably need
@@ -380,6 +388,10 @@
return DACtx->getFrame(CallStr).getThisPointeeStorageLocation();
}
+StorageLocation *Environment::getReturnStorageLocation() const {
+ return DACtx->getFrame(CallStr).getReturnStorageLocation();
+}
+
PointerValue &Environment::getOrCreateNullPointerValue(QualType PointeeType) {
return DACtx->getOrCreateNullPointerValue(PointeeType);
}
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -221,6 +221,10 @@
/// in the environment.
StorageLocation *getThisPointeeStorageLocation() const;
+ /// Returns the storage location of the return value or null if it was not set
+ /// yet.
+ StorageLocation *getReturnStorageLocation() const;
+
/// Returns a pointer value that represents a null pointer. Calls with
/// `PointeeType` that are canonically equivalent will return the same result.
PointerValue &getOrCreateNullPointerValue(QualType PointeeType);
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -107,6 +107,20 @@
return ThisPointeeLoc;
}
+ /// Assigns `Loc` as the storage location of the return value.
+ ///
+ /// Requirements:
+ ///
+ /// `return` must not be assigned a storage location.
+ void setReturnStorageLocation(StorageLocation &Loc) {
+ assert(ReturnLoc == nullptr);
+ ReturnLoc = &Loc;
+ }
+
+ /// Returns the storage location of the returned value or null if it was not
+ /// set yet.
+ StorageLocation *getReturnStorageLocation() const { return ReturnLoc; }
+
private:
// Maps from program declarations and statements to storage locations that are
// assigned to them. These assignments are global (aggregated across all basic
@@ -117,6 +131,7 @@
llvm::DenseMap<const Expr *, StorageLocation *> ExprToLoc;
StorageLocation *ThisPointeeLoc = nullptr;
+ StorageLocation *ReturnLoc = nullptr;
};
/// Owns objects that encompass the state of a program and stores context that
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
