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 @@ -329,8 +329,8 @@ llvm::DenseSet &Visited, int Depth, int &CreatedValuesCount); - StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const; - const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const; + StorageLocation *skip(StorageLocation &Loc, SkipPast SP) const; + const StorageLocation *skip(const StorageLocation &Loc, SkipPast SP) const; // `DACtx` is not null and not owned by this object. DataflowAnalysisContext *DACtx; diff --git a/clang/include/clang/Analysis/FlowSensitive/Value.h b/clang/include/clang/Analysis/FlowSensitive/Value.h --- a/clang/include/clang/Analysis/FlowSensitive/Value.h +++ b/clang/include/clang/Analysis/FlowSensitive/Value.h @@ -186,17 +186,17 @@ /// Models a symbolic pointer. Specifically, any value of type `T*`. class PointerValue final : public Value { public: - explicit PointerValue(StorageLocation &PointeeLoc) + explicit PointerValue(StorageLocation *PointeeLoc) : Value(Kind::Pointer), PointeeLoc(PointeeLoc) {} static bool classof(const Value *Val) { return Val->getKind() == Kind::Pointer; } - StorageLocation &getPointeeLoc() const { return PointeeLoc; } + StorageLocation *getPointeeLoc() const { return PointeeLoc; } private: - StorageLocation &PointeeLoc; + 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 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -57,7 +57,7 @@ } if (auto *IndVal1 = dyn_cast(Val1)) { auto *IndVal2 = cast(Val2); - return &IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc(); + return IndVal1->getPointeeLoc() == IndVal2->getPointeeLoc(); } return false; } @@ -348,7 +348,7 @@ StorageLocation *Environment::getStorageLocation(const ValueDecl &D, SkipPast SP) const { auto It = DeclToLoc.find(&D); - return It == DeclToLoc.end() ? nullptr : &skip(*It->second, SP); + return It == DeclToLoc.end() ? nullptr : skip(*It->second, SP); } void Environment::setStorageLocation(const Expr &E, StorageLocation &Loc) { @@ -361,7 +361,7 @@ SkipPast SP) const { // FIXME: Add a test with parens. auto It = ExprToLoc.find(&ignoreCFGOmittedNodes(E)); - return It == ExprToLoc.end() ? nullptr : &skip(*It->second, SP); + return It == ExprToLoc.end() ? nullptr : skip(*It->second, SP); } StorageLocation *Environment::getThisPointeeStorageLocation() const { @@ -485,7 +485,7 @@ setValue(PointeeLoc, *PointeeVal); } - return &takeOwnership(std::make_unique(PointeeLoc)); + return &takeOwnership(std::make_unique(&PointeeLoc)); } if (Type->isStructureOrClassType()) { @@ -514,26 +514,27 @@ return nullptr; } -StorageLocation &Environment::skip(StorageLocation &Loc, SkipPast SP) const { +StorageLocation *Environment::skip(StorageLocation &Loc, SkipPast SP) const { switch (SP) { case SkipPast::None: - return Loc; + return &Loc; case SkipPast::Reference: // References cannot be chained so we only need to skip past one level of // indirection. if (auto *Val = dyn_cast_or_null(getValue(Loc))) - return Val->getReferentLoc(); - return Loc; + return &Val->getReferentLoc(); + return &Loc; case SkipPast::ReferenceThenPointer: - StorageLocation &LocPastRef = skip(Loc, SkipPast::Reference); - if (auto *Val = dyn_cast_or_null(getValue(LocPastRef))) + StorageLocation *LocPastRef = skip(Loc, SkipPast::Reference); + assert(LocPastRef != nullptr); + if (auto *Val = dyn_cast_or_null(getValue(*LocPastRef))) return Val->getPointeeLoc(); return LocPastRef; } llvm_unreachable("bad SkipPast kind"); } -const StorageLocation &Environment::skip(const StorageLocation &Loc, +const StorageLocation *Environment::skip(const StorageLocation &Loc, SkipPast SP) const { return skip(*const_cast(&Loc), SP); } diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -263,10 +263,14 @@ if (SubExprVal == nullptr) break; - auto &Loc = Env.createStorageLocation(*S); - Env.setStorageLocation(*S, Loc); - Env.setValue(Loc, Env.takeOwnership(std::make_unique( - SubExprVal->getPointeeLoc()))); + // If PointeeLoc is null, then we are dereferencing a nullptr, skip + // creating a value for the dereference + if (auto *PointeeLoc = SubExprVal->getPointeeLoc()) { + auto &Loc = Env.createStorageLocation(*S); + Env.setStorageLocation(*S, Loc); + Env.setValue(Loc, Env.takeOwnership( + std::make_unique(*PointeeLoc))); + } break; } case UO_AddrOf: { @@ -280,7 +284,7 @@ auto &PointerLoc = Env.createStorageLocation(*S); auto &PointerVal = - Env.takeOwnership(std::make_unique(*PointeeLoc)); + Env.takeOwnership(std::make_unique(PointeeLoc)); Env.setStorageLocation(*S, PointerLoc); Env.setValue(PointerLoc, PointerVal); break; @@ -310,8 +314,8 @@ auto &Loc = Env.createStorageLocation(*S); Env.setStorageLocation(*S, Loc); - Env.setValue(Loc, Env.takeOwnership( - std::make_unique(*ThisPointeeLoc))); + Env.setValue( + Loc, Env.takeOwnership(std::make_unique(ThisPointeeLoc))); } void VisitMemberExpr(const MemberExpr *S) { 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 @@ -412,8 +412,9 @@ const auto *FooPtrVal = cast(BarReferentVal->getChild(*FooPtrDecl)); - const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc(); - EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull()); + const StorageLocation *FooPtrPointeeLoc = FooPtrVal->getPointeeLoc(); + ASSERT_THAT(FooPtrPointeeLoc, NotNull()); + EXPECT_THAT(Env.getValue(*FooPtrPointeeLoc), IsNull()); const auto *BazRefVal = cast(BarReferentVal->getChild(*BazRefDecl)); @@ -422,8 +423,9 @@ const auto *BazPtrVal = cast(BarReferentVal->getChild(*BazPtrDecl)); - const StorageLocation &BazPtrPointeeLoc = BazPtrVal->getPointeeLoc(); - EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull()); + const StorageLocation *BazPtrPointeeLoc = BazPtrVal->getPointeeLoc(); + ASSERT_THAT(BazPtrPointeeLoc, NotNull()); + EXPECT_THAT(Env.getValue(*BazPtrPointeeLoc), NotNull()); }); } @@ -454,10 +456,10 @@ ASSERT_TRUE(isa_and_nonnull(FooLoc)); const PointerValue *FooVal = cast(Env.getValue(*FooLoc)); - const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc(); - EXPECT_TRUE(isa(&FooPointeeLoc)); + const StorageLocation *FooPointeeLoc = FooVal->getPointeeLoc(); + EXPECT_TRUE(isa_and_nonnull(FooPointeeLoc)); - const Value *FooPointeeVal = Env.getValue(FooPointeeLoc); + const Value *FooPointeeVal = Env.getValue(*FooPointeeLoc); EXPECT_TRUE(isa_and_nonnull(FooPointeeVal)); }); } @@ -554,13 +556,17 @@ const auto *FooLoc = cast( Env.getStorageLocation(*FooDecl, SkipPast::None)); const auto *FooVal = cast(Env.getValue(*FooLoc)); + const auto *FooPointeeLoc = FooVal->getPointeeLoc(); + ASSERT_THAT(FooPointeeLoc, NotNull()); const auto *FooPointeeVal = - cast(Env.getValue(FooVal->getPointeeLoc())); + cast(Env.getValue(*FooPointeeLoc)); const auto *BarVal = cast(FooPointeeVal->getChild(*BarDecl)); + const auto *BarPointeeLoc = BarVal->getPointeeLoc(); + ASSERT_THAT(BarPointeeLoc, NotNull()); const auto *BarPointeeVal = - cast(Env.getValue(BarVal->getPointeeLoc())); + cast(Env.getValue(*BarPointeeLoc)); const auto *FooRefVal = cast(BarPointeeVal->getChild(*FooRefDecl)); @@ -569,8 +575,9 @@ const auto *FooPtrVal = cast(BarPointeeVal->getChild(*FooPtrDecl)); - const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc(); - EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull()); + const StorageLocation *FooPtrPointeeLoc = FooPtrVal->getPointeeLoc(); + ASSERT_THAT(FooPtrPointeeLoc, NotNull()); + EXPECT_THAT(Env.getValue(*FooPtrPointeeLoc), IsNull()); const auto *BazRefVal = cast(BarPointeeVal->getChild(*BazRefDecl)); @@ -579,8 +586,9 @@ const auto *BazPtrVal = cast(BarPointeeVal->getChild(*BazPtrDecl)); - const StorageLocation &BazPtrPointeeLoc = BazPtrVal->getPointeeLoc(); - EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull()); + const StorageLocation *BazPtrPointeeLoc = BazPtrVal->getPointeeLoc(); + ASSERT_THAT(BazPtrPointeeLoc, NotNull()); + EXPECT_THAT(Env.getValue(*BazPtrPointeeLoc), NotNull()); }); } @@ -799,7 +807,9 @@ const auto *BarVal = cast(Env.getValue(*BarDecl, SkipPast::None)); - EXPECT_EQ(Env.getValue(BarVal->getPointeeLoc()), FooVal); + const auto *BarPointeeLoc = BarVal->getPointeeLoc(); + ASSERT_THAT(BarPointeeLoc, NotNull()); + EXPECT_EQ(Env.getValue(*BarPointeeLoc), FooVal); const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz"); ASSERT_THAT(BazDecl, NotNull()); @@ -1004,10 +1014,10 @@ ASSERT_TRUE(isa_and_nonnull(FooLoc)); const PointerValue *FooVal = cast(Env.getValue(*FooLoc)); - const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc(); - EXPECT_TRUE(isa(&FooPointeeLoc)); + const StorageLocation *FooPointeeLoc = FooVal->getPointeeLoc(); + EXPECT_TRUE(isa_and_nonnull(FooPointeeLoc)); - const Value *FooPointeeVal = Env.getValue(FooPointeeLoc); + const Value *FooPointeeVal = Env.getValue(*FooPointeeLoc); EXPECT_TRUE(isa_and_nonnull(FooPointeeVal)); }); } @@ -2240,7 +2250,7 @@ Env.getStorageLocation(*FooDecl, SkipPast::None)); const auto *BarVal = cast(Env.getValue(*BarDecl, SkipPast::None)); - EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc); + EXPECT_EQ(BarVal->getPointeeLoc(), FooLoc); }); } @@ -2269,7 +2279,7 @@ cast(Env.getValue(*FooDecl, SkipPast::None)); const auto *BarVal = cast(Env.getValue(*BarDecl, SkipPast::None)); - EXPECT_EQ(&BarVal->getPointeeLoc(), &FooVal->getPointeeLoc()); + EXPECT_EQ(BarVal->getPointeeLoc(), FooVal->getPointeeLoc()); }); } @@ -2299,7 +2309,7 @@ cast(Env.getValue(*FooDecl, SkipPast::None)); const auto *BarVal = cast(Env.getValue(*BarDecl, SkipPast::None)); - EXPECT_EQ(&BarVal->getReferentLoc(), &FooVal->getPointeeLoc()); + EXPECT_EQ(&BarVal->getReferentLoc(), FooVal->getPointeeLoc()); }); } @@ -2369,8 +2379,10 @@ const auto *FooVal = cast(Env.getValue(*FooDecl, SkipPast::None)); + const auto *FooPointeeLoc = FooVal->getPointeeLoc(); + ASSERT_THAT(FooPointeeLoc, NotNull()); const auto *FooPointeeVal = - cast(Env.getValue(FooVal->getPointeeLoc())); + cast(Env.getValue(*FooPointeeLoc)); const auto *BarVal = dyn_cast_or_null( Env.getValue(*BarDecl, SkipPast::None)); @@ -3331,7 +3343,7 @@ dyn_cast(InnerEnv.getValue(*LDecl, SkipPast::None)); ASSERT_THAT(LVal, NotNull()); - EXPECT_EQ(&LVal->getPointeeLoc(), + EXPECT_EQ(LVal->getPointeeLoc(), InnerEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); // Outer. @@ -3341,7 +3353,7 @@ // The loop body may not have been executed, so we should not conclude // that `l` points to `val`. - EXPECT_NE(&LVal->getPointeeLoc(), + EXPECT_NE(LVal->getPointeeLoc(), OuterEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); }); } diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp --- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp @@ -676,7 +676,7 @@ Env.getStorageLocation(*FooDecl, SkipPast::None)); const auto *BarVal = cast(Env.getValue(*BarDecl, SkipPast::None)); - EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc); + EXPECT_EQ(BarVal->getPointeeLoc(), FooLoc); }); }