[clang] d862f66 - [clang][dataflow] Treat unions as structs.
Dani Ferreira Franco Moura via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 3 10:36:32 PST 2023
Author: Dani Ferreira Franco Moura
Date: 2023-01-03T18:36:24Z
New Revision: d862f66221de1463ee7c92fe2e78445edb30a601
URL: https://github.com/llvm/llvm-project/commit/d862f66221de1463ee7c92fe2e78445edb30a601
DIFF: https://github.com/llvm/llvm-project/commit/d862f66221de1463ee7c92fe2e78445edb30a601.diff
LOG: [clang][dataflow] Treat unions as structs.
This is a straightfoward way to handle unions in dataflow analysis. Without this change, nullability verification crashes on files that contain unions.
Reviewed By: gribozavr2, ymandel
Differential Revision: https://reviews.llvm.org/D140696
Added:
Modified:
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
Removed:
################################################################################
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<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 @@ void Environment::setValue(const StorageLocation &Loc, Value &Val) {
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 @@ Value *Environment::createValueUnlessSelfReferential(
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.
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<TransferVisitor> {
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<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 @@ 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<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);
});
}
More information about the cfe-commits
mailing list