Skip to content

Commit 000c8fe

Browse files
samestepsamestep
committed
[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
1 parent 42f32f6 commit 000c8fe

File tree

4 files changed

+199
-65
lines changed

4 files changed

+199
-65
lines changed

‎clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,14 +135,15 @@ class Environment {
135135
///
136136
/// Requirements:
137137
///
138-
/// The callee of `Call` must be a `FunctionDecl` with a body.
138+
/// The callee of `Call` must be a `FunctionDecl`.
139139
///
140140
/// The body of the callee must not reference globals.
141141
///
142142
/// The arguments of `Call` must map 1:1 to the callee's parameters.
143143
///
144144
/// Each argument of `Call` must already have a `StorageLocation`.
145145
Environment pushCall(const CallExpr *Call) const;
146+
Environment pushCall(const CXXConstructExpr *Call) const;
146147

147148
/// Moves gathered information back into `this` from a `CalleeEnv` created via
148149
/// `pushCall`.
@@ -381,6 +382,12 @@ class Environment {
381382
StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
382383
const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
383384

385+
/// Shared implementation of `pushCall` overloads. Note that unlike
386+
/// `pushCall`, this member is invoked on the environment of the callee, not
387+
/// of the caller.
388+
void pushCallInternal(const FunctionDecl *FuncDecl,
389+
ArrayRef<const Expr *> Args);
390+
384391
// `DACtx` is not null and not owned by this object.
385392
DataflowAnalysisContext *DACtx;
386393

‎clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -207,52 +207,68 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
207207

208208
Environment Environment::pushCall(const CallExpr *Call) const {
209209
Environment Env(*this);
210-
// FIXME: Support references here.
211-
Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference);
212-
213-
const auto *FuncDecl = Call->getDirectCallee();
214-
assert(FuncDecl != nullptr);
215210

216-
Env.setDeclCtx(FuncDecl);
217-
218-
// FIXME: In order to allow the callee to reference globals, we probably need
219-
// to call `initGlobalVars` here in some way.
211+
// FIXME: Support references here.
212+
Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);
220213

221214
if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
222215
if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
223-
Env.ThisPointeeLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
216+
Env.ThisPointeeLoc = getStorageLocation(*Arg, SkipPast::Reference);
224217
}
225218
}
226219

220+
Env.pushCallInternal(Call->getDirectCallee(),
221+
llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
222+
223+
return Env;
224+
}
225+
226+
Environment Environment::pushCall(const CXXConstructExpr *Call) const {
227+
Environment Env(*this);
228+
229+
// FIXME: Support references here.
230+
Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);
231+
232+
Env.ThisPointeeLoc = Env.ReturnLoc;
233+
234+
Env.pushCallInternal(Call->getConstructor(),
235+
llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
236+
237+
return Env;
238+
}
239+
240+
void Environment::pushCallInternal(const FunctionDecl *FuncDecl,
241+
ArrayRef<const Expr *> Args) {
242+
setDeclCtx(FuncDecl);
243+
244+
// FIXME: In order to allow the callee to reference globals, we probably need
245+
// to call `initGlobalVars` here in some way.
246+
227247
auto ParamIt = FuncDecl->param_begin();
228-
auto ArgIt = Call->arg_begin();
229-
auto ArgEnd = Call->arg_end();
230248

231249
// FIXME: Parameters don't always map to arguments 1:1; examples include
232250
// overloaded operators implemented as member functions, and parameter packs.
233-
for (; ArgIt != ArgEnd; ++ParamIt, ++ArgIt) {
251+
for (unsigned ArgIndex = 0; ArgIndex < Args.size(); ++ParamIt, ++ArgIndex) {
234252
assert(ParamIt != FuncDecl->param_end());
235253

236-
const Expr *Arg = *ArgIt;
237-
auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
254+
const Expr *Arg = Args[ArgIndex];
255+
auto *ArgLoc = getStorageLocation(*Arg, SkipPast::Reference);
238256
assert(ArgLoc != nullptr);
239257

240258
const VarDecl *Param = *ParamIt;
241-
auto &Loc = Env.createStorageLocation(*Param);
242-
Env.setStorageLocation(*Param, Loc);
259+
auto &Loc = createStorageLocation(*Param);
260+
setStorageLocation(*Param, Loc);
243261

244262
QualType ParamType = Param->getType();
245263
if (ParamType->isReferenceType()) {
246-
auto &Val = Env.takeOwnership(std::make_unique<ReferenceValue>(*ArgLoc));
247-
Env.setValue(Loc, Val);
248-
} else if (auto *ArgVal = Env.getValue(*ArgLoc)) {
249-
Env.setValue(Loc, *ArgVal);
250-
} else if (Value *Val = Env.createValue(ParamType)) {
251-
Env.setValue(Loc, *Val);
264+
auto &Val = takeOwnership(std::make_unique<ReferenceValue>(*ArgLoc));
265+
setValue(Loc, Val);
266+
} else if (auto *ArgVal = getValue(*ArgLoc)) {
267+
setValue(Loc, *ArgVal);
268+
} else if (Value *Val = createValue(ParamType)) {
269+
setValue(Loc, *Val);
252270
}
253271
}
254-
255-
return Env;
256272
}
257273

258274
void Environment::popCall(const Environment &CalleeEnv) {

‎clang/lib/Analysis/FlowSensitive/Transfer.cpp

Lines changed: 48 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,8 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
444444
Env.setStorageLocation(*S, Loc);
445445
if (Value *Val = Env.createValue(S->getType()))
446446
Env.setValue(Loc, *Val);
447+
448+
transferInlineCall(S, ConstructorDecl);
447449
}
448450

449451
void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *S) {
@@ -526,45 +528,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
526528
return;
527529
Env.setStorageLocation(*S, *ArgLoc);
528530
} else if (const FunctionDecl *F = S->getDirectCallee()) {
529-
// This case is for context-sensitive analysis.
530-
if (!Options.ContextSensitive)
531-
return;
532-
533-
const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
534-
if (!CFCtx)
535-
return;
536-
537-
// FIXME: We don't support context-sensitive analysis of recursion, so
538-
// we should return early here if `F` is the same as the `FunctionDecl`
539-
// holding `S` itself.
540-
541-
auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
542-
543-
// Note that it is important for the storage location of `S` to be set
544-
// before `pushCall`, because the latter uses it to set the storage
545-
// location for `return`.
546-
auto &ReturnLoc = Env.createStorageLocation(*S);
547-
Env.setStorageLocation(*S, ReturnLoc);
548-
auto CalleeEnv = Env.pushCall(S);
549-
550-
// FIXME: Use the same analysis as the caller for the callee. Note,
551-
// though, that doing so would require support for changing the analysis's
552-
// ASTContext.
553-
assert(
554-
CFCtx->getDecl() != nullptr &&
555-
"ControlFlowContexts in the environment should always carry a decl");
556-
auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(),
557-
DataflowAnalysisOptions());
558-
559-
auto BlockToOutputState =
560-
dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
561-
assert(BlockToOutputState);
562-
assert(ExitBlock < BlockToOutputState->size());
563-
564-
auto ExitState = (*BlockToOutputState)[ExitBlock];
565-
assert(ExitState);
566-
567-
Env.popCall(ExitState->Env);
531+
transferInlineCall(S, F);
568532
}
569533
}
570534

@@ -693,6 +657,51 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
693657
return Env.makeAtomicBoolValue();
694658
}
695659

660+
// If context sensitivity is enabled, try to analyze the body of the callee
661+
// `F` of `S`. The type `E` must be either `CallExpr` or `CXXConstructExpr`.
662+
template <typename E>
663+
void transferInlineCall(const E *S, const FunctionDecl *F) {
664+
if (!Options.ContextSensitive)
665+
return;
666+
667+
const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
668+
if (!CFCtx)
669+
return;
670+
671+
// FIXME: We don't support context-sensitive analysis of recursion, so
672+
// we should return early here if `F` is the same as the `FunctionDecl`
673+
// holding `S` itself.
674+
675+
auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
676+
677+
if (const auto *NonConstructExpr = dyn_cast<CallExpr>(S)) {
678+
// Note that it is important for the storage location of `S` to be set
679+
// before `pushCall`, because the latter uses it to set the storage
680+
// location for `return`.
681+
auto &ReturnLoc = Env.createStorageLocation(*S);
682+
Env.setStorageLocation(*S, ReturnLoc);
683+
}
684+
auto CalleeEnv = Env.pushCall(S);
685+
686+
// FIXME: Use the same analysis as the caller for the callee. Note,
687+
// though, that doing so would require support for changing the analysis's
688+
// ASTContext.
689+
assert(CFCtx->getDecl() != nullptr &&
690+
"ControlFlowContexts in the environment should always carry a decl");
691+
auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(),
692+
DataflowAnalysisOptions());
693+
694+
auto BlockToOutputState =
695+
dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
696+
assert(BlockToOutputState);
697+
assert(ExitBlock < BlockToOutputState->size());
698+
699+
auto ExitState = (*BlockToOutputState)[ExitBlock];
700+
assert(ExitState);
701+
702+
Env.popCall(ExitState->Env);
703+
}
704+
696705
const StmtToEnvMap &StmtToEnv;
697706
Environment &Env;
698707
TransferOptions Options;

‎clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4368,4 +4368,106 @@ TEST(TransferTest, ContextSensitiveMethodGetterAndSetter) {
43684368
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
43694369
}
43704370

4371+
TEST(TransferTest, ContextSensitiveConstructorBody) {
4372+
std::string Code = R"(
4373+
class MyClass {
4374+
public:
4375+
MyClass() { MyField = true; }
4376+
4377+
bool MyField;
4378+
};
4379+
4380+
void target() {
4381+
MyClass MyObj;
4382+
bool Foo = MyObj.MyField;
4383+
// [[p]]
4384+
}
4385+
)";
4386+
runDataflow(Code,
4387+
[](llvm::ArrayRef<
4388+
std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
4389+
Results,
4390+
ASTContext &ASTCtx) {
4391+
ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
4392+
const Environment &Env = Results[0].second.Env;
4393+
4394+
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
4395+
ASSERT_THAT(FooDecl, NotNull());
4396+
4397+
auto &FooVal =
4398+
*cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
4399+
EXPECT_TRUE(Env.flowConditionImplies(FooVal));
4400+
},
4401+
{/*.ApplyBuiltinTransfer=*/true,
4402+
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
4403+
}
4404+
4405+
TEST(TransferTest, ContextSensitiveConstructorInitializer) {
4406+
std::string Code = R"(
4407+
class MyClass {
4408+
public:
4409+
MyClass() : MyField(true) {}
4410+
4411+
bool MyField;
4412+
};
4413+
4414+
void target() {
4415+
MyClass MyObj;
4416+
bool Foo = MyObj.MyField;
4417+
// [[p]]
4418+
}
4419+
)";
4420+
runDataflow(Code,
4421+
[](llvm::ArrayRef<
4422+
std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
4423+
Results,
4424+
ASTContext &ASTCtx) {
4425+
ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
4426+
const Environment &Env = Results[0].second.Env;
4427+
4428+
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
4429+
ASSERT_THAT(FooDecl, NotNull());
4430+
4431+
auto &FooVal =
4432+
*cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
4433+
EXPECT_TRUE(Env.flowConditionImplies(FooVal));
4434+
},
4435+
{/*.ApplyBuiltinTransfer=*/true,
4436+
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
4437+
}
4438+
4439+
TEST(TransferTest, ContextSensitiveConstructorDefault) {
4440+
std::string Code = R"(
4441+
class MyClass {
4442+
public:
4443+
MyClass() = default;
4444+
4445+
bool MyField = true;
4446+
};
4447+
4448+
void target() {
4449+
MyClass MyObj;
4450+
bool Foo = MyObj.MyField;
4451+
// [[p]]
4452+
}
4453+
)";
4454+
runDataflow(Code,
4455+
[](llvm::ArrayRef<
4456+
std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
4457+
Results,
4458+
ASTContext &ASTCtx) {
4459+
ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
4460+
const Environment &Env = Results[0].second.Env;
4461+
4462+
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
4463+
ASSERT_THAT(FooDecl, NotNull());
4464+
4465+
auto &FooVal =
4466+
*cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
4467+
EXPECT_TRUE(Env.flowConditionImplies(FooVal));
4468+
},
4469+
{/*.ApplyBuiltinTransfer=*/true,
4470+
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
4471+
}
4472+
43714473
} // namespace

0 commit comments

Comments
 (0)