The "mangled-names" feature removes intrinsics from compiler_builtins that aren't in compiler_rt

[danakj]

We're trying to link Rust into a C++ target and we don't want to create ODR violations between compiler_rt and compiler_builtins.

To avoid this, we set the mangled-names feature on compiler_builtins, so that the intrinsics do not get demangled. This avoids all collisions!

However Rust's compiler_builtins provides some intrinsics that are not present in clang's compiler_rt, for 128-bit integers. Such as: __udivti3 and __umodti3 which are referenced by rust\library\std\src\sys\windows\thread_parking.rs:169.

An error showing this is here: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8780748803129506673/+/u/compile__with_patch_/stdout#L7628_7

The mangled-names feature claims that it is for using Rust with compiler_rt, which is good. But it still needs to provide intrinsics that a C++ compiler_rt does not have.

How do we use mangled-names but keep 128-bit integer intrinsics that Rust needs and C/C++ does not provide?

[bjorn3]

mangled-names is meant only for testing purposes as far as I can tell. If you use it, compiler-builtins is effectively unused as LLVM won't know to use the mangled names when emitting calls into it (and LLVM doesn't support changing the names afaik). Duplicate definitions in compiler_rt and compiler-builtins shouldn't matter. Rust's build system builds compiler-builtins with like 1000 codegen units and we place ever intrinsic in it's own module. This combined effectively causes every intrinsic to end up in a separate object file inside the libcompiler_builtins.rlib archive. By default linkers only pull in archive members if any of the symbols defined in them are actually used. This means that if an intrinsic is defined by both compiler-builtins and compiler_rt, the linker will pull in the right object for either compiler-builtins or compiler_rt, but not both, thus preventing a ODR violation.

Based on my reading of https://bugs.chromium.org/p/chromium/issues/detail?id=1445978 it seems the issue you are having is not that both impls collide (they should behave identically and only one of them will be picked by the linker) but that LLVM codegens an __aeabi_llsl call with a shift amount larger than the size in bits of the shifted value and the compiler-builtins impl doesn't allow this. I can't find any information about how it should behave for too large shift amounts. If that is UB, LLVM will have to be fixed. If it is defined behavior, the impl in compiler-builtins will have to be fixed.

Edit: It is UB. Quoting the ABI specification:

The shift functions only need to work for shift counts in 0..63.

[bjorn3]

LLVM's RISC-V backend has the same UB issue, which caused a problem with compiler-builtins too: llvm/llvm-project#57988

[danakj]

Thanks for the additional context and links.

If you use it, compiler-builtins is effectively unused

This is what we want, as we want to use the C++ intrinsics until such time as we can make the explicit choice to switch.

This means that if an intrinsic is defined by both compiler-builtins and compiler_rt, the linker will pull in the right object for either compiler-builtins or compiler_rt, but not both, thus preventing a ODR violation.

It does not use the right one unfortunately, it appears to favour the Rust ones for C++ as well, unless we put the compiler-rt lib on the command line before any rlibs. We discovered this when C++ code started panicking in Rust intrinsics due to us linking a small piece of Rust into the C++ binary: https://bugs.chromium.org/p/chromium/issues/detail?id=1445978#c2

Your reading is correct, and as well, the Rust intrinsics have since been fixed to not panic in this situation. However the implementations are not the exact same and we don't intend that linking a Rust library changes the implementation of all C++ intrinsics, with potential performance implications. We would really like to have control over which intrinsics are used, and keep using the C++ intrinsics until we see that the Rust ones are a better choice globally.

Additional context: clang-cl's compiler-rt is missing i128 div/mod specifically https://bugs.chromium.org/p/chromium/issues/detail?id=787617 which is why we hit this problem when turning on mangled-names.

[bjorn3]

Ideally LLVM would allow changing the names of the intrinsic calls it emits such that we can use our own implementation for rust code while allowing C/C++ code to keep using whichever implementation they want to use.

As for the missing intrinsics in compiler-rt, I guess the easiest way would be to copy the implementation in compiler-builtins to a separate crate. And for x86/x86_64, I think you did also have to copy src/probestack.rs if you want to omit compiler-builtins from the linker invocation (which enabling mangled-names is effectively equivalent too).

[danakj]

FWIW This is the Rust change to not panic on bad shifts: 1634193

[danakj]

Another option here would be to mark the symbols in compiler_builtins as weak. I know some other C++/Rust toolchains have done this to resolve the same issue. I was trying to avoid a post-build step like that though. If we could mark them all as #[linkage="weak"] behind a cfg that would be really good too (if linkage=weak works in nightly?)

[bjorn3]

Behind a feature that is off by default I think using weak linkage should be fine.

[danakj]

#526 will provide a feature to mark everything weak, that is off by default. I did it in such a way so as to avoid having to make all explicit weak requests conditional on the feature being disabled, without them colliding with the weak linkage specified in the macro.

[danakj]

Thanks for the support on this issue