diff --git a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h index fdfd03129b81e..f7ea7eb174c54 100644 --- a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h +++ b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h @@ -65,6 +65,9 @@ class ScalarStorageLocation final : public StorageLocation { /// 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 index b8e3e93390602..1a561b31aae9a 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -235,14 +235,12 @@ Environment::Environment(DataflowAnalysisContext &DACtx, 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); } } @@ -570,7 +568,7 @@ void Environment::setValue(const StorageLocation &Loc, Value &Val) { 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); @@ -684,7 +682,7 @@ Value *Environment::createValueUnlessSelfReferential( 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 index 336de81b06538..32e9d6f12e44e 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -487,10 +487,6 @@ class TransferVisitor : public ConstStmtVisitor { 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 index b5e10b24fffb5..a9b1f42f20360 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -1518,6 +1518,50 @@ TEST(TransferTest, ClassThisMember) { }); } +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 @@ TEST(TransferTest, AssignToUnionMember) { 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); }); }