Various symbols on win/apple are no longer weak after #598

[aeubanks]

After upgrading our rust toolchain version, on multiple win/apple platforms (arm64 win specifically for this error) we're seeing link errors like

../../third_party/llvm-build/Release+Asserts/bin/lld-link --rsp-quoting=posix "/OUT:./test_rust_metadata_cc_exe.exe" /nologo -libpath:../../third_party/llvm-build/Release+Asserts/lib/clang/20/lib/windows /winsysroot:../../third_party/depot_tools/win_toolchain/vs_files/7393122652 /MACHINE:ARM64  "/PDB:./test_rust_metadata_cc_exe.exe.pdb" "@./test_rust_metadata_cc_exe.exe.rsp"
lld-link: error: duplicate symbol: __udivti3
>>> defined at libcompiler_builtins_compiler_builtins.rlib(libcompiler_builtins_compiler_builtins.compiler_builtins.878e573648bac277-cgu.0.rcgu.o)
>>> defined at clang_rt.builtins-aarch64.lib(udivti3.c.obj)

lld-link: error: duplicate symbol: __umodti3
>>> defined at libcompiler_builtins_compiler_builtins.rlib(libcompiler_builtins_compiler_builtins.compiler_builtins.878e573648bac277-cgu.3.rcgu.o)
>>> defined at clang_rt.builtins-aarch64.lib(umodti3.c.obj)

lld-link: error: duplicate symbol: __muloti4
>>> defined at libcompiler_builtins_compiler_builtins.rlib(libcompiler_builtins_compiler_builtins.compiler_builtins.878e573648bac277-cgu.0.rcgu.o)
>>> defined at clang_rt.builtins-aarch64.lib(muloti4.c.obj)
ninja: build stopped: subcommand failed.

I believe this is due to #598. We were using the weak-intrinsics feature before. #598 removes that feature and makes everything weak by default, except for on win/apple platforms. That seems to be because some intrinsics unconditionally were not marked weak on win/apple. However, not all intrinsics were like that, e.g. the symbols in the error message above.

The following diff seems to make the error go away

diff --git a/src/macros.rs b/src/macros.rs
index fb14660..721d242 100644
--- a/src/macros.rs
+++ b/src/macros.rs
@@ -464,7 +464,7 @@ macro_rules! intrinsics {
         mod $name {
             $(#[$($attr)*])*
             #[no_mangle]
-            #[cfg_attr(all(not(windows), not(target_vendor = "apple")), linkage = "weak")]
+            #[linkage = "weak"]
             $(unsafe $($empty)?)? extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? {
                 super::$name($($argname),*)
             }

[Amanieu]

Thanks! Can you submit this as a PR?

[aeubanks]

To clarify, some but not all symbols were marked weak on win/apple. I'm not sure if marking them all as weak matters or not. I'll do a bit more testing, and audit other places that check win/apple.

[zmodem]

Should #598 be considered for reverting while this is being investigated?

[nico]

We haven't been able to update Rust in Chromium for weeks due to this bug.

@Amjad50: Is there a fix on the horizon, or could we revert #598 for now to keep HEAD green?

[tgross35]

Let's see if we can make them unconditionally weak first, since #598 wouldn't be the cleanest revert. If somebody puts up a PR we can start that process.

[Amjad50]

Hello @nico , thanks for mentioning me, I wasn't aware of the issue. I'll do some debugging tonight and check.

@aeubanks were you able to test that making them all 'weak' would fix the issue without problems?

[Amjad50]

So, regarding this issue. The main thing about that attr is that we thought weak isn't supported by windows and apple targets.
and thus it was conflicting with the builtin symbols, making it unconditional #[linkage = "weak"], will add the weak symbol, and thus the compiler will be able to ignore it. and there is the catch, based on our previous assessment, we think some compilers may not support this.

A better fix would be to completely remove these symbols on apple/windows targets, since they would be provided by the clang compiler it seems. The main idea of that PR was to provide math functions for systems without std, like custom kernels and whatnot.

[ChrisDenton]

At link time, Windows can have a kind weak symbols (e.g. see "Weak Externals" or /ALTERNATENAME). DLLs do not have a weak linking mechanism (which may or may not be relevant to Rust dylibs).

But in any case I would agree it's better to just remove these for now and then figure out if/how to add them back. Maybe that would require rustc changes.

[Amjad50]

It was mentioned in the issue description that in the previous version the weak-intrinsics feature was used, was it required? i.e. was it crashing without this feature? As if we fixed the issue by removing the symbols, it will result in different behavior.

[tgross35]

@Amjad50 are you able to put up a fix? Based on the above, this probably needs to be a manual partial revert of #598.

While that is being prepared, I still think we should try making symbols weak on all platforms as in the original comment by @aeubanks. This is a single line fix that we can merge quickly - if it corrects things then great, if not then we have a slower revert coming.

(beta branch is in ~9 days, we should try to have this resolved by then)

[Amjad50]

Yes, I'll then move on with enabling weak on all platforms except windows-gnu.

[tgross35]

Reopening until we verify this fixes things.

[tgross35]

At link time, Windows can have a kind weak symbols (e.g. see "Weak Externals" or /ALTERNATENAME). DLLs do not have a weak linking mechanism (which may or may not be relevant to Rust dylibs).

I noticed https:/rust-lang/rust/blob/8269be147b13812c201d0b64966adc1c6536ca59/compiler/rustc_codegen_ssa/src/back/link.rs#L2752-L2753 which makes it seem like builtins has to be static. So maybe we are okay here for DLLs too?

In any case, with the changes in #672 I think we should be at least as compatible as with the weak-intrinsics feature when it existed.