ymandel created this revision.
ymandel added reviewers: sgatev, gribozavr2.
Herald added subscribers: tschuett, steakhal.
Herald added a project: All.
ymandel updated this revision to Diff 417906.
ymandel added a comment.
ymandel updated this revision to Diff 419430.
ymandel retitled this revision from "[clang][dataflow] Fields bug repro. NOT 
FOR REVIEW." to "[clang][dataflow] Fix handling of base-class fields".
ymandel edited the summary of this revision.
ymandel updated this revision to Diff 419440.
ymandel edited the summary of this revision.
ymandel updated this revision to Diff 419449.
ymandel updated this revision to Diff 419451.
ymandel added a reviewer: xazax.hun.
Herald added a subscriber: rnkovacs.
ymandel published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Added repros for equality-related failures.


ymandel added a comment.

c


ymandel added a comment.

added missing call


ymandel added a comment.

refactor


ymandel added a comment.

tweak


This patch adds support for tracking of non-private base-class fields in 
derived classes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122273

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.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
@@ -1009,6 +1009,59 @@
       });
 }
 
+TEST_F(TransferTest, DerivedBaseMember) {
+  std::string Code = R"(
+    struct A {
+      int Bar;
+    };
+    struct B : public A {
+    };
+
+    void target(B Foo) {
+      int Baz = Foo.Bar;
+      // [[p]]
+    }
+  )";
+  runDataflow(
+      Code, [](llvm::ArrayRef<
+                   std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+                   Results,
+               ASTContext &ASTCtx) {
+        ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+        const Environment &Env = Results[0].second.Env;
+
+        const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+        ASSERT_THAT(FooDecl, NotNull());
+
+        ASSERT_TRUE(FooDecl->getType()->isStructureType());
+        const FieldDecl *BarDecl = nullptr;
+        for (const clang::CXXBaseSpecifier &Base :
+             FooDecl->getType()->getAsCXXRecordDecl()->bases()) {
+          QualType BaseType = Base.getType();
+          ASSERT_TRUE(BaseType->isStructureType());
+
+          for (const FieldDecl *Field : BaseType->getAsRecordDecl()->fields()) {
+            if (Field->getNameAsString() == "Bar") {
+              BarDecl = Field;
+            } else {
+              FAIL() << "Unexpected field: " << Field->getNameAsString();
+            }
+          }
+        }
+        ASSERT_THAT(BarDecl, NotNull());
+
+        const auto *FooLoc = cast<AggregateStorageLocation>(
+            Env.getStorageLocation(*FooDecl, SkipPast::None));
+        const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
+        const auto *BarVal = cast<IntegerValue>(FooVal->getChild(*BarDecl));
+
+        const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+        ASSERT_THAT(BazDecl, NotNull());
+
+        EXPECT_EQ(Env.getValue(*BazDecl, SkipPast::None), BarVal);
+      });
+}
+
 TEST_F(TransferTest, ClassMember) {
   std::string Code = R"(
     class A {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -164,6 +164,39 @@
   return JoinedConstraints;
 }
 
+static void
+getFieldsFromClassHierarchy(QualType Type, bool IgnorePrivateFields,
+                            llvm::DenseSet<const FieldDecl *> &Fields) {
+  if (Type->isIncompleteType() || Type->isDependentType() ||
+      !Type->isRecordType())
+    return;
+
+  for (const FieldDecl *Field : Type->getAsRecordDecl()->fields()) {
+    if (IgnorePrivateFields && (Field->getAccess() == clang::AS_private ||
+                                (Field->getAccess() == clang::AS_none &&
+                                 Type->getAsRecordDecl()->isClass())))
+      continue;
+    Fields.insert(Field);
+  }
+  if (auto *CXXRecord = Type->getAsCXXRecordDecl()) {
+    for (const clang::CXXBaseSpecifier &Base : CXXRecord->bases()) {
+      // Ignore private fields (including default access in C++ classes) in
+      // base classes, because they are not visible in derived classes.
+      getFieldsFromClassHierarchy(Base.getType(), /*IgnorePrivateFields=*/true,
+                                  Fields);
+    }
+  }
+}
+
+/// Gets the set of all fields accesible from the type.
+static llvm::DenseSet<const FieldDecl *>
+getAccessibleObjectFields(QualType Type) {
+  llvm::DenseSet<const FieldDecl *> Fields;
+  // Don't ignore private fields for the class itself, only its super classes.
+  getFieldsFromClassHierarchy(Type, /*IgnorePrivateFields=*/false, Fields);
+  return Fields;
+}
+
 Environment::Environment(DataflowAnalysisContext &DACtx,
                          const DeclContext &DeclCtx)
     : Environment(DACtx) {
@@ -296,7 +329,7 @@
     // FIXME: Explore options to avoid eager initialization of fields as some of
     // them might not be needed for a particular analysis.
     llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;
-    for (const FieldDecl *Field : Type->getAsRecordDecl()->fields()) {
+    for (const FieldDecl *Field : getAccessibleObjectFields(Type)) {
       FieldLocs.insert({Field, &createStorageLocation(Field->getType())});
     }
     return takeOwnership(
@@ -363,7 +396,7 @@
     const QualType Type = AggregateLoc.getType();
     assert(Type->isStructureOrClassType());
 
-    for (const FieldDecl *Field : Type->getAsRecordDecl()->fields()) {
+    for (const FieldDecl *Field : getAccessibleObjectFields(Type)) {
       assert(Field != nullptr);
       StorageLocation &FieldLoc = AggregateLoc.getChild(*Field);
       MemberLocToStruct[&FieldLoc] = std::make_pair(StructVal, Field);
@@ -479,7 +512,7 @@
     // FIXME: Initialize only fields that are accessed in the context that is
     // being analyzed.
     llvm::DenseMap<const ValueDecl *, Value *> FieldValues;
-    for (const FieldDecl *Field : Type->getAsRecordDecl()->fields()) {
+    for (const FieldDecl *Field : getAccessibleObjectFields(Type)) {
       assert(Field != nullptr);
 
       QualType FieldType = Field->getType();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to