__asan_globals_registered is not comdat when building a staticlib with LTO

[glandium]

Disclaimer: I tried to create a testcase from scratch, but for some reason I wasn't able to find a way to trigger the use of __asan_register_elf_globals instead of __asan_globals_register.

STR:

• Clone https://github.com/glandium/nss-builtins
• cd nss-builtins
• RUSTFLAGS="-Zsanitizer=address" CARGO_PROFILE_RELEASE_LTO=true cargo +nightly build --release
• objdump -t target/release/libbuiltins_static.a | grep asan_globals_registered

Actual output:

0000000000000000 l     O .bss.___asan_globals_registered	0000000000000008 ___asan_globals_registered
0000000000000000 l    d  .bss.___asan_globals_registered	0000000000000000 .bss.___asan_globals_registered

Expected output:
Something like:

0000000000000008       O *COM*	0000000000000008 .hidden ___asan_globals_registered

This doesn't happen without LTO.
The unfortunate consequence is that when the resulting static library is linked with C or C++ code compiled with clang with -fsanitize=address -fsanitize-address-globals-dead-stripping (that latter flag is now default in clang trunk), which also uses __asan_register_elf_globals/__asan_globals_registered, ODR violation detection kick in complaining about globals defined multiple times, because both the clang-side asan constructor and the rust asan constructor register all the globals. Normally, what happens is that they both use the same __asan_globals_registered (thus it normally being *COM*), and set its value, so that only one constructor registers the globals. With the LTOed staticlib, what happens is that there are two distinct __asan_globals_registered, so both constructors go through.

rustc +nightly --version --verbose:

rustc 1.72.0-nightly (d9c13cd45 2023-07-05)
binary: rustc
commit-hash: d9c13cd4531649c2028a8384cb4d4e54f985380e
commit-date: 2023-07-05
host: x86_64-unknown-linux-gnu
release: 1.72.0-nightly
LLVM version: 16.0.5

(Edit: fixed typos, changed the peculiar setup with -Clto and -Cembed-bitcode=yes to the more normal LTO, which shows the problem too)

[danakj]

https://bugs.chromium.org/p/chromium/issues/detail?id=1459233 is tracking this for Chromium as it causes our asan bots to fail.

[rnk]

Here is the code which creates the __asan_globals_registered global:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp#L2216

It's interesting that it uses common linkage (*COM* as you said). Common linkage should work, but to me it is a surprising choice. I would've expected this global to use linkonce_odr linkage and be marked comdat, which is what you would get for a C++17 inline global.

I don't know how Rust is producing LTOed static libraries, but maybe somewhere along the way there is a bug in how LTO is handling common linkage globals.

One possible summary of this issue is that the ODR violation detector is suffering from an ODR violation, we have two flags when we should have one.

[glandium]

One possible summary of this issue is that the ODR violation detector is suffering from an ODR violation, we have two flags when we should have one.

I like this take :)

[MaskRay]

I consider myself quite familiar with asan, LTO, Clang Driver, but I know very little about Rust.

rm -r target/release/libbuiltins_static.a
RUSTFLAGS="-Zsanitizer=address" CARGO_PROFILE_RELEASE_LTO=true cargo +nightly build --release --verbose

gives me this command (after removing --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat):

rustc --crate-name builtins_static --edition=2021 src/lib.rs --diagnostic-width=159 --crate-type staticlib --emit=dep-info,link -C opt-level=3 -C lto -C metadata=9ef9aaa79a14308e -C extra-filename=-9ef9aaa79a14308e --out-dir /tmp/p/nss-builtins/target/release/deps -L dependency=/tmp/p/nss-builtins/target/release/deps --extern pkcs11_bindings=/tmp/p/nss-builtins/target/release/deps/libpkcs11_bindings-cb6ed583361f31fa.rlib --extern smallvec=/tmp/p/nss-builtins/target/release/deps/libsmallvec-e6a97ae5d501d626.rlib -Zsanitizer=address

% ar t /tmp/p/nss-builtins/target/release/deps/libbuiltins_static-9ef9aaa79a14308e.a | wc -l
177
% ar x /tmp/p/nss-builtins/target/release/deps/libbuiltins_static-9ef9aaa79a14308e.a builtins_static-9ef9aaa79a14308e.builtins_static.7af90105-cgu.0.rcgu.o
% readelf -Ws builtins_static-9ef9aaa79a14308e.builtins_static.7af90105-cgu.0.rcgu.o | grep ___asan_globals_registered
   117: 0000000000000000     8 OBJECT  LOCAL  DEFAULT 3732 ___asan_globals_registered
  2944: 0000000000000000     0 SECTION LOCAL  DEFAULT 3732 .bss.___asan_globals_registered
% llvm-nm -gU builtins_static-9ef9aaa79a14308e.builtins_static.7af90105-cgu.0.rcgu.o
0000000000000000 T BUILTINSC_GetFunctionList
0000000000000000 V DW.ref.rust_eh_personality
0000000000000000 D _ZN3std3sys4unix4args3imp15ARGV_INIT_ARRAY17h244e25de9c1d3c88E
0000000000000000 T rust_eh_personality

Most defined symbols are localized. I do not know why the 4 symbols are special and rustc lto doesn't localize them.

I don't know how to rerun the rustc with a locally built (./x.py build with config.toml containing [rust]\ndebug=true).
I suspect that preventing ___asan_globals_registered from being localized will fix this bug.

[MaskRay]

-fcommon is consider bad nowadays but the ___asan_globals_registered COMMON symbol use case is fine. It is like a COMDAT group containing just a variable. COMMON is more size efficient than using a COMDAT group (there is a size overhead due to a 64 byte Elf64_Shdr header).

If we use a COMDAT for ___asan_globals_registered but rustc compiler/rustc_codegen_llvm/src/back/lto.rs still localizes the symbol, then we'd still have this bug.

[anforowicz]

Let me try to help move this bug forward by attempting to answer some of the questions above. I have limited experience with linkers, asan, and compilers, so please shout if there are any mistakes below.

---

RE: @MaskRay: I don't know how to rerun the rustc with a locally built (./x.py build)

I tweaked the original cargo cmdline (from the first comment/report on this bug) by adding RUSTC=<path to locally built rustc> and adding -Zbuild-std --target x86_64-unknown-linux-gnu (and other than that I've rerun the original repro steps + debugging steps from your earlier comment at #113404 (comment)):

RUSTC=$HOME/src/github/rust/build/x86_64-unknown-linux-gnu/stage1/bin/rustc RUSTFLAGS="-Zsanitizer=address" CARGO_PROFILE_RELEASE_LTO=true cargo +nightly build --release -Zbuild-std --target x86_64-unknown-linux-gnu

(Note that this changes the path where the build artifacts are - e.g. target/x86_64-unknown-linux-gnu/release/deps/libbuiltins_static-9bce9c00959dd948.a instead of target/release/deps/libbuiltins_static-9ef9aaa79a14308e.a)

---

RE: @MaskRay: I do not know why the 4 symbols are special and rustc lto doesn't localize them.

I am not sure if these are the reasons, but this is what I've found for some of the symbols emitted by llvm-nm -gU ...:

• I see that rust/compiler/rustc_codegen_llvm/src/context.rs marks the EH personality with llvm::UnnamedAddr::Global

• AFAIU _ZN3std3sys4unix4args3imp15ARGV_INIT_ARRAY17h244e25de9c1d3c88E corresponds to the static initializer in rust/library/std/src/sys/unix/args.rs which is marked as #[used]

• I see that BUILTINSC_GetFunctionList comes from the repro and is declared as #[no_mangle] which AFAIU tells rustc to export the function.

  • FWIW, I also see that rust/compiler/rustc_codegen_ssa/src/back/symbol_export.rs special-cases "rust_eh_personality" (see here), but changing this to cover names with "asan" substring didn't seem to have an effect on the llvm-nm -gU ... output)

---

RE: @MaskRay: I suspect that preventing ___asan_globals_registered from being localized will fix this bug.

Can you please elaborate on that? From your #113404 (comment), it seems that you are saying that a change in rustc is needed - did I get that right?

OTOH, it seems that ___asan_globals_registered comes from outside of rustc sources - it comes from llvm-project/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp and therefore it seems that maybe we need to change how it is marked for linking by LLVM? Interestingly the comment there mentions "a local symbol" - not sure if this is relevant:

// ASan version script has __asan_* wildcard. Triple underscore prevents a
// linker (gold) warning about attempting to export a local symbol.
const char kAsanGlobalsRegisteredFlagName[] = "___asan_globals_registered";

[anforowicz]

/cc @eugenis who AFAICT added the kAsanGlobalsRegisteredFlagName comment above in llvm/llvm-project@964f466#diff-74cc02fff013e3ab08aefcad70e555b1177549f8403d03395c856c3f8c5b5875