Differential D77776
[Driver] Default to libc++ on FreeBSD Authored by emaste on Apr 8 2020, 9:37 PM.
Details
Downstream may naively translate between DSL and LLVM target triple. If OS version is lost in the process then Clang would default to a version that's no longer supported by OS vendor. Example: https://bugzilla.mozilla.org/show_bug.cgi?id=1628567 $ cat a.cc #include <type_traits> $ clang++ -c a.cc $ clang++ --target=x86_64-unknown-freebsd -c a.cc a.cc:1:10: fatal error: 'type_traits' file not found #include <type_traits> ^~~~~~~~~~~~~
Diff Detail
Unit Tests Event TimelineComment Actions Alternatively, the checks could be changed to also handle OSMajorVersion == 0 and translate that to 10. Comment Actions Yeah, I think that is much better. If people want to explicitly target foobar-freebsd9, they should be free to do so. But the default should indeed be either the host version, or if that cannot be determined, the lowest supported version, which is currently 10. For these particular features the exact version doesn't matter that much though, it's just about whether libc++ should be the default. Comment Actions
yep, I've had this patch applied (wrote yesterday to fix building Rust in Ports without seeing this issue, haha): --- lib/Support/Triple.cpp.orig 2020-04-10 20:51:46 UTC +++ lib/Support/Triple.cpp @@ -14,6 +14,9 @@ #include "llvm/Support/Host.h" #include "llvm/Support/TargetParser.h" #include <cstring> +#ifdef __FreeBSD__ +#include <sys/param.h> +#endif using namespace llvm; StringRef Triple::getArchTypeName(ArchType Kind) { @@ -1073,6 +1076,13 @@ void Triple::getOSVersion(unsigned &Major, unsigned &M OSName.consume_front("macos"); parseVersionFromName(OSName, Major, Minor, Micro); + + if (getOS() == FreeBSD && Major == 0) +#ifdef __FreeBSD__ + Major = __FreeBSD_version / 100000; +#else + Major = 12; +#endif } bool Triple::getMacOSXVersion(unsigned &Major, unsigned &Minor, I don't see any other OS headers included in this file, so idk about code style, but the general idea should be like this. // btw, reported to cc-rs a while ago https://github.com/alexcrichton/cc-rs/issues/463 and came to the conclusion that this unversioned target must be coming from Ports somewhere o_0 Comment Actions
Where is this defined? Does someone build LLVM master on FreeBSD < 11.3? ports/ (for dependencies) quickly cut off EOL support to avoid bitrot due to lack of QA. Comment Actions Not certain, but there are a number of FreeBSD consumers still shipping products based on FreeBSD 10 who might be trying to use newer toolchains. I don't think we need to be too concerned about those cases but if it's trivial to avoid breaking them we might as well. Comment Actions I don't like the fact that this only changes one of the users of getTriple().getOSMajorVersion(). Could you add a new member function such as void FreeBSD::getMajorVersion() const {
unsigned Major = getTriple().getOSMajorVersion();
if (Major == 0)
return 10;
return Major
}and replace all uses of getTriple().getOSMajorVersion() with getMajorVersion(). +#ifdef __FreeBSD__ + return __FreeBSD_version / 100000; +#else + return 10; +#endif Comment Actions Why not, if this is a one-off case, it's perfectly OK to put it in here. Maybe add a comment as to why it is needed? Could you add a new member function such as
Maybe other OSes would also have this same issue, but then you'd have to replace this kind of logic for *all* of them, not only FreeBSD. That said, maybe there aren't so many places where the major version is checked, and where features are enabled or disabled depending on this version?
No, that would hardcode something at build time. We need to respect whatever triple the user is passing in. Comment Actions Nobody has proposed not respecting that, this is all in the context of the if (Major == 0) fallback Comment Actions Aha, then there is no need to do arithmetic with __FreeBSD_version, as __FreeBSD__ already contains just the major version. Comment Actions It's been over a year and no solutions have been merged! The problem just popped up again in Firefox after a pull. With clang 13.0, and the discussion above was before 11.0 release >_< Can someone commit something already?!
| ||||||||||||||
In main this is amd64-unknown-freebsd14.0, I have a fix for that staged before this diff
commit d99411fe6e0375041b80ba4f397a139218a3754d Author: Ed Maste <emaste@FreeBSD.org> Date: Mon Nov 22 11:26:55 2021 -0500 [Driver] correct typo in FreeBSD 14 test The test specified amd64-unknown-freebsd40.0 rather than 14.0. Since 40 is greater than 14 the test (for behaviour new in FreeBSD 14) worked despite the typo. Fixes: 699d47472c3f diff --git a/clang/test/Driver/freebsd.cpp b/clang/test/Driver/freebsd.cpp index fde888902e12..d199f6e2367a 100644 --- a/clang/test/Driver/freebsd.cpp +++ b/clang/test/Driver/freebsd.cpp @@ -5,7 +5,7 @@ // CHECK-TEN: "-lc++" "-lm" // CHECK-NINE: "-lstdc++" "-lm" -// RUN: %clangxx %s -### -pg -o %t.o -target amd64-unknown-freebsd40.0 -stdlib=platform 2>&1 \ +// RUN: %clangxx %s -### -pg -o %t.o -target amd64-unknown-freebsd14.0 -stdlib=platform 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-PG-FOURTEEN %s // RUN: %clangxx %s -### -pg -o %t.o -target amd64-unknown-freebsd10.0 -stdlib=platform 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-PG-TEN %s