Some mixes of Rust with C/C++ are broken for arm64 mac and windows

[glandium]

After #88354, Firefox builds fail with e.g:

LLVM ERROR: Function Import: link error: linking module flags 'branch-target-enforcement': IDs have conflicting behaviors in '/builds/worker/workspace/obj-build/aarch64-pc-windows-msvc/release/gkrust_gtest.libnsstring-be03ad04cbf7bb1a.nsstring.ce1e3b11-cgu.0.rcgu.o571656940' and '../../../dom/media/webrtc/jsapi/Unified_cpp_media_webrtc_jsapi0.obj'

This happens when targeting arm64 macos or arm64 windows.

Not only did the default change and now diverges from clang (causing the above error when mixing objects from clang and rust), but it's also apparently not possible to disable the feature.

Note this also now reached beta.

Cc: @jacobbramley, @Jmc18134, @nagisa

[jacobbramley]

It should be off by default. My first guess is that the problem is that we set the module flag unconditionally, but with a true/false value, so perhaps a false value is not equivalent to an unset value.

If that is the problem then there might be something deeper to investigate here, like some BTI mismatch between the Rust and C/C++ code that wasn't previously spotted. It would be interesting to know if the C/C++ code is compiled with BTI enabled.

I don't have Firefox (or recent Rust) build environment on my M1 Mac so it'll take me a while to reproduce. I'll make a start, but if others can get there first and unblock this quickly, then please do! We could also revert #88354 from beta whilst we investigate. I'm not sure of the process there.

[glandium]

It most probably involves linker-plugin-lto. A default firefox build wouldn't enable that. I suspect it's reproducible with a simple testcase that has both C and C++ code with linker-plugin-lto enabled.

[glandium]

(beware of #60059 if you try to use linker-plugin-lto)

[glandium]

Ok, I was also able to reproduce locally with a Linux aarch64 cross-build (modulo https://bugzilla.mozilla.org/show_bug.cgi?id=1749852) and it turns out the rust llvm-ir has:

1597 = !{i32 2, !"branch-target-enforcement", i32 0}
!1598 = !{i32 2, !"sign-return-address", i32 0}
!1599 = !{i32 2, !"sign-return-address-all", i32 0}
!1600 = !{i32 2, !"sign-return-address-with-bkey", i32 0}

while the clang llvm-ir has:

!6 = !{i32 1, !"branch-target-enforcement", i32 0}
!7 = !{i32 1, !"sign-return-address", i32 0}
!8 = !{i32 1, !"sign-return-address-all", i32 0}
!9 = !{i32 1, !"sign-return-address-with-bkey", i32 0}

llvm-ir from older versions of rust didn't have them at all.

[jacobbramley]

llvm-ir from older versions of rust didn't have them at all.

Nor does the llvm-ir from older versions of clang — at least the one I have pre-built — so the simple idea of marking them as 1 (error-on-mismatch) to match the newer clang might cause different problems.

[glandium]

the simple idea of marking them as 1 (error-on-mismatch) to match the newer clang might cause different problems.

why do you think so? the llvm-ir that doesn't contain these flags at all links fine with the llvm-ir that does (even with behavior set to Error).

[jacobbramley]

the simple idea of marking them as 1 (error-on-mismatch) to match the newer clang might cause different problems.

why do you think so? the llvm-ir that doesn't contain these flags at all links fine with the llvm-ir that does (even with behavior set to Error).

It was my own (naive) assumption, based on how BTI itself works: it's not correct to treat non-BTI machine code as BTI because if it lacks the BTI instructions it will be impossible to call any of its functions. However, that's not particularly relevant as this logic is all resolved before code generation.

That leaves two candidate solutions, then:

1. Use error-on-mismatch for these flags. This is nice because it catches configuration mismatches, and will be compatible with Clang and LTO.
2. Don't emit these flags at all if they aren't enabled. This most closely matches the existing (stable) behaviour but means that configuration mismatches (or compatibility issues like this one) may be missed.

Given the behaviour you described, and the LTO compatibility statement — "Best results are achieved by using a rustc and clang that are based on the exact same version of LLVM" — I think we can reasonably prefer option 1.

I'll start working on that today.

[apiraino]

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-high

[Mark-Simulacrum]

Reopening to track beta fix (proposed in #93523).

[pnkfelix]

resolved by #93523 (on beta), #93269 on master