diff --git a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h --- a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h +++ b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h @@ -65,6 +65,9 @@ /// struct with public members. The child map is flat, so when used for a struct /// or class type, all accessible members of base struct and class types are /// directly accesible as children of this location. +/// FIXME: Currently, the storage location of unions is modelled the same way as +/// that of structs or classes. Eventually, we need to change this modelling so +/// that all of the members of a given union have the same storage location. class AggregateStorageLocation final : public StorageLocation { public: explicit AggregateStorageLocation(QualType Type) 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 @@ -223,14 +223,12 @@ if (Parent->isLambda()) MethodDecl = dyn_cast(Parent->getDeclContext()); + // FIXME: Initialize the ThisPointeeLoc of lambdas too. if (MethodDecl && !MethodDecl->isStatic()) { QualType ThisPointeeType = MethodDecl->getThisObjectType(); - // FIXME: Add support for union types. - if (!ThisPointeeType->isUnionType()) { - ThisPointeeLoc = &createStorageLocation(ThisPointeeType); - if (Value *ThisPointeeVal = createValue(ThisPointeeType)) - setValue(*ThisPointeeLoc, *ThisPointeeVal); - } + ThisPointeeLoc = &createStorageLocation(ThisPointeeType); + if (Value *ThisPointeeVal = createValue(ThisPointeeType)) + setValue(*ThisPointeeLoc, *ThisPointeeVal); } } } @@ -549,7 +547,7 @@ auto &AggregateLoc = *cast(&Loc); const QualType Type = AggregateLoc.getType(); - assert(Type->isStructureOrClassType()); + assert(Type->isStructureOrClassType() || Type->isUnionType()); for (const FieldDecl *Field : getObjectFields(Type)) { assert(Field != nullptr); @@ -663,7 +661,7 @@ return &takeOwnership(std::make_unique(PointeeLoc)); } - if (Type->isStructureOrClassType()) { + if (Type->isStructureOrClassType() || Type->isUnionType()) { CreatedValuesCount++; // FIXME: Initialize only fields that are accessed in the context that is // being analyzed. 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 @@ -480,10 +480,6 @@ if (BaseLoc == nullptr) return; - // FIXME: Add support for union types. - if (BaseLoc->getType()->isUnionType()) - return; - auto &MemberLoc = BaseLoc->getChild(*Member); if (MemberLoc.getType()->isReferenceType()) { Env.setStorageLocation(*S, MemberLoc); 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 @@ -1518,6 +1518,50 @@ }); } +TEST(TransferTest, UnionThisMember) { + std::string Code = R"( + union A { + int Foo; + int Bar; + + void target() { + (void)0; // [[p]] + } + }; + )"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + const auto *ThisLoc = dyn_cast( + Env.getThisPointeeStorageLocation()); + ASSERT_THAT(ThisLoc, NotNull()); + + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const auto *FooLoc = + cast(&ThisLoc->getChild(*FooDecl)); + ASSERT_TRUE(isa_and_nonnull(FooLoc)); + + const Value *FooVal = Env.getValue(*FooLoc); + ASSERT_TRUE(isa_and_nonnull(FooVal)); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const auto *BarLoc = + cast(&ThisLoc->getChild(*BarDecl)); + ASSERT_TRUE(isa_and_nonnull(BarLoc)); + + const Value *BarVal = Env.getValue(*BarLoc); + ASSERT_TRUE(isa_and_nonnull(BarVal)); + }); +} + TEST(TransferTest, StructThisInLambda) { std::string ThisCaptureCode = R"( struct A { @@ -2537,12 +2581,34 @@ ASSERT_THAT(BazDecl, NotNull()); ASSERT_TRUE(BazDecl->getType()->isUnionType()); + auto BazFields = BazDecl->getType()->getAsRecordDecl()->fields(); + FieldDecl *FooDecl = nullptr; + for (FieldDecl *Field : BazFields) { + if (Field->getNameAsString() == "Foo") { + FooDecl = Field; + } else { + FAIL() << "Unexpected field: " << Field->getNameAsString(); + } + } + ASSERT_THAT(FooDecl, NotNull()); + const auto *BazLoc = dyn_cast_or_null( Env.getStorageLocation(*BazDecl, SkipPast::None)); ASSERT_THAT(BazLoc, NotNull()); + ASSERT_THAT(Env.getValue(*BazLoc), NotNull()); + + const auto *BazVal = cast(Env.getValue(*BazLoc)); + const auto *FooValFromBazVal = cast(BazVal->getChild(*FooDecl)); + const auto *FooValFromBazLoc = cast(Env.getValue(BazLoc->getChild(*FooDecl))); + EXPECT_EQ(FooValFromBazLoc, FooValFromBazVal); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + const auto *BarLoc = Env.getStorageLocation(*BarDecl, SkipPast::None); + ASSERT_TRUE(isa_and_nonnull(BarLoc)); - // FIXME: Add support for union types. - EXPECT_THAT(Env.getValue(*BazLoc), IsNull()); + EXPECT_EQ(Env.getValue(*BarLoc), FooValFromBazVal); + EXPECT_EQ(Env.getValue(*BarLoc), FooValFromBazLoc); }); }