This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd862f66221de: [clang][dataflow] Treat unions as structs.
(authored by merrymeerkat).
Changed prior to commit:
https://reviews.llvm.org/D140696?vs=485693&id=486025#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140696/new/
https://reviews.llvm.org/D140696
Files:
clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ 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<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ const auto *ThisLoc = dyn_cast<AggregateStorageLocation>(
+ Env.getThisPointeeStorageLocation());
+ ASSERT_THAT(ThisLoc, NotNull());
+
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
+
+ const auto *FooLoc =
+ cast<ScalarStorageLocation>(&ThisLoc->getChild(*FooDecl));
+ ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
+
+ const Value *FooVal = Env.getValue(*FooLoc);
+ ASSERT_TRUE(isa_and_nonnull<IntegerValue>(FooVal));
+
+ const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ ASSERT_THAT(BarDecl, NotNull());
+
+ const auto *BarLoc =
+ cast<ScalarStorageLocation>(&ThisLoc->getChild(*BarDecl));
+ ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(BarLoc));
+
+ const Value *BarVal = Env.getValue(*BarLoc);
+ ASSERT_TRUE(isa_and_nonnull<IntegerValue>(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<AggregateStorageLocation>(
Env.getStorageLocation(*BazDecl, SkipPast::None));
ASSERT_THAT(BazLoc, NotNull());
+ ASSERT_THAT(Env.getValue(*BazLoc), NotNull());
+
+ const auto *BazVal = cast<StructValue>(Env.getValue(*BazLoc));
+ const auto *FooValFromBazVal = cast<IntegerValue>(BazVal->getChild(*FooDecl));
+ const auto *FooValFromBazLoc = cast<IntegerValue>(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<ScalarStorageLocation>(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);
});
}
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -487,10 +487,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);
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -235,14 +235,12 @@
if (Parent->isLambda())
MethodDecl = dyn_cast<CXXMethodDecl>(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 @@
auto &AggregateLoc = *cast<AggregateStorageLocation>(&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 @@
return &takeOwnership(std::make_unique<PointerValue>(PointeeLoc));
}
- if (Type->isStructureOrClassType()) {
+ if (Type->isStructureOrClassType() || Type->isUnionType()) {
CreatedValuesCount++;
// FIXME: Initialize only fields that are accessed in the context that is
// being analyzed.
Index: clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
+++ 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)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
