[clang] [Clang] Handle structs with inner structs and no fields (PR #89126)

Bill Wendling via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 19 12:34:58 PDT 2024


https://github.com/bwendling updated https://github.com/llvm/llvm-project/pull/89126

>From 36ddb5811f11a1f6968705005713f34713026dbb Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Wed, 17 Apr 2024 12:23:02 -0700
Subject: [PATCH 1/8] [Clang] Handle structs with inner structs and no fields

A struct that declares an inner struct, but no fields, won't have a
field count. So getting the offset of the inner struct fails. This
happens in both C and C++:

  struct foo {
    struct bar {
      int Quantizermatrix[];
    };
  };

Here 'struct foo' has no fields.
---
 clang/lib/CodeGen/CGBuiltin.cpp               | 15 ++++++++++++-
 clang/test/CodeGen/attr-counted-by-pr88931.c  | 22 +++++++++++++++++++
 .../test/CodeGen/attr-counted-by-pr88931.cpp  | 21 ++++++++++++++++++
 3 files changed, 57 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/attr-counted-by-pr88931.c
 create mode 100644 clang/test/CodeGen/attr-counted-by-pr88931.cpp

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index a05874e63c73c2..bee12c4469e7f7 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -829,6 +829,9 @@ const FieldDecl *CodeGenFunction::FindFlexibleArrayMemberField(
   unsigned FieldNo = 0;
   bool IsUnion = RD->isUnion();
 
+  if (RD->isImplicit())
+    return nullptr;
+
   for (const Decl *D : RD->decls()) {
     if (const auto *Field = dyn_cast<FieldDecl>(D);
         Field && (Name.empty() || Field->getNameAsString() == Name) &&
@@ -844,7 +847,17 @@ const FieldDecl *CodeGenFunction::FindFlexibleArrayMemberField(
       if (const FieldDecl *Field =
               FindFlexibleArrayMemberField(Ctx, Record, Name, Offset)) {
         const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
-        Offset += Layout.getFieldOffset(FieldNo);
+        if (Layout.getFieldCount()) {
+          // A struct that holds only an inner struct won't have any fields. E.g.
+          //
+          //     struct foo {
+          //         struct bar {
+          //             int count;
+          //             int array[];
+          //         };
+          //     };
+          Offset += Layout.getFieldOffset(FieldNo);
+        }
         return Field;
       }
 
diff --git a/clang/test/CodeGen/attr-counted-by-pr88931.c b/clang/test/CodeGen/attr-counted-by-pr88931.c
new file mode 100644
index 00000000000000..baa1bbc8397456
--- /dev/null
+++ b/clang/test/CodeGen/attr-counted-by-pr88931.c
@@ -0,0 +1,22 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 4
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O2 -Wno-missing-declarations -emit-llvm -o - %s | FileCheck %s
+
+struct foo {
+  struct bar {
+    int count;
+    int array[] __attribute__((counted_by(count)));
+  };
+};
+
+void init(void * __attribute__((pass_dynamic_object_size(0))));
+
+// CHECK-LABEL: define dso_local void @test1(
+// CHECK-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ARRAY:%.*]] = getelementptr inbounds i8, ptr [[P]], i64 4
+// CHECK-NEXT:    tail call void @init(ptr noundef nonnull [[ARRAY]], i64 noundef -1) #[[ATTR2:[0-9]+]]
+// CHECK-NEXT:    ret void
+//
+void test1(struct bar *p) {
+  init(p->array);
+}
diff --git a/clang/test/CodeGen/attr-counted-by-pr88931.cpp b/clang/test/CodeGen/attr-counted-by-pr88931.cpp
new file mode 100644
index 00000000000000..2a8cc1d07e50d9
--- /dev/null
+++ b/clang/test/CodeGen/attr-counted-by-pr88931.cpp
@@ -0,0 +1,21 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 4
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O2 -Wall -emit-llvm -o - %s | FileCheck %s
+
+struct foo {
+  struct bar {
+    int array[];
+    bar();
+  };
+};
+
+void init(void * __attribute__((pass_dynamic_object_size(0))));
+
+// CHECK-LABEL: define dso_local void @_ZN3foo3barC1Ev(
+// CHECK-SAME: ptr noundef nonnull align 4 dereferenceable(1) [[THIS:%.*]]) unnamed_addr #[[ATTR0:[0-9]+]] align 2 {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    tail call void @_Z4initPvU25pass_dynamic_object_size0(ptr noundef nonnull [[THIS]], i64 noundef -1) #[[ATTR2:[0-9]+]]
+// CHECK-NEXT:    ret void
+//
+foo::bar::bar() {
+  init(array);
+}

>From fbb22e6aa21961763c3eb1ca45d44e6c8add1ceb Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Wed, 17 Apr 2024 12:32:27 -0700
Subject: [PATCH 2/8] Reformat

---
 clang/lib/CodeGen/CGBuiltin.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index bee12c4469e7f7..e7d2b84da8bc4d 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -848,7 +848,8 @@ const FieldDecl *CodeGenFunction::FindFlexibleArrayMemberField(
               FindFlexibleArrayMemberField(Ctx, Record, Name, Offset)) {
         const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
         if (Layout.getFieldCount()) {
-          // A struct that holds only an inner struct won't have any fields. E.g.
+          // A struct that holds only an inner struct won't have any fields.
+          // E.g.
           //
           //     struct foo {
           //         struct bar {

>From 4c4046f638a4b02f2fcea8a99b37c58a4eb8aba4 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Thu, 18 Apr 2024 13:52:58 -0700
Subject: [PATCH 3/8] Improve calculation by focusing only on FieldDecls.

---
 clang/lib/CodeGen/CGBuiltin.cpp | 34 +++++++++++----------------------
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index e7d2b84da8bc4d..fe8070f9fd2086 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -826,43 +826,31 @@ const FieldDecl *CodeGenFunction::FindFlexibleArrayMemberField(
     ASTContext &Ctx, const RecordDecl *RD, StringRef Name, uint64_t &Offset) {
   const LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel =
       getLangOpts().getStrictFlexArraysLevel();
-  unsigned FieldNo = 0;
-  bool IsUnion = RD->isUnion();
+  uint32_t FieldNo = 0;
 
   if (RD->isImplicit())
     return nullptr;
 
-  for (const Decl *D : RD->decls()) {
-    if (const auto *Field = dyn_cast<FieldDecl>(D);
-        Field && (Name.empty() || Field->getNameAsString() == Name) &&
+  for (const FieldDecl *FD : RD->fields()) {
+    if ((Name.empty() || FD->getNameAsString() == Name) &&
         Decl::isFlexibleArrayMemberLike(
-            Ctx, Field, Field->getType(), StrictFlexArraysLevel,
+            Ctx, FD, FD->getType(), StrictFlexArraysLevel,
             /*IgnoreTemplateOrMacroSubstitution=*/true)) {
       const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
       Offset += Layout.getFieldOffset(FieldNo);
-      return Field;
+      return FD;
     }
 
-    if (const auto *Record = dyn_cast<RecordDecl>(D))
-      if (const FieldDecl *Field =
-              FindFlexibleArrayMemberField(Ctx, Record, Name, Offset)) {
+    QualType Ty = FD->getType();
+    if (!Ty->isPointerType() && (Ty->isStructureType() || Ty->isUnionType()))
+      if (const FieldDecl *Field = FindFlexibleArrayMemberField(
+              Ctx, Ty->getAsRecordDecl(), Name, Offset)) {
         const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
-        if (Layout.getFieldCount()) {
-          // A struct that holds only an inner struct won't have any fields.
-          // E.g.
-          //
-          //     struct foo {
-          //         struct bar {
-          //             int count;
-          //             int array[];
-          //         };
-          //     };
-          Offset += Layout.getFieldOffset(FieldNo);
-        }
+        Offset += Layout.getFieldOffset(FieldNo);
         return Field;
       }
 
-    if (!IsUnion && isa<FieldDecl>(D))
+    if (!RD->isUnion())
       ++FieldNo;
   }
 

>From 350550b7e1093dbbfbf4bbffcc92845965f3b6d7 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Thu, 18 Apr 2024 14:18:35 -0700
Subject: [PATCH 4/8] Simplify

---
 clang/lib/CodeGen/CGBuiltin.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index fe8070f9fd2086..a0c02aefbcc429 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -842,7 +842,7 @@ const FieldDecl *CodeGenFunction::FindFlexibleArrayMemberField(
     }
 
     QualType Ty = FD->getType();
-    if (!Ty->isPointerType() && (Ty->isStructureType() || Ty->isUnionType()))
+    if (Ty->isRecordType()) {
       if (const FieldDecl *Field = FindFlexibleArrayMemberField(
               Ctx, Ty->getAsRecordDecl(), Name, Offset)) {
         const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);

>From d3eda51734c64064c0e1ed280cb61739015657c7 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Thu, 18 Apr 2024 14:25:04 -0700
Subject: [PATCH 5/8] Fix small error

---
 clang/lib/CodeGen/CGBuiltin.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index a0c02aefbcc429..4319501035e257 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -849,6 +849,7 @@ const FieldDecl *CodeGenFunction::FindFlexibleArrayMemberField(
         Offset += Layout.getFieldOffset(FieldNo);
         return Field;
       }
+    }
 
     if (!RD->isUnion())
       ++FieldNo;

>From dad1c32efa5e02ba5aca762da1aaa9a19da167d6 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Thu, 18 Apr 2024 14:25:14 -0700
Subject: [PATCH 6/8] Improve testcase.

---
 clang/test/CodeGen/attr-counted-by-pr88931.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/test/CodeGen/attr-counted-by-pr88931.c b/clang/test/CodeGen/attr-counted-by-pr88931.c
index baa1bbc8397456..8692a0281f0a17 100644
--- a/clang/test/CodeGen/attr-counted-by-pr88931.c
+++ b/clang/test/CodeGen/attr-counted-by-pr88931.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O2 -Wno-missing-declarations -emit-llvm -o - %s | FileCheck %s
 
 struct foo {
+  int x,y,z;
   struct bar {
     int count;
     int array[] __attribute__((counted_by(count)));

>From 1613ce6ed49880cceb0f063d09206ace7feaa5f8 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Thu, 18 Apr 2024 14:29:25 -0700
Subject: [PATCH 7/8] Add new testcase

---
 clang/test/CodeGen/attr-counted-by-pr88931.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/clang/test/CodeGen/attr-counted-by-pr88931.c b/clang/test/CodeGen/attr-counted-by-pr88931.c
index 8692a0281f0a17..cabc37eac767c9 100644
--- a/clang/test/CodeGen/attr-counted-by-pr88931.c
+++ b/clang/test/CodeGen/attr-counted-by-pr88931.c
@@ -21,3 +21,20 @@ void init(void * __attribute__((pass_dynamic_object_size(0))));
 void test1(struct bar *p) {
   init(p->array);
 }
+
+struct mux {
+  int count;
+  int array[] __attribute__((counted_by(count)));
+};
+
+struct bux { struct mux x; };
+
+// CHECK-LABEL: define dso_local void @test2(
+// CHECK-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    tail call void @init(ptr noundef [[P]], i64 noundef -1) #[[ATTR2]]
+// CHECK-NEXT:    ret void
+//
+void test2(struct foo *p) {
+  init(p);
+}

>From f9b3bf3e4459b27a350095307ac180be3f240abd Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Fri, 19 Apr 2024 12:34:42 -0700
Subject: [PATCH 8/8] Use correct struct.

---
 clang/test/CodeGen/attr-counted-by-pr88931.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/CodeGen/attr-counted-by-pr88931.c b/clang/test/CodeGen/attr-counted-by-pr88931.c
index cabc37eac767c9..cc3d751c7c6d83 100644
--- a/clang/test/CodeGen/attr-counted-by-pr88931.c
+++ b/clang/test/CodeGen/attr-counted-by-pr88931.c
@@ -35,6 +35,6 @@ struct bux { struct mux x; };
 // CHECK-NEXT:    tail call void @init(ptr noundef [[P]], i64 noundef -1) #[[ATTR2]]
 // CHECK-NEXT:    ret void
 //
-void test2(struct foo *p) {
+void test2(struct bux *p) {
   init(p);
 }



More information about the cfe-commits mailing list