Cannot opt out of LLVM MergeFunctions pass on stable

[michaelwoerister]

LLVM's MergeFunctions introduces aliases which can trigger a bug in macOS's ld64 linker when compiling with ThinLTO: https://github.com/froydnj/ld64-aliases-bug

For this reason Firefox has to be built with a patched version of ld64 when compiling with xLTO. Since the bug in question is pretty nasty (the above example shows a program just returning a different value with no indication that anything is wrong), we should probably try to not trigger it.

Right now, however, it seems that one cannot even turn the MergeFunctions pass off in stable Rust. (I wonder if the pass should even be disabled by default on macOS?)

cc @rust-lang/compiler, @rust-lang/wg-codegen & @peterhj (this seems related to #57356)

[nox]

Is it a mergefunc bug or a ThinLTO bug? Can it be triggered without ThinLTO? If not, I would rather not blindly disable it on macOS, at least not when ThinLTO isn't enabled.

[michaelwoerister]

It's not a bug in mergefunc. It might not even be a bug in ThinLTO. It's just that mergefunc is the only thing in rustc that generates symbol aliases in LLVM IR, and these are what can lead to "miscompilations" when using the macOS linker.

[nikic]

Has the ld64 bug been reported somewhere?

[nox]

It's not a bug in mergefunc. It might not even be a bug in ThinLTO. It's just that mergefunc is the only thing in rustc that generates symbol aliases in LLVM IR, and these are what can lead to "miscompilations" when using the macOS linker.

We could use mergefunc-use-aliases to prevent the pass from emitting aliases on macOS, AFAIK.

https://github.com/llvm-mirror/llvm/blob/2afd9e698dbfc2f3011baf3bfdb47d03c253272b/lib/Transforms/IPO/MergeFunctions.cpp#L168-L171

[nikic]

@nox This is already supported via either -Z merge-functions=trampolines or merge_functions: MergeFunctions::Trampolines in the target spec.

[nox]

@nikic Do you mean that the test case also fails with mergefunc-use-aliases enabled, or that we already have a knob to disable them?

[nox]

rust/src/librustc_codegen_llvm/llvm_util.rs

Lines 65 to 74 in 185b9ac

	if get_major_version() >= 8 {
	match sess.opts.debugging_opts.merge_functions
	.unwrap_or(sess.target.target.options.merge_functions) {
	MergeFunctions::Disabled |
	MergeFunctions::Trampolines => {}
	MergeFunctions::Aliases => {
	add("-mergefunc-use-aliases");
	}
	}
	}

Answering my own question, we already have a knob for that.

[nox]

Which was added because aliases already cause issues on other architectures.

[peterhj]

As @nikic and @nox point to there is the unstable -Z merge-functions option which I've only tested on nightly. I guess there could be a stabilized -C merge-functions option, but maybe what you really want to do is to patch the x86_64 macos target definition (I believe this is the correct one: https://github.com/rust-lang/rust/blob/97ba4c95d00108ac79c86d2bbc6834b1fef008a2/src/librustc_target/spec/x86_64_apple_darwin.rs).

[michaelwoerister]

@nikic

Has the ld64 bug been reported somewhere?

Yes, I've been told that it has been reported to Apple.

On nightly, one can already control the mergefunc pass. On stable, however, one is stuck with the default behavior. That's what this issue is mostly about.

[pnkfelix]

@michaelwoerister just want to double-check my understanding: I would have thought one could use the -C options -C no-prepopulate-passes and -C passes=val to override the set of passes, even on the stable channel.

Am I incorrect about this?

(If I am correct, I nonetheless will admit that this is an ugly (and very fragile) method to opt out of the MergeFunctions pass)