[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2023-02-03 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen updated this revision to Diff 494756. samitolvanen added a comment. This revision is now accepted and ready to land. Herald added subscribers: ormris, steven_wu. Don't drop type hashes from VisibleToRegularObj symbols. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2023-01-25 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. ah, sorry, I think I might be conflating "kcfi-seal" with "ibt-seal?" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138337/new/ https://reviews.llvm.org/D138337 ___

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2023-01-25 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen added a comment. In D138337#4080628 , @nickdesaulniers wrote: >> If you're referring to ibt-seal, I assume a similar optimization could be >> added for BTI too. > > Yes, please. Does it make sense to add to this patch, or a follow up on

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2023-01-25 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. > If you're referring to ibt-seal, I assume a similar optimization could be > added for BTI too. Yes, please. Does it make sense to add to this patch, or a follow up on top? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2023-01-25 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen added a comment. In D138337#4080615 , @nickdesaulniers wrote: > The test is for x86, does this also work for aarch64 with BTI? IBT/BTI doesn't affect this specific optimization, it's only about KCFI. If you're referring to ibt-seal, I

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2023-01-25 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. The test is for x86, does this also work for aarch64 with BTI? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138337/new/ https://reviews.llvm.org/D138337 ___ cfe-commits

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2023-01-25 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen added a comment. In D138337#4078843 , @nickdesaulniers wrote: > Is https://reviews.llvm.org/D142163 a blocker for this? Yes, and this patch needs to be updated to take `VisibleToRegularObj` into account. I'll update this patch once we

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2023-01-24 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. Is https://reviews.llvm.org/D142163 a blocker for this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138337/new/ https://reviews.llvm.org/D138337 ___ cfe-commits

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2022-12-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. (Sorry for my belated response.) If we make ThinLTO properly track combined the address-taken property, and combine precise `addressTaken` and `VisibleToRegularObj`, it seems that we can use this condition to decide whether ENDBR is needed with an appropriate code

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2022-12-05 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen added a comment. In D138337#3972493 , @pcc wrote: > Isn't that just a bug in the optimization? It shouldn't be applying this to > symbols where `VisibleToRegularObj` is set. The current patch simply replicates ibt-seal behavior, but sure,

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2022-12-05 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment. In D138337#3972138 , @samitolvanen wrote: > In D138337#3972009 , @pcc wrote: > >> Can't this be implicit if LTO is being used? > > I would prefer to keep this behind a flag (similarly to

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2022-12-05 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen added a comment. In D138337#3972009 , @pcc wrote: > Can't this be implicit if LTO is being used? I would prefer to keep this behind a flag (similarly to `-mibt-seal`), so we can better control when and where the optimization enabled. For

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2022-12-05 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment. Can't this be implicit if LTO is being used? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138337/new/ https://reviews.llvm.org/D138337 ___ cfe-commits mailing list

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2022-11-18 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen added reviewers: nickdesaulniers, pcc, MaskRay, kees, joaomoreira. samitolvanen added a comment. Herald added a subscriber: StephenFan. ClangBuiltLinux issue: https://github.com/ClangBuiltLinux/linux/issues/1737 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2022-11-18 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen created this revision. Herald added subscribers: pengfei, hiraditya, inglorion. Herald added a project: All. samitolvanen requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, MaskRay. Herald added projects: clang, LLVM. With -fsanitize=kcfi, Clang