[PATCH] D123958: [randstruct] Randomize all elements of a record

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D123958#3461020 <https://reviews.llvm.org/D123958#3461020>, @void wrote:

> In D123958#3459205 <https://reviews.llvm.org/D123958#3459205>, @aaron.ballman wrote:
>
>> I think you'll need a more targeted approach than assuming the only kinds of declarations in a struct are field-like in C.
>>
>> It seems that the issue you've got is with anonymous objects in a structure where the inner fields are available for lookup in the outer structure. One question I have is: what's the expectation for the user? There's two ways to look at this. 1) The anonymous object is a single field; that its members can be found in the outer object is not relevant. 2) The fields of the anonymous object should also be randomized. The same is true for any inner structure, not just anonymous ones.
>>
>> I had assumed that any structure not marked for randomization would not be randomized. Based on that, I don't think inner structure objects (anonymous or otherwise) should automatically randomize their fields. WDYT?
>
> We don't randomize inner structures unless they have the `randomize_layout` flag. That's always been the case, and this patch doesn't change that.

That's good, I misunderstood originally, so thanks!

> The issue is that we were dropping the inner structures/unions because they aren't `FieldDecl`s, but `RecordDecl`s, with an `IndirectFieldDecl` if the inner structure is anonymous. The `IndirectFieldDecl` bits appear after the `RecordDecl` they're attached to, but doesn't seem like the ordering is important. The `IndirectFieldDecl` thing is there so that the anonymous fields are available in the outer structure.

Agreed that we need to fix that behavior, but I think it's pretty fragile to assume every declaration in a struct can be reordered. I mentioned static assert decls above as one obvious example, but also consider things like:

  struct S {
    enum E {
      One,
    };
  
    enum E whatever;
  } __attribute__((randomize_layout));

We currently drop the enumeration declaration entirely (oops), but if we rearrange all declaration orders, then the declaration of `whatever` may suddenly look to come BEFORE we know what the type of `enum E` will be.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123958/new/

https://reviews.llvm.org/D123958