[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-09-28 Thread Justin Cady via Phabricator via cfe-commits
justincady added a comment. I suspect there's another unintended consequence of this change with regards to broken ODR violation detection. I filed an issue and am connecting it here should someone find this review first. Repository: rG

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-07-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D152604#4494975 , @rnk wrote: > It sounds like two users have hit this now: Chromium and Rust folks. This is > a flag flip, so it's pretty small and safe to rollback, and IMO we should > consider that while we debug the

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-07-12 Thread Mike Hommey via Phabricator via cfe-commits
glandium added a comment. In D152604#4494975 , @rnk wrote: > It sounds like two users have hit this now: Chromium and Rust folks. s/Rust/Firefox/. And it looks like we're hitting it for the same reason: linking a static rust (LTOed) library with C++

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-07-12 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. It sounds like two users have hit this now: Chromium and Rust folks. This is a flag flip, so it's pretty small and safe to rollback, and IMO we should consider that while we debug the underlying issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-07-06 Thread Mike Hommey via Phabricator via cfe-commits
glandium added a comment. In the realm of unintended consequences, this broke ODR violation detection when linking a rust static library with asan enabled because, while __asan_globals_registered is COMDAT in clang, for some reason, it's not in rust... So you end up with two asan.module_ctor

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-06-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D152604#4415403 , @MaskRay wrote: > I have thought about not applying `-fsanitize-address-globals-dead-stripping` > in `-fno-data-sections` mode, so that we won't have sections like `.bss.a`, > but the driver

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-06-12 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa8d3ae712290: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-06-12 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis accepted this revision. eugenis added a comment. LGTM This is long overdue, I think. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152604/new/ https://reviews.llvm.org/D152604 ___

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-06-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D152604#4415392 , @rnk wrote: > I think there's a fair bit more cleanup and simplification to be done, see > asanUsesGlobalsGC > > and

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-06-12 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I think there's a fair bit more cleanup and simplification to be done, see asanUsesGlobalsGC and the comments there. We could check CGOpts.DataSections right there, for example, and

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-06-09 Thread Petr Hosek via Phabricator via cfe-commits
phosek accepted this revision. phosek added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152604/new/ https://reviews.llvm.org/D152604

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-06-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: Sanitizers, aeubanks, rnk, eugenis, phosek, probinson. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. -fsanitize-address-globals-dead-stripping