void updated this revision to Diff 494772. void added a comment. Move test and improve checks.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143300/new/ https://reviews.llvm.org/D143300 Files: clang/lib/Sema/SemaDecl.cpp clang/test/CodeGen/init-randomized-struct-fwd-decl.c Index: clang/test/CodeGen/init-randomized-struct-fwd-decl.c =================================================================== --- /dev/null +++ clang/test/CodeGen/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: define {{.*}}@t1 +// CHECK: getelementptr inbounds %struct.foo, ptr %0, i32 0, i32 2 + +// FWD-DECL-LABEL: define {{.*}}@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: define {{.*}}@t2 +// CHECK: getelementptr inbounds %struct.baz, ptr %0, i32 0, i32 4 + +// FWD-DECL-LABEL: define {{.*}}@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;
Index: clang/test/CodeGen/init-randomized-struct-fwd-decl.c =================================================================== --- /dev/null +++ clang/test/CodeGen/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: define {{.*}}@t1 +// CHECK: getelementptr inbounds %struct.foo, ptr %0, i32 0, i32 2 + +// FWD-DECL-LABEL: define {{.*}}@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: define {{.*}}@t2 +// CHECK: getelementptr inbounds %struct.baz, ptr %0, i32 0, i32 4 + +// FWD-DECL-LABEL: define {{.*}}@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;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits