diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h --- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h @@ -137,22 +137,6 @@ 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 @@ llvm::DenseMap DeclToLoc; llvm::DenseMap 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 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -380,7 +380,9 @@ // 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 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -155,8 +155,9 @@ 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 @@ 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 @@ // 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(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 @@ 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 different - // 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 different 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 @@ if (ReturnLoc != Other.ReturnLoc) return false; + if (ThisPointeeLoc != Other.ThisPointeeLoc) + return false; + if (DeclToLoc != Other.DeclToLoc) return false; @@ -294,12 +303,14 @@ 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::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 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -4229,4 +4229,143 @@ /*.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>> + 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(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>> + 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(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>> + 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(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>> + 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(Env.getValue(*FooDecl, SkipPast::None)); + EXPECT_TRUE(Env.flowConditionImplies(FooVal)); + }, + {/*.ApplyBuiltinTransfer=*/true, + /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); +} + } // namespace