[clang] 0eaecbb - [clang][dataflow] Handle return statements
Sam Estep via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 4 10:42:24 PDT 2022
Author: Sam Estep
Date: 2022-08-04T17:42:19Z
New Revision: 0eaecbbc231883b43d3ac761b276d9f505c89c27
URL: https://github.com/llvm/llvm-project/commit/0eaecbbc231883b43d3ac761b276d9f505c89c27
DIFF: https://github.com/llvm/llvm-project/commit/0eaecbbc231883b43d3ac761b276d9f505c89c27.diff
LOG: [clang][dataflow] Handle return statements
This patch adds a `ReturnLoc` field to the `Environment`, serving a similar to the `ThisPointeeLoc` field in the `DataflowAnalysisContext`. It then uses that (along with a new `VisitReturnStmt` method in `TransferVisitor`) to handle non-`void`-returning functions in context-sensitive analysis.
Reviewed By: ymandel, sgatev
Differential Revision: https://reviews.llvm.org/D130600
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index ab60d313f242c..c83d0cbbbdbb3 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -322,6 +322,7 @@ class DataflowAnalysisContext {
llvm::DenseMap<const ValueDecl *, StorageLocation *> DeclToLoc;
llvm::DenseMap<const Expr *, StorageLocation *> ExprToLoc;
+ // FIXME: Move this into `Environment`.
StorageLocation *ThisPointeeLoc = nullptr;
// Null pointer values, keyed by the canonical pointee type.
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 1ecac86125a7b..e8a1c2db53b5c 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -222,6 +222,9 @@ class Environment {
/// in the environment.
StorageLocation *getThisPointeeStorageLocation() const;
+ /// Returns the storage location of the return value or null, if unset.
+ 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);
@@ -374,6 +377,11 @@ class Environment {
// `DACtx` is not null and not owned by this object.
DataflowAnalysisContext *DACtx;
+ // In a properly initialized `Environment`, `ReturnLoc` should only be null if
+ // its `DeclContext` could not be cast to a `FunctionDecl`.
+ StorageLocation *ReturnLoc = nullptr;
+ // FIXME: Move `ThisPointeeLoc` here from `DataflowAnalysisContext`.
+
// Maps from program declarations and statements to storage locations that are
// assigned to them. Unlike the maps in `DataflowAnalysisContext`, these
// include only storage locations that are in scope for a particular basic
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 4e5b2adee5c99..f4dadbb79b5c9 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -154,9 +154,9 @@ Environment::Environment(DataflowAnalysisContext &DACtx)
: DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
Environment::Environment(const Environment &Other)
- : DACtx(Other.DACtx), DeclToLoc(Other.DeclToLoc),
- ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
- MemberLocToStruct(Other.MemberLocToStruct),
+ : DACtx(Other.DACtx), ReturnLoc(Other.ReturnLoc),
+ DeclToLoc(Other.DeclToLoc), ExprToLoc(Other.ExprToLoc),
+ LocToVal(Other.LocToVal), MemberLocToStruct(Other.MemberLocToStruct),
FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) {
}
@@ -179,6 +179,9 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
if (Value *ParamVal = createValue(ParamDecl->getType()))
setValue(ParamLoc, *ParamVal);
}
+
+ QualType ReturnType = FuncDecl->getReturnType();
+ ReturnLoc = &createStorageLocation(ReturnType);
}
if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(&DeclCtx)) {
@@ -202,10 +205,11 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
Environment Environment::pushCall(const CallExpr *Call) const {
Environment Env(*this);
+ // FIXME: Support references here.
+ Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference);
const auto *FuncDecl = Call->getDirectCallee();
assert(FuncDecl != nullptr);
- assert(FuncDecl->getBody() != nullptr);
// FIXME: In order to allow the callee to reference globals, we probably need
// to call `initGlobalVars` here in some way.
@@ -241,12 +245,13 @@ Environment Environment::pushCall(const CallExpr *Call) const {
}
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
diff erent 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.
+ // We ignore `DACtx` because it's already the same in both. We don't want the
+ // callee's `ReturnLoc`. We don't bring back `DeclToLoc` and `ExprToLoc`
+ // because we want to be able to later analyze the same callee in a
diff erent
+ // 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);
@@ -256,6 +261,9 @@ bool Environment::equivalentTo(const Environment &Other,
Environment::ValueModel &Model) const {
assert(DACtx == Other.DACtx);
+ if (ReturnLoc != Other.ReturnLoc)
+ return false;
+
if (DeclToLoc != Other.DeclToLoc)
return false;
@@ -285,11 +293,14 @@ bool Environment::equivalentTo(const Environment &Other,
LatticeJoinEffect Environment::join(const Environment &Other,
Environment::ValueModel &Model) {
assert(DACtx == Other.DACtx);
+ assert(ReturnLoc == Other.ReturnLoc);
auto Effect = LatticeJoinEffect::Unchanged;
Environment JoinedEnv(*DACtx);
+ JoinedEnv.ReturnLoc = ReturnLoc;
+
JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
Effect = LatticeJoinEffect::Changed;
@@ -382,6 +393,10 @@ StorageLocation *Environment::getThisPointeeStorageLocation() const {
return DACtx->getThisPointeeStorageLocation();
}
+StorageLocation *Environment::getReturnStorageLocation() const {
+ return ReturnLoc;
+}
+
PointerValue &Environment::getOrCreateNullPointerValue(QualType PointeeType) {
return DACtx->getOrCreateNullPointerValue(PointeeType);
}
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 8b2e79826e814..430dc8d5faa97 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -332,6 +332,25 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
std::make_unique<PointerValue>(*ThisPointeeLoc)));
}
+ void VisitReturnStmt(const ReturnStmt *S) {
+ auto *Ret = S->getRetValue();
+ if (Ret == nullptr)
+ return;
+
+ auto *Val = Env.getValue(*Ret, SkipPast::None);
+ if (Val == nullptr)
+ return;
+
+ // FIXME: Support reference-type returns.
+ if (Val->getKind() == Value::Kind::Reference)
+ return;
+
+ auto *Loc = Env.getReturnStorageLocation();
+ assert(Loc != nullptr);
+ // FIXME: Model NRVO.
+ Env.setValue(*Loc, *Val);
+ }
+
void VisitMemberExpr(const MemberExpr *S) {
ValueDecl *Member = S->getMemberDecl();
assert(Member != nullptr);
@@ -521,6 +540,11 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
+ // Note that it is important for the storage location of `S` to be set
+ // before `pushCall`, because the latter uses it to set the storage
+ // location for `return`.
+ auto &ReturnLoc = Env.createStorageLocation(*S);
+ Env.setStorageLocation(*S, ReturnLoc);
auto CalleeEnv = Env.pushCall(S);
// FIXME: Use the same analysis as the caller for the callee. Note,
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index d6171763f4ecf..1d114d9df1759 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -4121,4 +4121,112 @@ TEST(TransferTest, ContextSensitiveSetMultipleBlocks) {
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
}
+TEST(TransferTest, ContextSensitiveReturnVoid) {
+ std::string Code = R"(
+ void Noop() { return; }
+
+ void target() {
+ Noop();
+ // [[p]]
+ }
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ // This just tests that the analysis doesn't crash.
+ },
+ {/*.ApplyBuiltinTransfer=*/true,
+ /*.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}});
+}
+
+TEST(TransferTest, ContextSensitiveReturnArg) {
+ std::string Code = R"(
+ bool GiveBool();
+ bool GiveBack(bool Arg) { return Arg; }
+
+ void target() {
+ bool Foo = GiveBool();
+ bool Bar = GiveBack(Foo);
+ bool Baz = Foo == Bar;
+ // [[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 *BazDecl = findValueDecl(ASTCtx, "Baz");
+ ASSERT_THAT(BazDecl, NotNull());
+
+ auto &BazVal =
+ *cast<BoolValue>(Env.getValue(*BazDecl, SkipPast::None));
+ EXPECT_TRUE(Env.flowConditionImplies(BazVal));
+ },
+ {/*.ApplyBuiltinTransfer=*/true,
+ /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
} // namespace
More information about the cfe-commits
mailing list