[PATCH] D123958: [randstruct] Randomize all elements of a record
Bill Wendling via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 27 13:06:20 PDT 2022
void added inline comments.
================
Comment at: clang/lib/AST/Randstruct.cpp:201
+ dyn_cast<ConstantArrayType>(RandomizedFields.back()->getType()))
+ if (CA->getSize().sle(2) || CA->isIncompleteArrayType())
+ FlexibleArray = RandomizedFields.pop_back_val();
----------------
aaron.ballman wrote:
> CA->isIncompleteArrayType() looks to be at the wrong spot. An incomplete array type is one of type `IncompleteArrayType`, which a `ConstantArrayType` is not. Are we missing test coverage, or does the `hasFlexibleArrayMember()` make it so that we just need to remove this check entirely?
It looks like if it's an `IncompleteArrayType`, then it will be captured by `hasFlexibleArrayMember`. I think we can do without the check here. (It doesn't hurt, because it should always return `false`.)
================
Comment at: clang/unittests/AST/RandstructTest.cpp:368
+ int h;
+ char name[0];
+ } __attribute__((randomize_layout));
----------------
aaron.ballman wrote:
> Can you add a test where the last field is `char name[1];` and another one for `char name[];` so that we have full coverage there?
The `name[]` is above this test. The `name[1]` is due to a copy-paste error (oops). Fixed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123958/new/
https://reviews.llvm.org/D123958
More information about the cfe-commits
mailing list