Fix --target cflag on FreeBSD when compiling against clang #785

mqudsi · 2023-02-01T19:27:44Z

Clang does not/did not support being invoked with a target triple that does not contain at least the major os version tacked on to the end, and will fail to find standard library headers if it's not present 0.

e.g. invoking clang++ with --target=x86_64-unknown-freebsd may present errors like

warning: In file included from src/cxx.cc:1:
warning: src/../include/cxx.h:2:10: fatal error: 'algorithm' file not found
warning: #include <algorithm>
warning:          ^~~~~~~~~~~
warning: 1 error generated.

error: failed to run custom build command for `cxx v1.0.81`

This is rectified by detecting cases where clang is used and the target triple is set to xxx-xxx-freebsd and then invoking uname -r to obtain the os version and appending it to the triple before converting it into a --target=... argument to the compiler.

This has been fixed upstream in the LLVM project for Clang 14 and above 1, but systems not running the latest bleeding edge will not benefit from that. This issue may be reproduced with clang 13.0.0 on FreeBSD 13.1.

Closes #463.

cc @valpackett

Clang does not/did not support being invoked with a target triple that does not contain at least the major os version tacked on to the end, and will fail to find standard library headers if it's not present [0]. e.g. invoking `clang++` with `--target=x86_64-unknown-freebsd` may present errors like ``` warning: In file included from src/cxx.cc:1: warning: src/../include/cxx.h:2:10: fatal error: 'algorithm' file not found warning: #include <algorithm> warning: ^~~~~~~~~~~ warning: 1 error generated. error: failed to run custom build command for `cxx v1.0.81` ``` This is rectified by detecting cases where clang is used and the target triple is set to `xxx-xxx-freebsd` and then invoking `uname -r` to obtain the os version and appending it to the triple before converting it into a `--target=...` argument to the compiler. This has been fixed upstream in the LLVM project for Clang 14 and above [1], but systems not running the latest bleeding edge will not benefit from that. This issue may be reproduced with clang 13.0.0 on FreeBSD 13.1. Closes rust-lang#463. [0]: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238556 [1]: https://reviews.llvm.org/D77776

src/lib.rs

dot-asm · 2023-02-01T21:31:27Z

src/lib.rs

+                                .output()
+                                .expect("Failed to execute uname!")
+                                .stdout,
+                        )


cc-rs never makes the assumption that target is related uname output. However, it should be possible to make an assumption that if self.get_host() is the same as target, then clang would produce desired result without any --target option. I [for one] would even argue that it would be possible to generalize and make such assumption for all platforms, and replace the else below with else if self.get_host()? != target. One can even wonder if it would be appropriate to take it to the outer if...

Yeah we should only do this in non-cross compilation situations. I also think we probably shouldn't use uname -r but freebsd-version, as is done here: https://github.com/rust-lang/libc/blob/106b57a796145c42f9a385a2a8cf764abbc66624/build.rs#LL160C46-L160C61

@dot-asm Thanks for the self.get_host() hint; I wasn't sure how to find out if cross-compilation was intended or not. My preference was for excluding the triple altogether (this is the preference of the FreeBSD team, but read on), however I assumed that would not be palatable. I'm glad to hear it's something you're open to!

There is a very important caveat though: this only holds if the compiler is clang(\+\+)?(-[0-9.]+)? or cc|c\+\+. If $CXX or $CC was set and resulted in a different compiler being selected, we would need to specify and pass along the triple since there's no guarantee that the chosen compiler is native to the target (regardless of what self.get_host() returns, I believe). (This is at least the case if using a multiarch gcc install on Linux, at any rate.)

I'm not sure what we're supposed to do if the target triple does differ from the host triple. I would say we can just assume the triple has been set correctly in that case, but the problem is that cargo/rust triples differ from the llvm/clang ones (in that the version number is not appended). So the triple may very well be correct/valid for rust but insufficient for the targeted FreeBSD release. If we're explicitly in the cross-compilation branch, it doesn't make much sense to use the host uname/freebsd-version output. At the same time, I'm not aware of any mechanism that would allow us to introspect the actual intended target (which may not be available/live in the first place).

I feel like the answer there would require patching this logic elsewhere so that a user could invoke a build w/ target set to x86_64-unknown-freebsd9 and cargo/rust would use the first part of that (without the trailing version number) but pass the whole thing to the compiler?

I feel like the answer there would require patching this logic elsewhere so that a user could invoke a build w/ target set to x86_64-unknown-freebsd9

I think we support things like setting the CC env var to something with flags, such as CC="clang --target=x86_64-unknown-freebsd9". The code intends to support this anyway, but there might be some issue in this particular case I'm missing.

I didn't delve into the code deep enough to figure it out, but the question I see from here is whether or not cmd.push_cc_arg("--target=xxx-yyy-zzz") will defer to an existing CFLAGS/CPPFLAGS/CXXFLAGS-derived --target=aaa-bbb-ccc value (rather than overwrite or append to it).

I'm glad to hear it's something you're open to!

Just in case, I'm just a cc-rs user, just like you. In other words what I'm open to doesn't have more weight than what you're open to :-)

x86_64-unknown-freebsd9

Keep in mind that cc-rs is not responsible for linking. In other words if it's actually a significant/hard requirement, then wouldn't you have to find a way to make the rustc pass the same flag to the linker? But it's just straightforward cc even(*) in this case, right? This is basically why I suggested that it's possible to make the assumption that self.gethost()? == target warrants --target-less invocation. Because that's what rustc would normally do. Now, as for cross-compilation cases. Normally you'd have to explicitly specify the cross-linker, and it would be more appropriate to use that information to deduce the command-line for use in cc-rs. More appropriate than assuming this-or-that based on uname -r or freebsd-version ;-)

(*) "even" is a reference to Linux.

I am treating compilation and linking as separate problems to solve; presumably if a linker was specified then either it is by default the correct linker for the target platform (i.e. correct target is hard-coded into its executable) or the required configuration variables have also been set in the environment to get it to work. If you rely on clang to execute the linker and you're not doing anything advanced, I'm pretty sure just executing clang w/ the correct --target and the default --drive-mode then it'll (try to) invoke the linker correctly.

I'm also guessing basic cross-compilation and cross-linking is working or we'd have heard by now? (I have been wrong before!)

I am treating compilation and linking as separate problems to solve;

But they are not completely independent, things have to align. Not necessarily perfectly, but aligned nevertheless.

I'm pretty sure just executing clang w/ the correct --target and the default --drive-mode then it'll (try to) invoke the linker correctly.

And the point is that you have no control over --target passed to the linker. Not passing --target to cc simply mirrors the link stage and effectively aligns C compiler with Rust linker.

As for remark about FreeBSD people preferring target triplet over relying on --target-less invocation to target the current environment. Is it actually so that the option to not specify --target to match the current environment is about to disappear? Or is it rather that they want the option to target OS versions other than the current(*)? I bet it's the latter. But in such case what do we achieve with the suggested modification? Do we give the user the option to choose(*)? No, rather contrary actually, the user gets locked to the current system with no escape. Well, what do I know, maybe it is in fact the actual goal :-)

(*) EDIT. In the context "choice" refers to OS version, not hardware platform.

mqudsi · 2023-02-01T23:59:31Z

I pushed an update containing the requested fixes, but without eliding --target in case self.get_host() == target. As the surrounding code currently stands, we would actually just have an empty else if target.ends_with("-freebsd") && self.get_host()?.eq(target) { branch containing just the comment clarifying that this parameter isn't needed if we're using clang and targeting FreeBSD.

I guess I was left a bit unsure by the simultaneous request to use freebsd-version and to check if the host and target match. I'm not seeing the case where we would query the system version at all if the host and target match?

I know that unlike gcc, clang is natively a cross-compiler. But does that mean we can assume ToolFamily::Clang always means the default target for whatever $CC/$CXX is set to will also match the host? It seemed safer to just keep assigning the --target as before, just specifically addressing clang 13's needs rather than potentially breaking a configuration I'm not taking into account.

EDIT:

Note that the name of the clang binary may automatically infer the target triple, e.g. <target-triple>-clang is akin to executing clang --target=<target-triple>. If that name information isn't already being incorporated, then it is indeed incorrect to assume ToolFamily::Clang implies we can just omit the target because if $CC is set to i686-unknown-freebsd-clang on an x86_64-unknown-freebsd host but the target isn't overridden when cargo/rust is invoked, we'd be invoking it to build without a triple but the hard-coded default triple would be used instead. Arguably user error, but it would be a regression.

* Use `freebsd-version` instead of `uname`. This returns the currently installed userland version, which is a closer match to the desired target triple than the running kernel version. * Only match `freebsd-vesion` or `uname` version output if we are compiling to the same target as the running host. * Properly return errors.

src/lib.rs

thomcc · 2023-02-02T00:39:22Z

This looks fine to me. Barring issues, I'll try to get this into the release I'm planning on doing this weekend.

CC @devnexen who often contributes stuff for FreeBSD (and seems to be somewhat involved in the project) in https://github.com/rust-lang/libc, in case I'm missing something.

mqudsi · 2023-02-02T00:41:18Z

Awesome - thanks for reviewing this so swiftly!

dot-asm · 2023-02-02T12:12:33Z

Note that the name of the clang binary may automatically infer the target triple,

I would strengthen this statement as "the [prefixed] name does automatically infer the target triple." [Otherwise why would it have the prefix to begin with?] Then take it further and say that it's not necessarily appropriate to pass the triple to a prefixed compiler command. Because it might be more nuanced than cc-rs can possibly anticipate. Which formally leads us to the conclusion that passing --target to an explicitly prefixed command should be considered error-prone.

if $CC is set to i686-unknown-freebsd-clang on an x86_64-unknown-freebsd host ... user error, but it would be a regression,

I can't pinpoint it, but I sense a misconception hiding here. It's not $CC that determines the Rust target, but --target argument passed to cargo[!]. Then it's the user's responsibility to instruct Rust how to link(*). Customarily it's done by specifying a suitably prefixed C cross-compiler command. I don't know if it's the most common, but I do it through linker assignment in .cargo/config.toml. Only now we're getting to the question which C/C++ compiler is called to compile C/C++ and which flags to pass. cc-rs attempts to make it work without user doing anything special(**), but $CC has an absolute precedence. So if the user sets $CC to point at an incompatible compiler, it's on the user. Always was. So it's not clear what regression we are talking about here. I mean $CC mismatching the --target passed to cargo was always problematic... Or am I missing the point you're trying to make?

(*) Well, with exception at least for bare-metal targets and Windows. Maybe even something else, but not multi-user systems AFAIK. Either way, essential to note that $CC has no effect on this step. Question is how to link a pure Rust hello-world.rs.
(**) and occasionally gets it wrong. If it didn't, we wouldn't have this discussion :-)

mqudsi · 2023-02-02T18:31:07Z

I would strengthen this statement as "the [prefixed] name does automatically infer the target triple."

Of course, but I didn't say "a triple-prefixed compiler name [might change the default target]" but rather the less general "the name of the binary [might change the default target]". I have clang-xx symlinked as ccc for $reasons but that obviously doesn't change its behavior in this regard :)

(I also don't know if clang inspects argv[0] to figure out if it's being executed as <target-triple>-clang and automatically changes the default target or if <target-triple>-clang is a shim that executes clang --target=<target-triple> and hedged my bets so my statement would apply even in case of the latter, although I'm guessing it's the former.)

It's not $CC that determines the Rust target, but --target argument passed to cargo[!].
[...] Or am I missing the point you're trying to make?

Yes; I'm specifically speaking about native host == target compilation with no explicit --target passed to cargo. If you used env CC=i686-unknown-freebsd-clang cargo build and if we (cc-rs) didn't pass --target= to the compiler because we detected that self.get_host() == target as I mentioned (which implies no cross-compilation and either no --target passed to cargo or else passed --target=<same as host>), $CC's default target would kick in. This was the edge case I had a nagging feeling about way above: w/ no --target passed to $CC, in this case this particular compiler's default target would be incorrect and would not match the host (despite it being ToolFamily::Clang). (Which was the "important caveat" I mentioned about the binary name strictly matching one of the two regular expressions, ^ and $ anchored.)

As I said, this specific (hypothetical) regression could of course be chalked down to end user error, but it's not necessarily going to be as obvious an infraction as the contrived example above if $CC were set separately, inherited from some other tool or build system, or several scripts/aliases distant.

Due to an upstream issue with cc-rs [0], the rust-generated C++ interface would fail to compile. A PR has been opened to patch the issue upstream [1], but in the meantime `Cargo.toml` has been patched to use a fork of cc-rs with the relevant fixes. [0]: rust-lang/cc-rs#463 [1]: rust-lang/cc-rs#785

devnexen · 2023-02-02T20:01:30Z

This looks fine to me. Barring issues, I'll try to get this into the release I'm planning on doing this weekend.

CC @devnexen who often contributes stuff for FreeBSD (and seems to be somewhat involved in the project) in https://github.com/rust-lang/libc, in case I'm missing something.

Looks fine by me too.

thomcc

I'm happy with this change. Hopefully it doesn't break anything, but we can address it if it happens to.

Due to an upstream issue with cc-rs [0], the rust-generated C++ interface would fail to compile. A PR has been opened to patch the issue upstream [1], but in the meantime `Cargo.toml` has been patched to use a fork of cc-rs with the relevant fixes. [0]: rust-lang/cc-rs#463 [1]: rust-lang/cc-rs#785

dot-asm · 2023-02-05T16:30:26Z

Cross-compilation was mentioned, but does the suggested modification fix at least corresponding problem with env CXX=clang++ cargo build --target=i686-unknown-freebsd executed on x86_64 FreeBSD? No. Hence #788.

dot-asm · 2023-02-05T16:47:50Z

Or am I missing the point you're trying to make?

Yes;

Ah! You refer to the fact that a-b-c-clang --target=x-y-z apparently works and produces x-y-z binary [code]. This is an unexpected turn to me. Yet I would still argue that it would be more appropriate to not pass --target=x-y-z to a prefixed command line. It's simply not safe assumption to make. As you don't really know where does it come from and that it will take you to the intended headers. I would even say that a warning about mismatch between a-b-c-compiler and [the current] target would be appropriate...

This avoids certain corner cases where supplying the same llvm triple used by rust for clang++ can cause native (host == target) C++ builds to break. This fix supersedes rust-lang#785. See relevant discussions in rust-lang#785 and rust-lang#788.

When cross compiling, we are forced to emit the `--target=<triple>` compiler flag, but this can break clang++ C++ builds on FreeBSD if the triple isn't set to a value that causes clang++ to infer the use of libc++ instead of libstdc++ (the latter of which does not support C++11 and beyond on FreeBSD). By manually specifying the usage of `-std=libc++` we avoid the need to know which version of FreeBSD we are targeting (as we do not have that information from the rust target triple and cannot infer it from the host as we are cross-compiling). Note that we were already specifying a default stdlib value libc++ for `cpp_link_stdlib` under FreeBSD but that is not sufficient. This fix supersedes rust-lang#788. See relevant discussions in rust-lang#785 and rust-lang#788.

When cross compiling, we are forced to emit the `--target=<triple>` compiler flag, but this can break clang++ C++ builds on FreeBSD if the triple isn't set to a value that causes clang++ to infer the use of libc++ instead of libstdc++ (the latter of which does not support C++11 and beyond on FreeBSD). By manually specifying the usage of `-std=libc++` we avoid the need to know which version of FreeBSD we are targeting (as we do not have that information from the rust target triple and cannot infer it from the host as we are cross-compiling). Note that we were already specifying a default stdlib value libc++ for `cpp_link_stdlib` under FreeBSD but that is not sufficient. There are other solutions that would work when not cross compiling such as omitting the `--target` altogether, but there's no compelling reason to add more code if this solution works for both native and cross-compiling cases. This fix supersedes rust-lang#788. See relevant discussions in rust-lang#785 and rust-lang#788.

mqudsi mentioned this pull request Feb 1, 2023

Rewrite it in Rust fish-shell/fish-shell#9512

Merged

thomcc requested changes Feb 1, 2023

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

dot-asm reviewed Feb 1, 2023

View reviewed changes

mqudsi force-pushed the freebsd-triple-fix branch from 5a61ea9 to fba2456 Compare February 1, 2023 23:50

mqudsi force-pushed the freebsd-triple-fix branch from fba2456 to 0e9373f Compare February 2, 2023 00:24

thomcc reviewed Feb 2, 2023

View reviewed changes

src/lib.rs Show resolved Hide resolved

thomcc approved these changes Feb 2, 2023

View reviewed changes

thomcc merged commit 1997951 into rust-lang:main Feb 2, 2023

mqudsi mentioned this pull request Feb 7, 2023

Append freebsd-version to all --target=*-freebsd if executed on FreeBSD. #788

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix --target cflag on FreeBSD when compiling against clang #785

Fix --target cflag on FreeBSD when compiling against clang #785

mqudsi commented Feb 1, 2023 •

edited

Loading

dot-asm Feb 1, 2023

thomcc Feb 1, 2023

mqudsi Feb 1, 2023

thomcc Feb 1, 2023 •

edited

Loading

mqudsi Feb 1, 2023 •

edited

Loading

dot-asm Feb 2, 2023

mqudsi Feb 2, 2023

dot-asm Feb 2, 2023 •

edited

Loading

mqudsi commented Feb 1, 2023 •

edited

Loading

thomcc commented Feb 2, 2023

mqudsi commented Feb 2, 2023

dot-asm commented Feb 2, 2023

mqudsi commented Feb 2, 2023 •

edited

Loading

devnexen commented Feb 2, 2023

thomcc left a comment

dot-asm commented Feb 5, 2023

dot-asm commented Feb 5, 2023 •

edited

Loading

Fix --target cflag on FreeBSD when compiling against clang #785

Fix --target cflag on FreeBSD when compiling against clang #785

Conversation

mqudsi commented Feb 1, 2023 • edited Loading

dot-asm Feb 1, 2023

Choose a reason for hiding this comment

thomcc Feb 1, 2023

Choose a reason for hiding this comment

mqudsi Feb 1, 2023

Choose a reason for hiding this comment

thomcc Feb 1, 2023 • edited Loading

Choose a reason for hiding this comment

mqudsi Feb 1, 2023 • edited Loading

Choose a reason for hiding this comment

dot-asm Feb 2, 2023

Choose a reason for hiding this comment

mqudsi Feb 2, 2023

Choose a reason for hiding this comment

dot-asm Feb 2, 2023 • edited Loading

Choose a reason for hiding this comment

mqudsi commented Feb 1, 2023 • edited Loading

thomcc commented Feb 2, 2023

mqudsi commented Feb 2, 2023

dot-asm commented Feb 2, 2023

mqudsi commented Feb 2, 2023 • edited Loading

devnexen commented Feb 2, 2023

thomcc left a comment

Choose a reason for hiding this comment

dot-asm commented Feb 5, 2023

dot-asm commented Feb 5, 2023 • edited Loading

mqudsi commented Feb 1, 2023 •

edited

Loading

thomcc Feb 1, 2023 •

edited

Loading

mqudsi Feb 1, 2023 •

edited

Loading

dot-asm Feb 2, 2023 •

edited

Loading

mqudsi commented Feb 1, 2023 •

edited

Loading

mqudsi commented Feb 2, 2023 •

edited

Loading

dot-asm commented Feb 5, 2023 •

edited

Loading