bootstrap: LLVM build disregards LDFLAGS

[jonhoo]

When the bootstrap code builds LLVM, it sometimes sets the CMake variable CMAKE_EXE_LINKER_FLAGS. For example, when instructed to also build the LLVM tools:

rust/src/bootstrap/native.rs

Lines 242 to 250 in 502d6aa

	if builder.config.llvm_tools_enabled {
	if !target.contains("msvc") {
	if target.contains("apple") {
	cfg.define("CMAKE_EXE_LINKER_FLAGS", "-static-libstdc++");
	} else {
	cfg.define("CMAKE_EXE_LINKER_FLAGS", "-Wl,-Bsymbolic -static-libstdc++");
	}
	}
	}

When CMAKE_EXE_LINKER_FLAGS is unset, CMake initializes it to LDFLAGS, but it does not consider LDFLAGS if CMAKE_EXE_LINKER_FLAGS is set. This means that Rust's LLVM bootstrap will sometimes respect (potentially required) LDFLAGS and sometimes not depending on a number of unexpected factors, such as whether llvm tools are also being built, whether we're building for a non-FreeBSD RISC-V target, or if we're building for powerpc-unknown-freebsd. This, in turn, makes it difficult to bootstrap Rust in situations where additional linker flags are required, such as if dependencies are not stored in standard system-wide location such as /usr/lib.

I propose that anywhere we set CMAKE_EXE_LINKER_FLAGS, we append LDFLAGS from the environment to the value to retain parity with the behavior when the configuration isn't set.

[Mark-Simulacrum]

That makes sense to me, would be happy to accept a PR. It may also make sense to ask/file a similar bug against llvm recommending the logic to append always happen, since it seems like better behavior to me.

[jonhoo]

I don't think this is really a bug in llvm; this is a standard CMAKE flag, and the observed behavior is expected behavior; changing it would be a breaking change to cmake sadly.

I'd be happy to push a PR, though looking through the code raised a few other questions for me:

---

First, when llvm.ldflags is set, the current implementation sets the CMAKE_*_LINKER_FLAGS variables unilaterally

rust/src/bootstrap/native.rs

Lines 553 to 557 in 502d6aa

	if let Some(ref s) = builder.config.llvm_ldflags {
	cfg.define("CMAKE_SHARED_LINKER_FLAGS", s);
	cfg.define("CMAKE_MODULE_LINKER_FLAGS", s);
	cfg.define("CMAKE_EXE_LINKER_FLAGS", s);
	}

And since configure_cmake is run after all the logic that sets the linker flags based on other parts of the config, here:

rust/src/bootstrap/native.rs

Line 354 in 502d6aa

configure_cmake(builder, target, &mut cfg, true);

It means that llvm.ldflags is currently an override, not an append. Do we want to keep that behavior, meaning that if llvm.ldflags is set in config.toml, LDFLAGS will not be appended? Should we document that llvm.ldflags overrides any ldflags the bootstrap may want to set? Or should we change it to be appending the way llvm.cflags and llvm.cppflags are?

---

Second, the current bootstrap logic sets CMAKE_EXE_LINKER_FLAGS multiple times:

rust/src/bootstrap/native.rs

Line 247 in 502d6aa

cfg.define("CMAKE_EXE_LINKER_FLAGS", "-Wl,-Bsymbolic -static-libstdc++");

rust/src/bootstrap/native.rs

Line 263 in 502d6aa

cfg.define("CMAKE_EXE_LINKER_FLAGS", "-latomic -static-libstdc++");

rust/src/bootstrap/native.rs

Line 312 in 502d6aa

cfg.define("CMAKE_EXE_LINKER_FLAGS", "-fuse-ld=bfd");

Most of those are mutually exclusive, but when they're not, the later setting takes precedence. Is this intentional, or should the logic be fixed so that ldflags is added to, and then set in a single place at the end?

Anecdotally, we handle CFLAGS by appending:

rust/src/bootstrap/native.rs

Lines 522 to 525 in 502d6aa

	if builder.config.llvm_clang_cl.is_some() {
	cflags.push_str(&format!(" --target={}", target))
	}
	cfg.define("CMAKE_C_FLAGS", cflags);

and same with CXXFLAGS:

rust/src/bootstrap/native.rs

Lines 530 to 536 in 502d6aa

	if let Some(ref s) = builder.config.llvm_cxxflags {
	cxxflags.push_str(&format!(" {}", s));
	}
	if builder.config.llvm_clang_cl.is_some() {
	cxxflags.push_str(&format!(" --target={}", target))
	}
	cfg.define("CMAKE_CXX_FLAGS", cxxflags);

---

Third, it turns out we have a similar problem with CFLAGS and CXXFLAGS, except those we always override, so CFLAGS and CXXFLAGS from the environment is never respected. Should that be fixed too?

[Mark-Simulacrum]

I'm pretty sure that most of this behavior is unintentional and can largely be explained by incremental changes without really looking above/below in the file to make sure things aren't being set twice etc. I'm not particularly worried about breaking changes in bootstrap (e.g., if we start respecting LDFLAGS or CXXFLAGS etc), particularly when we're moving to what seems like relatively cleaner model.

In general happy to see a PR that goes through and cleans the logic up.

[jonhoo]

PR: #93918