Skip to content

Commit 7587065

Browse files
eugeniseugenis
committed
Revert "[clang][dataflow] Analyze constructor bodies"
https://lab.llvm.org/buildbot/#/builders/74/builds/12713 This reverts commit 000c8fe.
1 parent 26089d4 commit 7587065

File tree

4 files changed

+65
-199
lines changed

4 files changed

+65
-199
lines changed

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,14 @@ class Environment {
135135
///
136136
/// Requirements:
137137
///
138-
/// The callee of `Call` must be a `FunctionDecl`.
138+
/// The callee of `Call` must be a `FunctionDecl` with a body.
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;
147146

148147
/// Moves gathered information back into `this` from a `CalleeEnv` created via
149148
/// `pushCall`.
@@ -382,12 +381,6 @@ class Environment {
382381
StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
383382
const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
384383

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-
391384
// `DACtx` is not null and not owned by this object.
392385
DataflowAnalysisContext *DACtx;
393386

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

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

208208
Environment Environment::pushCall(const CallExpr *Call) const {
209209
Environment Env(*this);
210-
211210
// FIXME: Support references here.
212-
Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);
213-
214-
if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
215-
if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
216-
Env.ThisPointeeLoc = getStorageLocation(*Arg, SkipPast::Reference);
217-
}
218-
}
219-
220-
Env.pushCallInternal(Call->getDirectCallee(),
221-
llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
211+
Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference);
222212

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);
213+
const auto *FuncDecl = Call->getDirectCallee();
214+
assert(FuncDecl != nullptr);
231215

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);
216+
Env.setDeclCtx(FuncDecl);
243217

244218
// FIXME: In order to allow the callee to reference globals, we probably need
245219
// to call `initGlobalVars` here in some way.
246220

221+
if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
222+
if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
223+
Env.ThisPointeeLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
224+
}
225+
}
226+
247227
auto ParamIt = FuncDecl->param_begin();
228+
auto ArgIt = Call->arg_begin();
229+
auto ArgEnd = Call->arg_end();
248230

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

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

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

262244
QualType ParamType = Param->getType();
263245
if (ParamType->isReferenceType()) {
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);
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);
270252
}
271253
}
254+
255+
return Env;
272256
}
273257

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

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

Lines changed: 39 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -444,8 +444,6 @@ 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);
449447
}
450448

451449
void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *S) {
@@ -528,7 +526,45 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
528526
return;
529527
Env.setStorageLocation(*S, *ArgLoc);
530528
} else if (const FunctionDecl *F = S->getDirectCallee()) {
531-
transferInlineCall(S, F);
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);
532568
}
533569
}
534570

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

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-
705696
const StmtToEnvMap &StmtToEnv;
706697
Environment &Env;
707698
TransferOptions Options;

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

Lines changed: 0 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -4368,106 +4368,4 @@ 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-
44734371
} // namespace

0 commit comments

Comments
 (0)