Author: Wei Yi Tee Date: 2022-06-09T01:29:16+02:00 New Revision: a1b2b7d9790b8a150d798fcc672387607986dbe0
URL: https://github.com/llvm/llvm-project/commit/a1b2b7d9790b8a150d798fcc672387607986dbe0 DIFF: https://github.com/llvm/llvm-project/commit/a1b2b7d9790b8a150d798fcc672387607986dbe0.diff LOG: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue This patch precedes a future patch to make PointeeLoc for PointerValue possibly empty (for nullptr), by using a pointer instead of a reference type. ReferenceValue should maintain a non-empty PointeeLoc reference. Reviewed By: gribozavr2 Differential Revision: https://reviews.llvm.org/D127312 Added: Modified: clang/include/clang/Analysis/FlowSensitive/Value.h clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp clang/unittests/Analysis/FlowSensitive/TransferTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Analysis/FlowSensitive/Value.h b/clang/include/clang/Analysis/FlowSensitive/Value.h index 1aedd8a300dd5..3ac4b8c60996b 100644 --- a/clang/include/clang/Analysis/FlowSensitive/Value.h +++ b/clang/include/clang/Analysis/FlowSensitive/Value.h @@ -166,15 +166,15 @@ class IntegerValue : public Value { } }; -/// Base class for values that refer to storage locations. -class IndirectionValue : public Value { +/// Models a dereferenced pointer. For example, a reference in C++ or an lvalue +/// in C. +class ReferenceValue final : public Value { public: - /// Constructs a value that refers to `PointeeLoc`. - explicit IndirectionValue(Kind ValueKind, StorageLocation &PointeeLoc) - : Value(ValueKind), PointeeLoc(PointeeLoc) {} + explicit ReferenceValue(StorageLocation &PointeeLoc) + : Value(Kind::Reference), PointeeLoc(PointeeLoc) {} static bool classof(const Value *Val) { - return Val->getKind() == Kind::Reference || Val->getKind() == Kind::Pointer; + return Val->getKind() == Kind::Reference; } StorageLocation &getPointeeLoc() const { return PointeeLoc; } @@ -183,27 +183,20 @@ class IndirectionValue : public Value { StorageLocation &PointeeLoc; }; -/// Models a dereferenced pointer. For example, a reference in C++ or an lvalue -/// in C. -class ReferenceValue final : public IndirectionValue { -public: - explicit ReferenceValue(StorageLocation &PointeeLoc) - : IndirectionValue(Kind::Reference, PointeeLoc) {} - - static bool classof(const Value *Val) { - return Val->getKind() == Kind::Reference; - } -}; - /// Models a symbolic pointer. Specifically, any value of type `T*`. -class PointerValue final : public IndirectionValue { +class PointerValue final : public Value { public: explicit PointerValue(StorageLocation &PointeeLoc) - : IndirectionValue(Kind::Pointer, PointeeLoc) {} + : Value(Kind::Pointer), PointeeLoc(PointeeLoc) {} static bool classof(const Value *Val) { return Val->getKind() == Kind::Pointer; } + + StorageLocation &getPointeeLoc() const { return PointeeLoc; } + +private: + StorageLocation &PointeeLoc; }; /// Models a value of `struct` or `class` type, with a flat map of fields to diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 13fef0d4286cf..033ef6afbeb2f 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -50,22 +50,25 @@ llvm::DenseMap<K, V> intersectDenseMaps(const llvm::DenseMap<K, V> &Map1, return Result; } +static bool areEquivalentIndirectionValues(Value *Val1, Value *Val2) { + if (auto *IndVal1 = dyn_cast<ReferenceValue>(Val1)) { + auto *IndVal2 = cast<ReferenceValue>(Val2); + return &IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc(); + } + if (auto *IndVal1 = dyn_cast<PointerValue>(Val1)) { + auto *IndVal2 = cast<PointerValue>(Val2); + return &IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc(); + } + return false; +} + /// Returns true if and only if `Val1` is equivalent to `Val2`. static bool equivalentValues(QualType Type, Value *Val1, const Environment &Env1, Value *Val2, const Environment &Env2, Environment::ValueModel &Model) { - if (Val1 == Val2) - return true; - - if (auto *IndVal1 = dyn_cast<IndirectionValue>(Val1)) { - auto *IndVal2 = cast<IndirectionValue>(Val2); - assert(IndVal1->getKind() == IndVal2->getKind()); - if (&IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc()) - return true; - } - - return Model.compareEquivalent(Type, *Val1, Env1, *Val2, Env2); + return Val1 == Val2 || areEquivalentIndirectionValues(Val1, Val2) || + Model.compareEquivalent(Type, *Val1, Env1, *Val2, Env2); } /// Attempts to merge distinct values `Val1` and `Val2` in `Env1` and `Env2`, @@ -89,12 +92,8 @@ static Value *mergeDistinctValues(QualType Type, Value *Val1, } // FIXME: add unit tests that cover this statement. - if (auto *IndVal1 = dyn_cast<IndirectionValue>(Val1)) { - auto *IndVal2 = cast<IndirectionValue>(Val2); - assert(IndVal1->getKind() == IndVal2->getKind()); - if (&IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc()) { - return Val1; - } + if (areEquivalentIndirectionValues(Val1, Val2)) { + return Val1; } // FIXME: Consider destroying `MergedValue` immediately if `ValueModel::merge` diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index a8552161b70d0..5780968d81591 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -122,24 +122,24 @@ TEST_F(TransferTest, IntVarDecl) { // [[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; + 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()); + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); - const StorageLocation *FooLoc = - Env.getStorageLocation(*FooDecl, SkipPast::None); - ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc)); + const StorageLocation *FooLoc = + Env.getStorageLocation(*FooDecl, SkipPast::None); + ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc)); - const Value *FooVal = Env.getValue(*FooLoc); - EXPECT_TRUE(isa_and_nonnull<IntegerValue>(FooVal)); - }); + const Value *FooVal = Env.getValue(*FooLoc); + EXPECT_TRUE(isa_and_nonnull<IntegerValue>(FooVal)); + }); } TEST_F(TransferTest, StructVarDecl) { @@ -293,29 +293,29 @@ TEST_F(TransferTest, ReferenceVarDecl) { // [[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; + 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()); + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); - const StorageLocation *FooLoc = - Env.getStorageLocation(*FooDecl, SkipPast::None); - ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc)); + const StorageLocation *FooLoc = + Env.getStorageLocation(*FooDecl, SkipPast::None); + ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc)); - const ReferenceValue *FooVal = - cast<ReferenceValue>(Env.getValue(*FooLoc)); - const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc(); - EXPECT_TRUE(isa<AggregateStorageLocation>(&FooPointeeLoc)); + const ReferenceValue *FooVal = + cast<ReferenceValue>(Env.getValue(*FooLoc)); + const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc(); + EXPECT_TRUE(isa<AggregateStorageLocation>(&FooPointeeLoc)); - const Value *FooPointeeVal = Env.getValue(FooPointeeLoc); - EXPECT_TRUE(isa_and_nonnull<StructValue>(FooPointeeVal)); - }); + const Value *FooPointeeVal = Env.getValue(FooPointeeLoc); + EXPECT_TRUE(isa_and_nonnull<StructValue>(FooPointeeVal)); + }); } TEST_F(TransferTest, SelfReferentialReferenceVarDecl) { @@ -3327,23 +3327,23 @@ TEST_F(TransferTest, LoopWithStructReferenceAssignmentConverges) { ASSERT_THAT(LDecl, NotNull()); // Inner. - auto *LVal = dyn_cast<IndirectionValue>( - InnerEnv.getValue(*LDecl, SkipPast::None)); + auto *LVal = + dyn_cast<PointerValue>(InnerEnv.getValue(*LDecl, SkipPast::None)); ASSERT_THAT(LVal, NotNull()); EXPECT_EQ(&LVal->getPointeeLoc(), InnerEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); // Outer. - LVal = dyn_cast<IndirectionValue>( - OuterEnv.getValue(*LDecl, SkipPast::None)); + LVal = + dyn_cast<PointerValue>(OuterEnv.getValue(*LDecl, SkipPast::None)); ASSERT_THAT(LVal, NotNull()); // The loop body may not have been executed, so we should not conclude // that `l` points to `val`. EXPECT_NE(&LVal->getPointeeLoc(), OuterEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); -}); + }); } TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
