Skip to content

Commit 8611a77

Browse files
samestepsamestep
committed
[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
1 parent 954de25 commit 8611a77

File tree

4 files changed

+165
-32
lines changed

4 files changed

+165
-32
lines changed

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

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -137,22 +137,6 @@ class DataflowAnalysisContext {
137137
return It == ExprToLoc.end() ? nullptr : It->second;
138138
}
139139

140-
/// Assigns `Loc` as the storage location of the `this` pointee.
141-
///
142-
/// Requirements:
143-
///
144-
/// The `this` pointee must not be assigned a storage location.
145-
void setThisPointeeStorageLocation(StorageLocation &Loc) {
146-
assert(ThisPointeeLoc == nullptr);
147-
ThisPointeeLoc = &Loc;
148-
}
149-
150-
/// Returns the storage location assigned to the `this` pointee or null if the
151-
/// `this` pointee has no assigned storage location.
152-
StorageLocation *getThisPointeeStorageLocation() const {
153-
return ThisPointeeLoc;
154-
}
155-
156140
/// Returns a pointer value that represents a null pointer. Calls with
157141
/// `PointeeType` that are canonically equivalent will return the same result.
158142
/// A null `PointeeType` can be used for the pointee of `std::nullptr_t`.
@@ -322,9 +306,6 @@ class DataflowAnalysisContext {
322306
llvm::DenseMap<const ValueDecl *, StorageLocation *> DeclToLoc;
323307
llvm::DenseMap<const Expr *, StorageLocation *> ExprToLoc;
324308

325-
// FIXME: Move this into `Environment`.
326-
StorageLocation *ThisPointeeLoc = nullptr;
327-
328309
// Null pointer values, keyed by the canonical pointee type.
329310
//
330311
// FIXME: The pointer values are indexed by the pointee types which are

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,9 @@ class Environment {
380380
// In a properly initialized `Environment`, `ReturnLoc` should only be null if
381381
// its `DeclContext` could not be cast to a `FunctionDecl`.
382382
StorageLocation *ReturnLoc = nullptr;
383-
// FIXME: Move `ThisPointeeLoc` here from `DataflowAnalysisContext`.
383+
// The storage location of the `this` pointee. Should only be null if the
384+
// function being analyzed is only a function and not a method.
385+
StorageLocation *ThisPointeeLoc = nullptr;
384386

385387
// Maps from program declarations and statements to storage locations that are
386388
// assigned to them. Unlike the maps in `DataflowAnalysisContext`, these

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

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,9 @@ Environment::Environment(DataflowAnalysisContext &DACtx)
155155

156156
Environment::Environment(const Environment &Other)
157157
: DACtx(Other.DACtx), ReturnLoc(Other.ReturnLoc),
158-
DeclToLoc(Other.DeclToLoc), ExprToLoc(Other.ExprToLoc),
159-
LocToVal(Other.LocToVal), MemberLocToStruct(Other.MemberLocToStruct),
158+
ThisPointeeLoc(Other.ThisPointeeLoc), DeclToLoc(Other.DeclToLoc),
159+
ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
160+
MemberLocToStruct(Other.MemberLocToStruct),
160161
FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) {
161162
}
162163

@@ -194,10 +195,9 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
194195
QualType ThisPointeeType = MethodDecl->getThisObjectType();
195196
// FIXME: Add support for union types.
196197
if (!ThisPointeeType->isUnionType()) {
197-
auto &ThisPointeeLoc = createStorageLocation(ThisPointeeType);
198-
DACtx.setThisPointeeStorageLocation(ThisPointeeLoc);
198+
ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
199199
if (Value *ThisPointeeVal = createValue(ThisPointeeType))
200-
setValue(ThisPointeeLoc, *ThisPointeeVal);
200+
setValue(*ThisPointeeLoc, *ThisPointeeVal);
201201
}
202202
}
203203
}
@@ -213,6 +213,12 @@ Environment Environment::pushCall(const CallExpr *Call) const {
213213
// FIXME: In order to allow the callee to reference globals, we probably need
214214
// to call `initGlobalVars` here in some way.
215215

216+
if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
217+
if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
218+
Env.ThisPointeeLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
219+
}
220+
}
221+
216222
auto ParamIt = FuncDecl->param_begin();
217223
auto ArgIt = Call->arg_begin();
218224
auto ArgEnd = Call->arg_end();
@@ -246,12 +252,12 @@ Environment Environment::pushCall(const CallExpr *Call) const {
246252

247253
void Environment::popCall(const Environment &CalleeEnv) {
248254
// We ignore `DACtx` because it's already the same in both. We don't want the
249-
// callee's `ReturnLoc`. We don't bring back `DeclToLoc` and `ExprToLoc`
250-
// because we want to be able to later analyze the same callee in a different
251-
// context, and `setStorageLocation` requires there to not already be a
252-
// storage location assigned. Conceptually, these maps capture information
253-
// from the local scope, so when popping that scope, we do not propagate the
254-
// maps.
255+
// callee's `ReturnLoc` or `ThisPointeeLoc`. We don't bring back `DeclToLoc`
256+
// and `ExprToLoc` because we want to be able to later analyze the same callee
257+
// in a different context, and `setStorageLocation` requires there to not
258+
// already be a storage location assigned. Conceptually, these maps capture
259+
// information from the local scope, so when popping that scope, we do not
260+
// propagate the maps.
255261
this->LocToVal = std::move(CalleeEnv.LocToVal);
256262
this->MemberLocToStruct = std::move(CalleeEnv.MemberLocToStruct);
257263
this->FlowConditionToken = std::move(CalleeEnv.FlowConditionToken);
@@ -264,6 +270,9 @@ bool Environment::equivalentTo(const Environment &Other,
264270
if (ReturnLoc != Other.ReturnLoc)
265271
return false;
266272

273+
if (ThisPointeeLoc != Other.ThisPointeeLoc)
274+
return false;
275+
267276
if (DeclToLoc != Other.DeclToLoc)
268277
return false;
269278

@@ -294,12 +303,14 @@ LatticeJoinEffect Environment::join(const Environment &Other,
294303
Environment::ValueModel &Model) {
295304
assert(DACtx == Other.DACtx);
296305
assert(ReturnLoc == Other.ReturnLoc);
306+
assert(ThisPointeeLoc == Other.ThisPointeeLoc);
297307

298308
auto Effect = LatticeJoinEffect::Unchanged;
299309

300310
Environment JoinedEnv(*DACtx);
301311

302312
JoinedEnv.ReturnLoc = ReturnLoc;
313+
JoinedEnv.ThisPointeeLoc = ThisPointeeLoc;
303314

304315
JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
305316
if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
@@ -390,7 +401,7 @@ StorageLocation *Environment::getStorageLocation(const Expr &E,
390401
}
391402

392403
StorageLocation *Environment::getThisPointeeStorageLocation() const {
393-
return DACtx->getThisPointeeStorageLocation();
404+
return ThisPointeeLoc;
394405
}
395406

396407
StorageLocation *Environment::getReturnStorageLocation() const {

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

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4229,4 +4229,143 @@ TEST(TransferTest, ContextSensitiveReturnArg) {
42294229
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
42304230
}
42314231

4232+
TEST(TransferTest, ContextSensitiveMethodLiteral) {
4233+
std::string Code = R"(
4234+
class MyClass {
4235+
public:
4236+
bool giveBool() { return true; }
4237+
};
4238+
4239+
void target() {
4240+
MyClass MyObj;
4241+
bool Foo = MyObj.giveBool();
4242+
// [[p]]
4243+
}
4244+
)";
4245+
runDataflow(Code,
4246+
[](llvm::ArrayRef<
4247+
std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
4248+
Results,
4249+
ASTContext &ASTCtx) {
4250+
ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
4251+
const Environment &Env = Results[0].second.Env;
4252+
4253+
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
4254+
ASSERT_THAT(FooDecl, NotNull());
4255+
4256+
auto &FooVal =
4257+
*cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
4258+
EXPECT_TRUE(Env.flowConditionImplies(FooVal));
4259+
},
4260+
{/*.ApplyBuiltinTransfer=*/true,
4261+
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
4262+
}
4263+
4264+
TEST(TransferTest, ContextSensitiveMethodGetter) {
4265+
std::string Code = R"(
4266+
class MyClass {
4267+
public:
4268+
bool getField() { return Field; }
4269+
4270+
bool Field;
4271+
};
4272+
4273+
void target() {
4274+
MyClass MyObj;
4275+
MyObj.Field = true;
4276+
bool Foo = MyObj.getField();
4277+
// [[p]]
4278+
}
4279+
)";
4280+
runDataflow(Code,
4281+
[](llvm::ArrayRef<
4282+
std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
4283+
Results,
4284+
ASTContext &ASTCtx) {
4285+
ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
4286+
const Environment &Env = Results[0].second.Env;
4287+
4288+
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
4289+
ASSERT_THAT(FooDecl, NotNull());
4290+
4291+
auto &FooVal =
4292+
*cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
4293+
EXPECT_TRUE(Env.flowConditionImplies(FooVal));
4294+
},
4295+
{/*.ApplyBuiltinTransfer=*/true,
4296+
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
4297+
}
4298+
4299+
TEST(TransferTest, ContextSensitiveMethodSetter) {
4300+
std::string Code = R"(
4301+
class MyClass {
4302+
public:
4303+
bool setField(bool Val) { Field = Val; }
4304+
4305+
bool Field;
4306+
};
4307+
4308+
void target() {
4309+
MyClass MyObj;
4310+
MyObj.setField(true);
4311+
bool Foo = MyObj.Field;
4312+
// [[p]]
4313+
}
4314+
)";
4315+
runDataflow(Code,
4316+
[](llvm::ArrayRef<
4317+
std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
4318+
Results,
4319+
ASTContext &ASTCtx) {
4320+
ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
4321+
const Environment &Env = Results[0].second.Env;
4322+
4323+
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
4324+
ASSERT_THAT(FooDecl, NotNull());
4325+
4326+
auto &FooVal =
4327+
*cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
4328+
EXPECT_TRUE(Env.flowConditionImplies(FooVal));
4329+
},
4330+
{/*.ApplyBuiltinTransfer=*/true,
4331+
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
4332+
}
4333+
4334+
TEST(TransferTest, ContextSensitiveMethodGetterAndSetter) {
4335+
std::string Code = R"(
4336+
class MyClass {
4337+
public:
4338+
bool getField() { return Field; }
4339+
bool setField(bool Val) { Field = Val; }
4340+
4341+
private:
4342+
bool Field;
4343+
};
4344+
4345+
void target() {
4346+
MyClass MyObj;
4347+
MyObj.setField(true);
4348+
bool Foo = MyObj.getField();
4349+
// [[p]]
4350+
}
4351+
)";
4352+
runDataflow(Code,
4353+
[](llvm::ArrayRef<
4354+
std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
4355+
Results,
4356+
ASTContext &ASTCtx) {
4357+
ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
4358+
const Environment &Env = Results[0].second.Env;
4359+
4360+
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
4361+
ASSERT_THAT(FooDecl, NotNull());
4362+
4363+
auto &FooVal =
4364+
*cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
4365+
EXPECT_TRUE(Env.flowConditionImplies(FooVal));
4366+
},
4367+
{/*.ApplyBuiltinTransfer=*/true,
4368+
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
4369+
}
4370+
42324371
} // namespace

0 commit comments

Comments
 (0)