[clang] 8611a77 - [clang][dataflow] Analyze method bodies
Sam Estep via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 4 10:45:52 PDT 2022
Author: Sam Estep
Date: 2022-08-04T17:45:47Z
New Revision: 8611a77ee7ee342f5925cba2fa6df023596af9d9
URL: https://github.com/llvm/llvm-project/commit/8611a77ee7ee342f5925cba2fa6df023596af9d9
DIFF: https://github.com/llvm/llvm-project/commit/8611a77ee7ee342f5925cba2fa6df023596af9d9.diff
LOG: [clang][dataflow] Analyze method bodies
This patch adds the ability to context-sensitively analyze method bodies, by moving `ThisPointeeLoc` from `DataflowAnalysisContext` to `Environment`, and adding code in `pushCall` to set it.
Reviewed By: ymandel, sgatev, xazax.hun
Differential Revision: https://reviews.llvm.org/D131170
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.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 c83d0cbbbdbb3..e7533794de48b 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -137,22 +137,6 @@ class DataflowAnalysisContext {
return It == ExprToLoc.end() ? nullptr : It->second;
}
- /// Assigns `Loc` as the storage location of the `this` pointee.
- ///
- /// Requirements:
- ///
- /// The `this` pointee must not be assigned a storage location.
- void setThisPointeeStorageLocation(StorageLocation &Loc) {
- assert(ThisPointeeLoc == nullptr);
- ThisPointeeLoc = &Loc;
- }
-
- /// Returns the storage location assigned to the `this` pointee or null if the
- /// `this` pointee has no assigned storage location.
- StorageLocation *getThisPointeeStorageLocation() const {
- return ThisPointeeLoc;
- }
-
/// Returns a pointer value that represents a null pointer. Calls with
/// `PointeeType` that are canonically equivalent will return the same result.
/// A null `PointeeType` can be used for the pointee of `std::nullptr_t`.
@@ -322,9 +306,6 @@ 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.
//
// FIXME: The pointer values are indexed by the pointee types which are
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index e8a1c2db53b5c..fc43b6b43575f 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -380,7 +380,9 @@ class Environment {
// 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`.
+ // The storage location of the `this` pointee. Should only be null if the
+ // function being analyzed is only a function and not a method.
+ StorageLocation *ThisPointeeLoc = nullptr;
// Maps from program declarations and statements to storage locations that are
// assigned to them. Unlike the maps in `DataflowAnalysisContext`, these
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index f4dadbb79b5c9..ff27a2a45179b 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -155,8 +155,9 @@ Environment::Environment(DataflowAnalysisContext &DACtx)
Environment::Environment(const Environment &Other)
: DACtx(Other.DACtx), ReturnLoc(Other.ReturnLoc),
- DeclToLoc(Other.DeclToLoc), ExprToLoc(Other.ExprToLoc),
- LocToVal(Other.LocToVal), MemberLocToStruct(Other.MemberLocToStruct),
+ ThisPointeeLoc(Other.ThisPointeeLoc), DeclToLoc(Other.DeclToLoc),
+ ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
+ MemberLocToStruct(Other.MemberLocToStruct),
FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) {
}
@@ -194,10 +195,9 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
QualType ThisPointeeType = MethodDecl->getThisObjectType();
// FIXME: Add support for union types.
if (!ThisPointeeType->isUnionType()) {
- auto &ThisPointeeLoc = createStorageLocation(ThisPointeeType);
- DACtx.setThisPointeeStorageLocation(ThisPointeeLoc);
+ ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
if (Value *ThisPointeeVal = createValue(ThisPointeeType))
- setValue(ThisPointeeLoc, *ThisPointeeVal);
+ setValue(*ThisPointeeLoc, *ThisPointeeVal);
}
}
}
@@ -213,6 +213,12 @@ Environment Environment::pushCall(const CallExpr *Call) const {
// FIXME: In order to allow the callee to reference globals, we probably need
// to call `initGlobalVars` here in some way.
+ if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
+ if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
+ Env.ThisPointeeLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
+ }
+ }
+
auto ParamIt = FuncDecl->param_begin();
auto ArgIt = Call->arg_begin();
auto ArgEnd = Call->arg_end();
@@ -246,12 +252,12 @@ 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 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.
+ // callee's `ReturnLoc` or `ThisPointeeLoc`. 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);
@@ -264,6 +270,9 @@ bool Environment::equivalentTo(const Environment &Other,
if (ReturnLoc != Other.ReturnLoc)
return false;
+ if (ThisPointeeLoc != Other.ThisPointeeLoc)
+ return false;
+
if (DeclToLoc != Other.DeclToLoc)
return false;
@@ -294,12 +303,14 @@ LatticeJoinEffect Environment::join(const Environment &Other,
Environment::ValueModel &Model) {
assert(DACtx == Other.DACtx);
assert(ReturnLoc == Other.ReturnLoc);
+ assert(ThisPointeeLoc == Other.ThisPointeeLoc);
auto Effect = LatticeJoinEffect::Unchanged;
Environment JoinedEnv(*DACtx);
JoinedEnv.ReturnLoc = ReturnLoc;
+ JoinedEnv.ThisPointeeLoc = ThisPointeeLoc;
JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
@@ -390,7 +401,7 @@ StorageLocation *Environment::getStorageLocation(const Expr &E,
}
StorageLocation *Environment::getThisPointeeStorageLocation() const {
- return DACtx->getThisPointeeStorageLocation();
+ return ThisPointeeLoc;
}
StorageLocation *Environment::getReturnStorageLocation() const {
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 1d114d9df1759..ebca4b3ea7f15 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -4229,4 +4229,143 @@ TEST(TransferTest, ContextSensitiveReturnArg) {
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
}
+TEST(TransferTest, ContextSensitiveMethodLiteral) {
+ std::string Code = R"(
+ class MyClass {
+ public:
+ bool giveBool() { return true; }
+ };
+
+ void target() {
+ MyClass MyObj;
+ bool Foo = MyObj.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, ContextSensitiveMethodGetter) {
+ std::string Code = R"(
+ class MyClass {
+ public:
+ bool getField() { return Field; }
+
+ bool Field;
+ };
+
+ void target() {
+ MyClass MyObj;
+ MyObj.Field = true;
+ bool Foo = MyObj.getField();
+ // [[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, ContextSensitiveMethodSetter) {
+ std::string Code = R"(
+ class MyClass {
+ public:
+ bool setField(bool Val) { Field = Val; }
+
+ bool Field;
+ };
+
+ void target() {
+ MyClass MyObj;
+ MyObj.setField(true);
+ bool Foo = MyObj.Field;
+ // [[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, ContextSensitiveMethodGetterAndSetter) {
+ std::string Code = R"(
+ class MyClass {
+ public:
+ bool getField() { return Field; }
+ bool setField(bool Val) { Field = Val; }
+
+ private:
+ bool Field;
+ };
+
+ void target() {
+ MyClass MyObj;
+ MyObj.setField(true);
+ bool Foo = MyObj.getField();
+ // [[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}});
+}
+
} // namespace
More information about the cfe-commits
mailing list