[PATCH] D121556: [randstruct] Add randomize structure layout support

Fri Mar 25 13:05:23 PDT 2022

void added inline comments.

================
Comment at: clang/include/clang/AST/Decl.h:2842
   mutable unsigned CachedFieldIndex : 30;
+  mutable unsigned OriginalFieldIndex : 30;

----------------
aaron.ballman wrote:
> It's unfortunate that every field node in the AST is now going to be 4 bytes larger for this feature; I worry about the extra memory pressure from the additional overhead, so if there's a way for us to save some space here, I think it might be worth it. (I'm worried that our max template instantiation depth will shrink because of this.)
Turns out this field wasn't used. I removed it.

================
Comment at: clang/include/clang/Basic/AttrDocs.td:6367
+program should be compiled with the same seed, but keep the seed safe
+otherwise.
+
----------------
aaron.ballman wrote:
> I think it would help to explain the effects of this attribute in conjunction with structure packing techniques (the `packed` attribute and bit-fields) and whether any attempt is made to keep the structure packed or those fields together or not. I'd expect no such attempt is made, but people may want to know "attempts to pack this structure" and "randomize this structure layout" are not compatible. (We may want to diagnose in the case of seeing the `packed` attribute, in fact.)
> 
> We should also be more clear that the seed specified does not have to be numeric in nature (from the file or when directly on the command line).
Having randomization and the `packed` attribute probably isn't the best option. I'll modify things to prevent that from happening.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556