Differential D22034

Use __FreeBSD_version to determine if gets() has been removed.
ClosedPublic
Actions

Authored by jhb on Oct 14 2019, 10:47 PM.

Details

Reviewers

emaste
dim
imp

Commits

rS353579: Use __FreeBSD_version to determine if gets() has been removed.

Summary

GCC compilers set FreeBSD statically to a build-time determined
targeted version (which in ports always matches the build host's
version). This means that when building any version (12 or 13, etc.)
of riscv or some other architecture via GCC on a 12.x host,
FreeBSD will always be set to 12. As a result, FreeBSD cannot
be used to reliably detect the target FreeBSD version being built.
Instead, __FreeBSD_version from either <sys/param.h> (in the kernel)
or <osreldate.h> (in userland) should be used.

This changes the gets() test in libc++ to use __FreeBSD_version from
<osreldate.h>.

Reported by: jenkins (amd64-gcc and riscv64)

Test Plan

riscv64 can build on a 12.x host now without erroring in algorithm.o
amd64-gcc jenkins jobs are failing due to the same error currently

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jhb created this revision.Oct 14 2019, 10:47 PM

Harbormaster completed remote builds in B27037: Diff 63277.Oct 14 2019, 10:47 PM

imp accepted this revision.Oct 15 2019, 2:18 AM

This revision is now accepted and ready to land.Oct 15 2019, 2:18 AM

Hmm, I had explicitly avoided to add any #include statements, due to there being very few of them in this header (it is included by literally everything in libc++). I would rather fix the external toolchains instead, to return the correct version of FreeBSD. But if people feel this is the right way to solve the issue, so be it. But maybe we should not send this upstream.

In D22034#481238, @dim wrote:

Hmm, I had explicitly avoided to add any #include statements, due to there being very few of them in this header (it is included by literally everything in libc++). I would rather fix the external toolchains instead, to return the correct version of FreeBSD. But if people feel this is the right way to solve the issue, so be it. But maybe we should not send this upstream.

The problem is that the toolchain can't be fixed. The host version is used, not the target... the compiler has no way of knowing...

GCC can't really be fixed. It doesn't have a runtime equivalent of --target and FreeBSD is hardcoded based on what was configured at runtime. It's not practical to have the GCC ports build and install per-OS version compilers (e.g. devel/powerpc64-gcc would have to install separate binaries for freebsd11 vs freebsd12 vs freebsd13).

Ah, I now realized that <osreldate.h> is a very minimal header, I was thinking that it would pull in lots of pollution, like <sys/param.h>... In that case it is probably the least ugly solution.

In D22034#481338, @dim wrote:

Ah, I now realized that <osreldate.h> is a very minimal header, I was thinking that it would pull in lots of pollution, like <sys/param.h>... In that case it is probably the least ugly solution.

Yes, I really didn't want to pull in sys/param.h due to all the pollution and it took me a while to find osreldate.h which is more palatable.

Closed by commit rS353579: Use __FreeBSD_version to determine if gets() has been removed. (authored by jhb). · Explain WhyOct 15 2019, 6:16 PM

This revision was automatically updated to reflect the committed changes.

jhb added a commit: rS353579: Use __FreeBSD_version to determine if gets() has been removed..

dim mentioned this in rS356460: Merge llvm, clang, compiler-rt, libc++, libunwind, lld, lldb and openmp.Jan 7 2020, 6:13 PM

dim mentioned this in rS360658: Merge llvm, clang, compiler-rt, libc++, libunwind, lld, lldb and openmp.May 5 2020, 1:05 PM