[clang] eb91fd5 - [clang][dataflow] Analyze constructor bodies
Wei Yi Tee via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 11 05:55:25 PDT 2022
Author: Sam Estep
Date: 2022-08-11T12:46:20Z
New Revision: eb91fd5cbc6995099bd38c04c839686cbdd18b92
URL: https://github.com/llvm/llvm-project/commit/eb91fd5cbc6995099bd38c04c839686cbdd18b92
DIFF: https://github.com/llvm/llvm-project/commit/eb91fd5cbc6995099bd38c04c839686cbdd18b92.diff
LOG: [clang][dataflow] Analyze constructor bodies
This patch adds the ability to context-sensitively analyze constructor bodies, by changing `pushCall` to allow both `CallExpr` and `CXXConstructExpr`, and extracting the main context-sensitive logic out of `VisitCallExpr` into a new `transferInlineCall` method which is now also called at the end of `VisitCXXConstructExpr`.
Reviewed By: ymandel, sgatev, xazax.hun
Differential Revision: https://reviews.llvm.org/D131438
Added:
Modified:
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/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 94fe2be37b2eb..c30a76267716d 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -135,7 +135,7 @@ class Environment {
///
/// Requirements:
///
- /// The callee of `Call` must be a `FunctionDecl` with a body.
+ /// The callee of `Call` must be a `FunctionDecl`.
///
/// The body of the callee must not reference globals.
///
@@ -143,6 +143,7 @@ class Environment {
///
/// Each argument of `Call` must already have a `StorageLocation`.
Environment pushCall(const CallExpr *Call) const;
+ Environment pushCall(const CXXConstructExpr *Call) const;
/// Moves gathered information back into `this` from a `CalleeEnv` created via
/// `pushCall`.
@@ -381,6 +382,12 @@ class Environment {
StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
+ /// Shared implementation of `pushCall` overloads. Note that unlike
+ /// `pushCall`, this member is invoked on the environment of the callee, not
+ /// of the caller.
+ void pushCallInternal(const FunctionDecl *FuncDecl,
+ ArrayRef<const Expr *> Args);
+
// `DACtx` is not null and not owned by this object.
DataflowAnalysisContext *DACtx;
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 16c83cad9d9e3..e4af68e53e14e 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -207,52 +207,68 @@ 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);
- Env.setDeclCtx(FuncDecl);
-
- // FIXME: In order to allow the callee to reference globals, we probably need
- // to call `initGlobalVars` here in some way.
+ // FIXME: Support references here.
+ Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);
if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
- Env.ThisPointeeLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
+ Env.ThisPointeeLoc = getStorageLocation(*Arg, SkipPast::Reference);
}
}
+ Env.pushCallInternal(Call->getDirectCallee(),
+ llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
+
+ return Env;
+}
+
+Environment Environment::pushCall(const CXXConstructExpr *Call) const {
+ Environment Env(*this);
+
+ // FIXME: Support references here.
+ Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);
+
+ Env.ThisPointeeLoc = Env.ReturnLoc;
+
+ Env.pushCallInternal(Call->getConstructor(),
+ llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
+
+ return Env;
+}
+
+void Environment::pushCallInternal(const FunctionDecl *FuncDecl,
+ ArrayRef<const Expr *> Args) {
+ setDeclCtx(FuncDecl);
+
+ // FIXME: In order to allow the callee to reference globals, we probably need
+ // to call `initGlobalVars` here in some way.
+
auto ParamIt = FuncDecl->param_begin();
- auto ArgIt = Call->arg_begin();
- auto ArgEnd = Call->arg_end();
// FIXME: Parameters don't always map to arguments 1:1; examples include
// overloaded operators implemented as member functions, and parameter packs.
- for (; ArgIt != ArgEnd; ++ParamIt, ++ArgIt) {
+ for (unsigned ArgIndex = 0; ArgIndex < Args.size(); ++ParamIt, ++ArgIndex) {
assert(ParamIt != FuncDecl->param_end());
- const Expr *Arg = *ArgIt;
- auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
+ const Expr *Arg = Args[ArgIndex];
+ auto *ArgLoc = getStorageLocation(*Arg, SkipPast::Reference);
assert(ArgLoc != nullptr);
const VarDecl *Param = *ParamIt;
- auto &Loc = Env.createStorageLocation(*Param);
- Env.setStorageLocation(*Param, Loc);
+ auto &Loc = createStorageLocation(*Param);
+ 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);
+ auto &Val = takeOwnership(std::make_unique<ReferenceValue>(*ArgLoc));
+ setValue(Loc, Val);
+ } else if (auto *ArgVal = getValue(*ArgLoc)) {
+ setValue(Loc, *ArgVal);
+ } else if (Value *Val = createValue(ParamType)) {
+ setValue(Loc, *Val);
}
}
-
- return Env;
}
void Environment::popCall(const Environment &CalleeEnv) {
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 430dc8d5faa97..2f05b08fe9bc2 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -444,6 +444,8 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
Env.setStorageLocation(*S, Loc);
if (Value *Val = Env.createValue(S->getType()))
Env.setValue(Loc, *Val);
+
+ transferInlineCall(S, ConstructorDecl);
}
void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *S) {
@@ -526,45 +528,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
return;
Env.setStorageLocation(*S, *ArgLoc);
} else if (const FunctionDecl *F = S->getDirectCallee()) {
- // This case is for context-sensitive analysis.
- if (!Options.ContextSensitive)
- return;
-
- const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
- if (!CFCtx)
- return;
-
- // FIXME: We don't support context-sensitive analysis of recursion, so
- // we should return early here if `F` is the same as the `FunctionDecl`
- // holding `S` itself.
-
- 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,
- // though, that doing so would require support for changing the analysis's
- // ASTContext.
- assert(
- CFCtx->getDecl() != nullptr &&
- "ControlFlowContexts in the environment should always carry a decl");
- auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(),
- DataflowAnalysisOptions());
-
- auto BlockToOutputState =
- dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
- assert(BlockToOutputState);
- assert(ExitBlock < BlockToOutputState->size());
-
- auto ExitState = (*BlockToOutputState)[ExitBlock];
- assert(ExitState);
-
- Env.popCall(ExitState->Env);
+ transferInlineCall(S, F);
}
}
@@ -693,6 +657,51 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
return Env.makeAtomicBoolValue();
}
+ // If context sensitivity is enabled, try to analyze the body of the callee
+ // `F` of `S`. The type `E` must be either `CallExpr` or `CXXConstructExpr`.
+ template <typename E>
+ void transferInlineCall(const E *S, const FunctionDecl *F) {
+ if (!Options.ContextSensitive)
+ return;
+
+ const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
+ if (!CFCtx)
+ return;
+
+ // FIXME: We don't support context-sensitive analysis of recursion, so
+ // we should return early here if `F` is the same as the `FunctionDecl`
+ // holding `S` itself.
+
+ auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
+
+ if (const auto *NonConstructExpr = dyn_cast<CallExpr>(S)) {
+ // 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,
+ // though, that doing so would require support for changing the analysis's
+ // ASTContext.
+ assert(CFCtx->getDecl() != nullptr &&
+ "ControlFlowContexts in the environment should always carry a decl");
+ auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(),
+ DataflowAnalysisOptions());
+
+ auto BlockToOutputState =
+ dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
+ assert(BlockToOutputState);
+ assert(ExitBlock < BlockToOutputState->size());
+
+ auto ExitState = (*BlockToOutputState)[ExitBlock];
+ assert(ExitState);
+
+ Env.popCall(ExitState->Env);
+ }
+
const StmtToEnvMap &StmtToEnv;
Environment &Env;
TransferOptions Options;
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index ebca4b3ea7f15..af06021abccfd 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -4368,4 +4368,106 @@ TEST(TransferTest, ContextSensitiveMethodGetterAndSetter) {
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
}
+TEST(TransferTest, ContextSensitiveConstructorBody) {
+ std::string Code = R"(
+ class MyClass {
+ public:
+ MyClass() { MyField = true; }
+
+ bool MyField;
+ };
+
+ void target() {
+ MyClass MyObj;
+ bool Foo = MyObj.MyField;
+ // [[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, ContextSensitiveConstructorInitializer) {
+ std::string Code = R"(
+ class MyClass {
+ public:
+ MyClass() : MyField(true) {}
+
+ bool MyField;
+ };
+
+ void target() {
+ MyClass MyObj;
+ bool Foo = MyObj.MyField;
+ // [[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, ContextSensitiveConstructorDefault) {
+ std::string Code = R"(
+ class MyClass {
+ public:
+ MyClass() = default;
+
+ bool MyField = true;
+ };
+
+ void target() {
+ MyClass MyObj;
+ bool Foo = MyObj.MyField;
+ // [[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