Skip to content

Commit

Permalink
Preserve ___asan_globals_registered symbol during LTO.
Browse files Browse the repository at this point in the history
Fixes #113404
  • Loading branch information
anforowicz committed Aug 29, 2023
1 parent c587fd4 commit e6dddbd
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 0 deletions.
7 changes: 7 additions & 0 deletions compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,13 @@ extern "C" void LLVMRustPrintPasses() {
extern "C" void LLVMRustRunRestrictionPass(LLVMModuleRef M, char **Symbols,
size_t Len) {
auto PreserveFunctions = [=](const GlobalValue &GV) {
// Preserve LLVM-injected, ASAN-related symbols.
// See also https://github.com/rust-lang/rust/issues/113404.
if (GV.getName() == "___asan_globals_registered") {
return true;
}

// Preserve symbols exported from Rust modules.
for (size_t I = 0; I < Len; I++) {
if (GV.getName() == Symbols[I]) {
return true;
Expand Down
43 changes: 43 additions & 0 deletions tests/codegen/sanitizer/address-sanitizer-globals-tracking.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Verifies that AddressSanitizer symbols show up as expected in LLVM IR with `-Zsanitizer`.
// This is a regression test for https://github.com/rust-lang/rust/issues/113404
//
// Notes about the `compile-flags` below:
//
// * The original issue only reproed with LTO - this is why this angle has
// extra test coverage via different `revisions`
// * To observe the failure/repro at LLVM-IR level we need to use `staticlib`
// which necessitates `-C prefer-dynamic=false` - without the latter flag,
// we would have run into "cannot prefer dynamic linking when performing LTO".
//
// The test is restricted to `only-linux`, because the sanitizer-related instrumentation is target
// specific. In particular, `___asan_globals_registered` is only used in the
// `InstrumentGlobalsELF` and `InstrumentGlobalsMachO` code paths. The `only-linux` filter is
// narrower than really needed (i.e. narrower than ELF-or-MachO), but this seems ok - having a
// linux-only regression test should be sufficient here.
//
// needs-sanitizer-address
// only-linux
//
// revisions:ASAN ASAN-FAT-LTO
//[ASAN] compile-flags: -Zsanitizer=address
//[ASAN-FAT-LTO] compile-flags: -Zsanitizer=address -Cprefer-dynamic=false -Clto=fat

#![crate_type="staticlib"]

// The test below mimics `CACHED_POW10` from `library/core/src/num/flt2dec/strategy/grisu.rs` which
// (because of incorrect handling of `___asan_globals_registered` during LTO) was incorrectly
// reported as an ODR violation in https://crbug.com/1459233#c1. Before this bug was fixed,
// `___asan_globals_registered` would show up as `internal global i64` rather than `common hidden
// global i64`. (The test expectations ignore the exact type because on `arm-android` the type
// is `i32` rather than `i64`.)
//
// CHECK: @___asan_globals_registered = common hidden global
// CHECK: @__start_asan_globals = extern_weak hidden global
// CHECK: @__stop_asan_globals = extern_weak hidden global
#[no_mangle]
pub static CACHED_POW10: [(u64, i16, i16); 4] = [
(0xe61acf033d1a45df, -1087, -308),
(0xab70fe17c79ac6ca, -1060, -300),
(0xff77b1fcbebcdc4f, -1034, -292),
(0xbe5691ef416bd60c, -1007, -284),
];

0 comments on commit e6dddbd

Please sign in to comment.