[PATCH] D143300: [randstruct] Don't allow implicit forward decl to stop struct randomization

[Bill Wendling via Phabricator via cfe-commits]

void created this revision.
void added reviewers: aaron.ballman, MaskRay, nickdesaulniers.
Herald added a project: All.
void requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

If a struct/enum type used in a record doesn't have a forward decl /
def, an implicit one is injected into the struct. This stops clang from
randomizing the structure in some situations---i.e. when the struct
contains only function pointers. So we accept forward decls so they
don't prevent randomization.

Fixes 60349


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143300

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/init-randomized-struct-fwd-decl.c


Index: clang/test/Sema/init-randomized-struct-fwd-decl.c
===================================================================
--- /dev/null
+++ clang/test/Sema/init-randomized-struct-fwd-decl.c
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm -frandomize-layout-seed=1234567890abcdef < %s | FileCheck %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm -frandomize-layout-seed=1234567890abcdef -DFORWARD_DECL < %s | FileCheck -check-prefix=FWD-DECL %s
+// PR60349
+
+// Clang will add a forward declaration of "struct bar" and "enum qux" to the
+// structures. This shouldn't prevent these structures from being randomized.
+// So the 'f' element shouldn't be at the start of the structure anymore.
+
+#ifdef FORWARD_DECL
+struct bar;
+enum qux;
+#endif
+
+struct foo {
+        struct bar *(*f)(void);
+        struct bar *(*g)(void);
+        struct bar *(*h)(void);
+        struct bar *(*i)(void);
+        struct bar *(*j)(void);
+        struct bar *(*k)(void);
+} __attribute__((randomize_layout));
+
+// CHECK-LABEL: t1
+// CHECK: getelementptr inbounds %struct.foo, ptr %0, i32 0, i32 2
+
+// FWD-DECL-LABEL: t1
+// FWD-DECL: getelementptr inbounds %struct.foo, ptr %0, i32 0, i32 2
+struct bar *t1(struct foo *z) {
+  return z->f();
+}
+
+struct baz {
+        enum qux *(*f)(void);
+        enum qux *(*g)(void);
+        enum qux *(*h)(void);
+        enum qux *(*i)(void);
+        enum qux *(*j)(void);
+        enum qux *(*k)(void);
+} __attribute__((randomize_layout));
+
+// CHECK-LABEL: t2
+// CHECK: getelementptr inbounds %struct.baz, ptr %0, i32 0, i32 4
+
+// FWD-DECL-LABEL: t2
+// FWD-DECL: getelementptr inbounds %struct.baz, ptr %0, i32 0, i32 4
+enum qux *t2(struct baz *z) {
+  return z->f();
+}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -18874,10 +18874,24 @@
     ProcessDeclAttributeList(S, Record, Attrs);
 
     // Check to see if a FieldDecl is a pointer to a function.
-    auto IsFunctionPointer = [&](const Decl *D) {
+    auto IsFunctionPointerOrForwardDecl = [&](const Decl *D) {
       const FieldDecl *FD = dyn_cast<FieldDecl>(D);
-      if (!FD)
+      if (!FD) {
+        // Check whether this is a forward declaration that was inserted by
+        // Clang. This happens when a non-forward declared / defined type is
+        // used, e.g.:
+        //
+        //   struct foo {
+        //     struct bar *(*f)();
+        //     struct bar *(*g)();
+        //   };
+        //
+        // "struct bar" shows up in the decl AST as a "RecordDecl" with an
+        // incomplete definition.
+        if (const auto *RD = dyn_cast<RecordDecl>(D))
+          return !RD->isCompleteDefinition();
         return false;
+      }
       QualType FieldType = FD->getType().getDesugaredType(Context);
       if (isa<PointerType>(FieldType)) {
         QualType PointeeType = cast<PointerType>(FieldType)->getPointeeType();
@@ -18891,7 +18905,7 @@
     if (!getLangOpts().CPlusPlus &&
         (Record->hasAttr<RandomizeLayoutAttr>() ||
          (!Record->hasAttr<NoRandomizeLayoutAttr>() &&
-          llvm::all_of(Record->decls(), IsFunctionPointer))) &&
+          llvm::all_of(Record->decls(), IsFunctionPointerOrForwardDecl))) &&
         !Record->isUnion() && !getLangOpts().RandstructSeed.empty() &&
         !Record->isRandomized()) {
       SmallVector<Decl *, 32> NewDeclOrdering;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D143300.494757.patch
Type: text/x-patch
Size: 3495 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230203/74ab1540/attachment-0001.bin>