Mark ___asan_globals_registered as an exported symbol for LTO

compiler/rustc_codegen_ssa/src/back/metadata.rs

@@ -214,16 +214,7 @@ pub(crate) fn create_object_file(sess: &Session) -> Option<write::Object<'static
         // Unsupported architecture.
         _ => return None,
     };
-    let binary_format = if sess.target.is_like_osx {
-        BinaryFormat::MachO
-    } else if sess.target.is_like_windows {
-        BinaryFormat::Coff
-    } else if sess.target.is_like_aix {
-        BinaryFormat::Xcoff
-    } else {
-        BinaryFormat::Elf
-    };
-
+    let binary_format = sess.target.binary_format();
     let mut file = write::Object::new(binary_format, architecture, endianness);
     if sess.target.is_like_osx {
         if let Some(build_version) = macho_object_build_version_for_target(&sess.target) {

compiler/rustc_codegen_ssa/src/back/symbol_export.rs

@@ -2,6 +2,7 @@ use crate::base::allocator_kind_for_codegen;
 
 use std::collections::hash_map::Entry::*;
 
+use object::BinaryFormat;
 use rustc_ast::expand::allocator::{ALLOCATOR_METHODS, NO_ALLOC_SHIM_IS_UNSTABLE};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def::DefKind;
@@ -245,50 +246,46 @@ fn exported_symbols_provider_local(
         ))
     }
 
+    // Rust assumes that all code provided to (non-plugin) LTO comes from Rust, so it knows about
+    // all symbols that are involved. This doesn't hold up for symbols that get injected by LLVM,
+    // so they need to be special-cased.
+    let mut externally_injected_weak_symbols = Vec::new();
     if tcx.sess.instrument_coverage() || tcx.sess.opts.cg.profile_generate.enabled() {
         // These are weak symbols that point to the profile version and the
         // profile name, which need to be treated as exported so LTO doesn't nix
         // them.
-        const PROFILER_WEAK_SYMBOLS: [&str; 2] =
-            ["__llvm_profile_raw_version", "__llvm_profile_filename"];
-
-        symbols.extend(PROFILER_WEAK_SYMBOLS.iter().map(|sym| {
-            let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, sym));
-            (
-                exported_symbol,
-                SymbolExportInfo {
-                    level: SymbolExportLevel::C,
-                    kind: SymbolExportKind::Data,
-                    used: false,
-                },
-            )
-        }));
+        externally_injected_weak_symbols.push("__llvm_profile_raw_version");
+        externally_injected_weak_symbols.push("__llvm_profile_filename");
     }
-
     if tcx.sess.opts.unstable_opts.sanitizer.contains(SanitizerSet::MEMORY) {
-        let mut msan_weak_symbols = Vec::new();
-
         // Similar to profiling, preserve weak msan symbol during LTO.
         if tcx.sess.opts.unstable_opts.sanitizer_recover.contains(SanitizerSet::MEMORY) {
-            msan_weak_symbols.push("__msan_keep_going");
+            externally_injected_weak_symbols.push("__msan_keep_going");
         }
-
         if tcx.sess.opts.unstable_opts.sanitizer_memory_track_origins != 0 {
-            msan_weak_symbols.push("__msan_track_origins");
+            externally_injected_weak_symbols.push("__msan_track_origins");
+        }
+    }
+    if tcx.sess.opts.unstable_opts.sanitizer.contains(SanitizerSet::ADDRESS) {
+        // Similar to profiling, preserve weak asan symbols during LTO.
+        match tcx.sess.target.binary_format() {
+            BinaryFormat::Elf | BinaryFormat::MachO => {
+                externally_injected_weak_symbols.push("___asan_globals_registered")
+            }
+            _ => (),
         }
-
-        symbols.extend(msan_weak_symbols.into_iter().map(|sym| {
-            let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, sym));
-            (
-                exported_symbol,
-                SymbolExportInfo {
-                    level: SymbolExportLevel::C,
-                    kind: SymbolExportKind::Data,
-                    used: false,
-                },
-            )
-        }));
     }
+    symbols.extend(externally_injected_weak_symbols.into_iter().map(|sym| {
+        let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, sym));
+        (
+            exported_symbol,
+            SymbolExportInfo {
+                level: SymbolExportLevel::C,
+                kind: SymbolExportKind::Data,
+                used: false,
+            },
+        )
+    }));
 
     if tcx.crate_types().contains(&CrateType::Dylib)
         || tcx.crate_types().contains(&CrateType::ProcMacro)

compiler/rustc_target/src/spec/mod.rs

@@ -39,6 +39,7 @@ use crate::abi::{Endian, Integer, Size, TargetDataLayout, TargetDataLayoutErrors
 use crate::json::{Json, ToJson};
 use crate::spec::abi::{lookup as lookup_abi, Abi};
 use crate::spec::crt_objects::{CrtObjects, LinkSelfContainedDefault};
+use object::BinaryFormat;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_fs_util::try_canonicalize;
 use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
@@ -2053,6 +2054,19 @@ fn add_link_args(link_args: &mut LinkArgs, flavor: LinkerFlavor, args: &[&'stati
 }
 
 impl TargetOptions {
+    /// Binary format (e.g. ELF) implied by the `TargetOptions`.
+    pub fn binary_format(&self) -> BinaryFormat {
+        if self.is_like_osx {
+            BinaryFormat::MachO
+        } else if self.is_like_windows {
+            BinaryFormat::Coff
+        } else if self.is_like_aix {
+            BinaryFormat::Xcoff
+        } else {
+            BinaryFormat::Elf
+        }
+    }
+
     fn link_args(flavor: LinkerFlavor, args: &[&'static str]) -> LinkArgs {
         let mut link_args = LinkArgs::new();
         add_link_args(&mut link_args, flavor, args);

tests/codegen/sanitizer/address-sanitizer-globals-tracking.rs

@@ -0,0 +1,42 @@
+// Verifies that AddressSanitizer symbols show up as expected in LLVM IR with -Zsanitizer.
+//
+// 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.
+//
+// needs-sanitizer-address
+// only-linux
+//
+// revisions:ASAN ASAN-LTO
+//[ASAN]     compile-flags: -Zsanitizer=address
+//[ASAN-LTO] compile-flags: -Zsanitizer=address -C prefer-dynamic=false -C lto
+
+#![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`.)
+//
+// See https://github.com/rust-lang/rust/issues/113404 for more discussion.
+//
+// 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),
+];