[PATCH] D123763: [randstruct] Enforce using a designated init for a randomized struct

[Bill Wendling via Phabricator via cfe-commits]

void added inline comments.


================
Comment at: clang/lib/Sema/SemaInit.cpp:2171
 
     if (Field == FieldEnd) {
       // We've run out of fields. We're done.
----------------
stuij wrote:
> @void Unfortunately this end of fields check will break the non-designated initializer check below.
> 
> I GDB'ed through a failure of the below test, and if I'm understanding this correctly, the `CheckDesignatedInitializer` invocation above will move `Field` to the next available field in the struct. If there is none, we will break out of the loop and never reach the code below (On an AArch64 Linux host the field was placed last in the struct).
> 
> Instead I get a different error:
> 
>     error: 'error' diagnostics expected but not seen:
>       File /Users/zeno/code/llvm/clean/clang/test/Sema/init-randomized-struct.c Line 46: a randomized struct can only be initialized with a designated initializer
>     error: 'error' diagnostics seen but not expected:
>       File /Users/zeno/code/llvm/clean/clang/test/Sema/init-randomized-struct.c Line 46: excess elements in struct initializer
>     2 errors generated.
> 
> You can replicate this on other build setups by varying the value of -frandomoze-layout-seed. On x86_64 Linux and on Aarch64 OSX this worked for me (in seed value of lit test, change `f` to `d`):
>     -frandomize-layout-seed=1234567890abcded
> 
> Also, I know this was talked about before, and I know a fix is planned, but just to add my two cents: yes, it would be great if the `std::shuffle` could be changed to `llvm::shuffle`, also because we're expecting to produced the same code across different platforms for safety (compliance) reasons.
Oh! Good catch. Yeah, it's getting a bit messed up when the field that's initialized by a designated initializer is moved to the bottom of the `RecordDecl`. It looks like I can move it above the `Field == FieldEnd` check and it works (and bonus, I think it's the correct fix).

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123763