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