clang-randstruct / llvm-project

Randomize the order of fields in a structure layout as a compile-time hardening feature
3 stars 1 forks source link

Figure out precisely what the DeclContext's deal is #51

Open connorkuehl opened 5 years ago

connorkuehl commented 5 years ago

This is believed to be the source of our problems building the Linux kernel (or any more complicated project).

I think our "re-linking" of what is presumed to be a linked list of FieldDecls is compromising the integrity of the DeclContext.

Related issue: #42

Jafosterja commented 5 years ago
FieldDecl 0x556cbce85178 <poc.c:25:2, col:6> col:6 referenced a 'int'
FieldDecl 0x556cbce851d8 <poc.c:26:2, col:7> col:7 b 'char'
FieldDecl 0x556cbce852c0 <poc.c:27:2, col:9> col:6 c 'int [6]'
FieldDecl 0x556cbce85328 <poc.c:28:2, col:6> col:6 d 'int'
FieldDecl 0x556cbce85390 <poc.c:29:2, col:6> col:6 e 'int'
FieldDecl 0x556cbce853f8 <poc.c:30:2, col:6> col:6 f 'int'
FieldDecl 0x556cbce854a0 <poc.c:31:2, col:18> col:18 m 'struct mystruct':'struct mystruct'
FieldDecl 0x556cbce85540 <poc.c:32:2, col:20> col:11 potato 'unsigned int'
`-ConstantExpr 0x556cbce85528 <col:20> 'int'
  `-IntegerLiteral 0x556cbce854f0 <col:20> 'int' 1
FieldDecl 0x556cbce85680 <poc.c:33:2, col:13> col:9 hi 'int (*)()'
poc.c:18:8: warning: struct declared with 'randomize_layout' and 'no_randomize_layout' attributes; attribute
      'no_randomize_layout' takes precedence [-Wno-randomize-layout]
struct mystruct {
       ^
FieldDecl 0x556cbce85540 <poc.c:32:2, col:20> col:11 potato 'unsigned int'
`-ConstantExpr 0x556cbce85528 <col:20> 'int'
  `-IntegerLiteral 0x556cbce854f0 <col:20> 'int' 1
FieldDecl 0x556cbce85390 <poc.c:29:2, col:6> col:6 e 'int'
FieldDecl 0x556cbce85328 <poc.c:28:2, col:6> col:6 d 'int'
FieldDecl 0x556cbce854a0 <poc.c:31:2, col:18> col:18 m 'struct mystruct':'struct mystruct'
FieldDecl 0x556cbce852c0 <poc.c:27:2, col:9> col:6 c 'int [6]'
FieldDecl 0x556cbce85680 <poc.c:33:2, col:13> col:9 hi 'int (*)()'
FieldDecl 0x556cbce853f8 <poc.c:30:2, col:6> col:6 f 'int'
FieldDecl 0x556cbce851d8 <poc.c:26:2, col:7> col:7 b 'char'
FieldDecl 0x556cbce85178 <poc.c:25:2, col:6> col:6 referenced a 'int'
da-x commented 5 years ago

The problem is that reorganizedFields is stripping the non-FieldDecl items under RecordDecl. There are records relating to fields accesses of anonymous unions, for example IndirectFieldDecl. These are used in field access resolution.

To fix this, we can iterate using D->decls() instead of D->fields(), and put aside anything for which dyn_cast<FieldDecl> returns nullptr. After randomizing, we can put those other records back in, before calling commit().

da-x commented 5 years ago

Also if struct hasFlexibleArrayMember() attribute applies to the struct, you should not move the last struct's field, as it may be an [] array on which there is a 'last member of struct' offset assumption. And although [0] is used in some cases instead of [], Clang does not pick on it to be a flexible array member (maybe should open a bug on it).

connorkuehl commented 5 years ago

Thanks @da-x! I appreciate you taking the time to help demystify this for us. I can't speak for the others on the team, but I know that some day I'd like to pick this back up again; I'm just not in a position where I can make it a priority right now. Your advice is very helpful and sincerely appreciated.