freebsd: Add support for FreeBSD 12.x ABI

[dumbbell]

In FreeBSD 12.x, several ABI were changed in an incompatible way:

• Inodes were changed from 32-bit to 64-bit. This affects struct stat and struct dirent structures in Rust libc.

• Time-related members of struct kevent were changed to 64-bit. Again, this affects the definition of the same structure in Rust libc.

Currently, a Rust program compiled on FreeBSD 11.x will work on FreeBSD 12.x thanks to symbol versioning. Unfortunately, a program compiled on FreeBSD 12.x will break.

Until Rust gains support for target versions, we need to detect the correct structures to use, based on the build host. This won't make it possible to build a FreeBSD 11 executable on a FreeBSD 12 host, but it is good enough when the build and target hosts are the same.

This patch introduces a build script which uses freebsd-version(1) to detect the version. When it is 12 or more, it defines the freebsd12_abi feature.

Modules in src/unix/bsd/freebsdlike/freebsd were reorganized. They are now split based on this ABI version instead of the target architecture. For instance, struct stat is defined once in freebsd12.rs for all architectures.

struct kevent grew a new member, ext in FreeBSD 12.x. To avoid too much pain to users of this structure, both flavors (freebsd11.rs and freebsd12.rs) now have the ext member. In the case of freebsd11, this is a 0-length array. To help initialize the structure, one can use the libc::KEVENT_EXT_ZEROED constant.

Fixes rust-lang/rust#42681.

[dumbbell]

I'm not sure at all if the addition of the ext member to all flavors of struct kevent and the use of a 0-length array in the case of FreeBSD up-to 11.x is a good solution. My goal was to avoid the need to detect again the version of FreeBSD to users of this structure. Unfortunately, I don't know if the 0-length array is actually 0 bytes in memory.

Here is the tentative patch in the mio crate: dumbbell/mio@d241857

With unmodified libc and mio, we get assertions such as (taken from the hyper testsuite):

---- client::connect::tests::test_errors_missing_scheme stdout ----
	thread 'client::connect::tests::test_errors_missing_scheme' panicked at 'assertion failed: `(left == right)` (left: `0`, right: `16384`)', .../mio-0.6.10/src/sys/unix/kqueue.rs:117

With the two patches, the testsuites of mio and hyper are successful on FreeBSD 10.x and FreeBSD 12.x. I admit I didn't tested the changes on DragonFlyBSD, but I just moved DragonFly structures from the generic module to the OS-specific module.

[bors]

☔ The latest upstream changes (presumably #722) made this pull request unmergeable. Please resolve the merge conflicts.

[semarie]

Having several ABI in libc for FreeBSD would be the right thing regarding the break at OS level: it permits Rust program using libc to pick the right struct depending the OS version.

One thing is still problematic: the build.rs change select the ABI based on target_os attribute: so when a Linux will target specifically FreeBSD (like for rustup distribution), it will automatically choose one ABI (FreeBSD < 12) without a way to use the other.

So we have the following target ABI selection (regarding the host):

• libc is build on FreeBSD < 12 : libc provide FreeBSD <12 ABI
• libc is build on FreeBSD >= 12 : libc provide FreeBSD >=12 ABI
• libc is build somewhere else: libc provide FreeBSD <12 ABI

All these things under *-unkown-freebsd target.

But the PR solves several problems too: FreeBSD will be able to have the right ABI when built using FreeBSD, and it is a good step from my point of vue.

I will add some elements on the proposed RFC (target-extension) regarding the ABI selection in build.rs.

[alexcrichton]

Thanks for the PR! Unfortunately though I don't think this is a good strategy for the libc crate. This is essentially boiling down to compile-time detection, but the compile-time detection is fallible and lossy. For example this wouldn't work for cross-compiled scenarios (e.g. wouldn't get the compiler nightlies working) and in general doesn't allow tuning for various versions.

This is a general problem which rust-lang/rfcs#2048 is targeted at addressing, essentially allowing communication to the compiler through separate targets what's actually being targeted, allowing libc to precisely configure itself for the platform at hand.

[dumbbell]

Thanks for your comment!

I read and commented on the draft of this RFC, but I missed the pull request on GitHub and the subsequent comments. I'm reading them and I will experiment with the runtime detection you suggest because I'm not sure I understand how it works yet. I will comment in the RFC pull request with my findings.

In particular, symbols are versioned on FreeBSD, so for instance FreeBSD 12 still knows about the struct stat from FreeBSD up-to 11 and executables compiled for FreeBSD 11 will work out-of-the-box on FreeBSD 12. That's why we can use the Rust/Cargo bootstraps without issues on 12.x.

[alexcrichton]

@dumbbell whoa interesting!

That's why we can use the Rust/Cargo bootstraps without issues on 12.x.

What's going on with issue reports like rust-lang/rust#42681 then? Should we be linking to different symbols?

[dumbbell]

By out-of-the-box, I mean the rustc and cargo runs fine and this allows us to build the package of Rust. For this package on FreeBSD 12, the libc crate is simply patched by replacing the old types/structs by the new ones.

So if I take the example in rust-lang/rust#42681, rustc from our package produces an executable which works. However, rustc from bootstraps or rustup produces an executable which crashes as demonstrated by the reporter.

My understanding is that rustc from bootstraps/rustup knows only the old struct stat layout but it's built with and linked to the FreeBSD 11 symbols. So when executed on FreeBSD 12, the correct compat symbols are picked by the runtime loader and it works fine. However, the executables it produces are linked to the new symbols (because they simply ask for e.g. stat) but they are still using the old struct stat layout. This results in the crash. I'm almost sure that if the same rustc was executed on FreeBSD 11 to produce the program and the program was executed on FreeBSD 12, it would work.

[alexcrichton]

Interesting! Can we request the old symbols be used then? That's what we do for OSX (requesting old and/or new symbols)

[main--]

Indeed, that should be possible! For example, there should be both readdir@FBSD_1.0 and readdir@FBSD_1.5 where only the latter uses 64-bit inodes.

(I don't have a FreeBSD 12 here to check this but that's what's documented here and implemented here).

[alexcrichton]

Oh that'd be awesome! If we could update the libc crate to force linkage against those older symbols, that should at least solve the problem for now!

[asomers]

@dumbbell's proposal will fix self-builds on FreeBSD 12, but as @alexcrichton commented, it won't fix cross builds. I think it should be possible to fix cross builds too by using feature flags to select which ABI to use. An on-by-default feature flag named fbsd11_compat, when enabled, will cause libc to link against readdir@FBSD_1.0. When disabled, it will cause libc to link against readdir@FBSD_1.5. I haven't tried to implement this idea yet, but I see no reason why it wouldn't work. The only downside is that FreeBSD users wanting to take advantage of 64-bit inodes will have to set the flag.

[alexcrichton]

Not that if there are symbols for both the 32 and 64 but variants of functions then we could do a linux-like interface where both stat and stat64 (and all other symbols) are exported, and then users choose which they want

[semarie]

@alexcrichton it isn't so simple.

First, I doubt the compatibility layer was done for providing an alternate way to use the system, but more for helping transition. As example, the old-stat(2) syscall (SYS_freebsd11_stat) relies on old struct stat layout, and not a recent one. So all functionalities introduced with ino_t will be limited (or broken ? I didn't check how the system behave if you use old-stat on a inode using 64 bits, so when it isn't possible to represent it in 32 bits).

But I assume it is possible to make libc to only use old interface. But it is a bit abusing the compat layer, and it will break any program that will want to use struct stat (or any other structure using ino_t) for FFI: Rust libc provides the old structure, and any use of it would require special code for dealing with the ABI break for FreeBSD, as C code will use the new layout.

It could be done for transitional purpose for rustc, but requiring the same for all Rust ecosystem would be very dangerous: problems will not be detected at compile-time, but only at runtime with wired behaviour.

[semarie]

The actual behavior for FreeBSD 12 regarding ino_t changes is to silently truncate too large 64 bits values to 32 bits.

[dumbbell]

Hi! Sorry, I was in holidays for a few couple weeks and took some time to resume work.

I played with a simple C program to learn how to link it to the old symbols. Here is the original program which uses the three (AFAIK) structures whose layout was modified in FreeBSD 12 (struct statfs, struct stat and struct dirent):

#include <sys/param.h>
#include <sys/mount.h>
#include <sys/stat.h>

#include <dirent.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int
main(int argc, char *argv[])
{
	int ret;
	DIR *dirp;
	struct statfs statfsbuf;
	struct dirent *dp;
	struct stat statbuf;

	ret = statfs(".", &statfsbuf);
	if (ret != 0) {
		perror("Failed to statfs current working directory");
		return 1;
	}

	printf(
	    "statfs:\n"
	    "  Mountpoint: %s\n\n",
	    statfsbuf.f_mntonname);

	dirp = opendir(".");
	if (dirp == NULL) {
		perror("Failed to open current working directory");
		return 1;
	}

	while ((dp = readdir(dirp)) != NULL) {
		char *filename;

		filename = malloc(dp->d_namlen + 1);
		memcpy(filename, dp->d_name, dp->d_namlen);
		filename[dp->d_namlen] = '\0';

		printf("%s:\n", filename);

		ret = stat(filename, &statbuf);
		if (ret != 0) {
			free(filename);
			perror("Failed to stat");
			continue;
		}

		printf(
		    "  st_mode: %06o\n"
		    "  st_size: %ld bytes\n",
		    statbuf.st_mode,
		    statbuf.st_size);
		free(filename);
	}

	closedir(dirp);

	return 0;
}

Here is the modified program to explicitely link to the old symbols:

#define	_WANT_FREEBSD11_STATFS
#define	_WANT_FREEBSD11_STAT
#include <sys/param.h>
#include <sys/mount.h>
#include <sys/stat.h>

#define	_WANT_FREEBSD11_DIRENT
#include <dirent.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

__asm__(".symver statfs,statfs@FBSD_1.0");
__asm__(".symver readdir,readdir@FBSD_1.0");
__asm__(".symver stat,stat@FBSD_1.0");

int
main(int argc, char *argv[])
{
	int ret;
	DIR *dirp;
	struct freebsd11_statfs statfsbuf;
	struct freebsd11_dirent *dp;
	struct freebsd11_stat statbuf;

	ret = statfs(".", (struct statfs *)&statfsbuf);
	if (ret != 0) {
		perror("Failed to statfs current working directory");
		return 1;
	}

	printf(
	    "statfs:\n"
	    "  Mountpoint: %s\n\n",
	    statfsbuf.f_mntonname);

	dirp = opendir(".");
	if (dirp == NULL) {
		perror("Failed to open current working directory");
		return 1;
	}

	while ((dp = (struct freebsd11_dirent *)readdir(dirp)) != NULL) {
		char *filename;

		filename = malloc(dp->d_namlen + 1);
		memcpy(filename, dp->d_name, dp->d_namlen);
		filename[dp->d_namlen] = '\0';

		printf("%s:\n", filename);

		ret = stat(filename, (struct stat *)&statbuf);
		if (ret != 0) {
			free(filename);
			perror("Failed to stat");
			continue;
		}

		printf(
		    "  st_mode: %06o\n"
		    "  st_size: %ld bytes\n",
		    statbuf.st_mode,
		    statbuf.st_size);
		free(filename);
	}

	closedir(dirp);

	return 0;
}

I compiled the two files on FreeBSD 12. The second one is effectively linked to the old symbols but both programs return the same result. Also, the one linked to the old symbols still works on FreeBSD 11 (even though it was compiled on FreeBSD 12).

So basically:

• It defines the _WANT_FREEBSD11_* macros which exposes the old structures. They are defined in the system headers (along with the regular structures) and have their name prefixed by freebsd11_.
• It uses the freebsd11_-prefixed versions of struct statfs, struct stat and struct dirent. The variables are casted when passed to standard functions.
• It sets up the linking to the old symbol versions, thanks to the .symver directive.

Presumably, we would only need that last item in Rust because the structures are redefined in Rust.

As @asomers said, we could explicitely link to old symbols on FreeBSD 12+ and put that behind an opt-out feature flag. I'm willing to prepare a patch; @alexcrichton, do you have any pointers to where to start?

I don't know if it's a good long-term solution, but at least it would deal with the immediate incompatibility.

[dumbbell]

By "I don't know if it's a good long-term solution", I agree with @semarie that the old symbols are made available to help the transition. But if there an inode is greater than 32-bit, it'll break. And there is also struct kevent where time-related members were changed to 64-bit and reordered, and the structure grew a new field.

I would love to find something which fully works at runtime, but I don't know Rust well enough. In particular, I don't know if we could have e.g. a single struct stat or struct kevent in Rust (which match the FreeBSD 12 layouts) and "redirect" calls to the appropriate wrapper of stat(2) & friends (the redirect would be computed once during program load if possible). The wrapper would either use the Rust struct as-is or convert it to the previous layout before passing it to the C function. Also, I don't know if the cost of such a conversion would be acceptable.

[alexcrichton]

@dumbbell I think you could use #[link_name] on functions to cause the linkage name to look different, and that I think should do the trick for now?

[alexcrichton]

I'm going to close this due to inactivity, and I think it may be best to focus on the in-progress RFC for now perhaps?

[bdrewery]

We really need these issues fixed for FreeBSD. FreeBSD CURRENT is quite an active development platform, just as people tend to be advised to use rust-nightly.

[bdrewery]

kevent compat fixed in #936

[bdrewery]

Fixing the ino64 changes isn't as trivial due to the truncation problem. I'm tempted to just do it anyhow as right now without using the port or an annoying patchset for rust it is completely unusable on FreeBSD12.