[clang] f85a9a6 - [randstruct] Don't allow implicit forward decl to stop struct randomization

[Bill Wendling via cfe-commits]

Author: Bill Wendling
Date: 2023-02-06T14:26:32-08:00
New Revision: f85a9a6452e8f49f9768d66a86434a88a5891614

URL: https://github.com/llvm/llvm-project/commit/f85a9a6452e8f49f9768d66a86434a88a5891614
DIFF: https://github.com/llvm/llvm-project/commit/f85a9a6452e8f49f9768d66a86434a88a5891614.diff

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

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

Reviewed By: MaskRay, nickdesaulniers

Differential Revision: https://reviews.llvm.org/D143300

Added: 
    clang/test/CodeGen/init-randomized-struct-fwd-decl.c

Modified: 
    clang/lib/Sema/SemaDecl.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 51098dd922e4a..222471758b1d0 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -18873,10 +18873,24 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
     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 *TD = dyn_cast<TagDecl>(D))
+          return !TD->isCompleteDefinition();
         return false;
+      }
       QualType FieldType = FD->getType().getDesugaredType(Context);
       if (isa<PointerType>(FieldType)) {
         QualType PointeeType = cast<PointerType>(FieldType)->getPointeeType();
@@ -18890,7 +18904,7 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
     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;

diff  --git a/clang/test/CodeGen/init-randomized-struct-fwd-decl.c b/clang/test/CodeGen/init-randomized-struct-fwd-decl.c
new file mode 100644
index 0000000000000..fcfeb6ae8c313
--- /dev/null
+++ b/clang/test/CodeGen/init-randomized-struct-fwd-decl.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm -frandomize-layout-seed=1234567890abcdef < %s | FileCheck %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.
+
+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);
+};
+
+// CHECK-LABEL: define {{.*}}@t1(
+// CHECK-NOT: getelementptr inbounds %struct.foo, ptr %3, i32 0, i32 0
+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);
+};
+
+// CHECK-LABEL: define {{.*}}@t2(
+// CHECK-NOT: getelementptr inbounds %struct.baz, ptr %3, i32 0, i32 0
+enum qux *t2(struct baz *z) {
+  return z->f();
+}