Skip to content

Commit 7fed09b

Browse files
bwendlingtru
authored andcommitted
[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 (cherry picked from commit f85a9a6)
1 parent 055ee22 commit 7fed09b

File tree

2 files changed

+53
-3
lines changed

2 files changed

+53
-3
lines changed

clang/lib/Sema/SemaDecl.cpp

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18871,10 +18871,24 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
1887118871
ProcessDeclAttributeList(S, Record, Attrs);
1887218872

1887318873
// Check to see if a FieldDecl is a pointer to a function.
18874-
auto IsFunctionPointer = [&](const Decl *D) {
18874+
auto IsFunctionPointerOrForwardDecl = [&](const Decl *D) {
1887518875
const FieldDecl *FD = dyn_cast<FieldDecl>(D);
18876-
if (!FD)
18876+
if (!FD) {
18877+
// Check whether this is a forward declaration that was inserted by
18878+
// Clang. This happens when a non-forward declared / defined type is
18879+
// used, e.g.:
18880+
//
18881+
// struct foo {
18882+
// struct bar *(*f)();
18883+
// struct bar *(*g)();
18884+
// };
18885+
//
18886+
// "struct bar" shows up in the decl AST as a "RecordDecl" with an
18887+
// incomplete definition.
18888+
if (const auto *TD = dyn_cast<TagDecl>(D))
18889+
return !TD->isCompleteDefinition();
1887718890
return false;
18891+
}
1887818892
QualType FieldType = FD->getType().getDesugaredType(Context);
1887918893
if (isa<PointerType>(FieldType)) {
1888018894
QualType PointeeType = cast<PointerType>(FieldType)->getPointeeType();
@@ -18888,7 +18902,7 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
1888818902
if (!getLangOpts().CPlusPlus &&
1888918903
(Record->hasAttr<RandomizeLayoutAttr>() ||
1889018904
(!Record->hasAttr<NoRandomizeLayoutAttr>() &&
18891-
llvm::all_of(Record->decls(), IsFunctionPointer))) &&
18905+
llvm::all_of(Record->decls(), IsFunctionPointerOrForwardDecl))) &&
1889218906
!Record->isUnion() && !getLangOpts().RandstructSeed.empty() &&
1889318907
!Record->isRandomized()) {
1889418908
SmallVector<Decl *, 32> NewDeclOrdering;
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm -frandomize-layout-seed=1234567890abcdef < %s | FileCheck %s
2+
// PR60349
3+
4+
// Clang will add a forward declaration of "struct bar" and "enum qux" to the
5+
// structures. This shouldn't prevent these structures from being randomized.
6+
// So the 'f' element shouldn't be at the start of the structure anymore.
7+
8+
struct foo {
9+
struct bar *(*f)(void);
10+
struct bar *(*g)(void);
11+
struct bar *(*h)(void);
12+
struct bar *(*i)(void);
13+
struct bar *(*j)(void);
14+
struct bar *(*k)(void);
15+
};
16+
17+
// CHECK-LABEL: define {{.*}}@t1(
18+
// CHECK-NOT: getelementptr inbounds %struct.foo, ptr %3, i32 0, i32 0
19+
struct bar *t1(struct foo *z) {
20+
return z->f();
21+
}
22+
23+
struct baz {
24+
enum qux *(*f)(void);
25+
enum qux *(*g)(void);
26+
enum qux *(*h)(void);
27+
enum qux *(*i)(void);
28+
enum qux *(*j)(void);
29+
enum qux *(*k)(void);
30+
};
31+
32+
// CHECK-LABEL: define {{.*}}@t2(
33+
// CHECK-NOT: getelementptr inbounds %struct.baz, ptr %3, i32 0, i32 0
34+
enum qux *t2(struct baz *z) {
35+
return z->f();
36+
}

0 commit comments

Comments
 (0)