aaron.ballman added a comment.

In D123544#3446814 <https://reviews.llvm.org/D123544#3446814>, @void wrote:

> In D123544#3446416 <https://reviews.llvm.org/D123544#3446416>, @aaron.ballman 
> wrote:
>
>> However, I had forgotten that the base feature *requires* the user to pass a 
>> randomization seed via a flag in addition to requiring the attribute (thank 
>> you for bringing that back to my attention). Because this feature requires a 
>> feature flag to enable it, this behavior *is* a conforming extension (the 
>> user has to take an action to get the new behavior). That said, I'm still 
>> not convinced we want to do this automagically for users -- it's *really* 
>> easy for that flag to be set in a makefile somewhere and the user has no 
>> idea that their (non-designated) initialization is now a security 
>> vulnerability. If we had error diagnostics when the user is about to shoot 
>> their foot off, I'd be more comfortable with the automatic hardening 
>> behavior.
>
> We should definitely emit an error if a user is trying to use a default 
> initializer for a structure that's up for randomization. It's something that 
> affects the whole feature, not just structs of function pointers.

Agreed.

> Let me work on that. But otherwise are you okay with this patch?

Yes, I'm okay with the direction of this patch. However, because this still 
automatically randomizes some structure layouts once the user passes the flag, 
I think the diagnostic above should perhaps land first unless there's some 
strong reason not to. (To be honest, I probably should have insisted on it in 
the first patch, but this one feels significantly more dangerous because it 
involves function pointers specifically and is more of an "automatic" feature 
than the first patch.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123544

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to