SIGSEGV with beta/nightly compiler with i686-unknown-freebsd target

Darksonn commented 1 month ago

When running the Tokio test suite through CI using the beta/nightly compiler, the FreeBSD 32-bit build encounters a segmentation fault. After running CI with a few different configurations, I've found two different cases of the failure. The first segfault is in the compile_fail_full test. The path_read_write test also encounters a segfault.

running 1 test
error: test failed, to rerun pass `-p tests-build --test macros`

Caused by:
  process didn't exit successfully: `/tmp/cirrus-ci-build/target/i686-unknown-freebsd/debug/deps/macros-4f8b28e5ec17a589` (signal: 11, SIGSEGV: invalid memory reference)

Exit status: 101

Please see https://github.com/tokio-rs/tokio/pull/6856 for more output from the CI runs. Both failures happen in tests that interact with the file system. There are other tests that pass and don't touch the file system (e.g. all unit tests in tokio/src/ pass). As Tokio has no special logic for using the file system and just calls std, and as the compile_fail_full test is not calling into any Tokio code at all, this looks an awful lot like a bug somewhere in std's filesystem handling code. But that is just a guess. I don't have access to a FreeBSD machine, so it is difficult for me to investigate further.

The 64-bit FreeBSD build does not encounter these failures. There also are no problems when using the latest stable compiler. Note that the 32-bit binaries in question are cross-compiled from and executed on a 64-bit FreeBSD host machine.

Failing toolchains:

rustc 1.82.0-beta.3 (4976ae480 2024-09-09)
binary: rustc
commit-hash: 4976ae480e2b29cc46f44e1b9914469cc384fcc9
commit-date: 2024-09-09
host: x86_64-unknown-freebsd
release: 1.82.0-beta.3
LLVM version: 19.1.0

rustc 1.83.0-nightly (da889684c 2024-09-20)
binary: rustc
commit-hash: da889684c80508036ff036db8c159ffdcf27648a
commit-date: 2024-09-20
host: x86_64-unknown-freebsd
release: 1.83.0-nightly
LLVM version: 19.1.0

Passing toolchains:

rustc 1.81.0 (eeb90cda1 2024-09-04)
binary: rustc
commit-hash: eeb90cda1969383f56a2637cbd3037bdf598841c
commit-date: 2024-09-04
host: x86_64-unknown-freebsd
release: 1.81.0
LLVM version: 18.1.7

Darksonn commented 1 month ago

I can try to bisect it to a specific nightly next week. If it's useful, you all are welcome to use our CI to debug this - you can open a PR to Tokio to run our CI setup.

lolbinarycat commented 1 month ago

the compile_fail_full test is not calling into any Tokio code at all

it calls various tokio macros, doesn't it?? also, it doesn't seem to call anything in std besides std::time::Instant::now, so i'm not sure how this could be a path handling bug?

workingjubilee commented 1 month ago

@Darksonn Is this our compiler or is it the one from FreeBSD ports?

They modded the target.

Darksonn commented 1 month ago

I have bisected the issue. It succeeds on nightly-2024-08-20 and fails on nightly-2024-08-21.

@lolbinarycat The compile_fail_full test calls into the trybuild crate, which uses std::fs to probe which build tests exists.

@workingjubilee I got the compiler from rustup, so I assume it's your compiler. Our ci script runs the following commands:

pkg install -y bash
curl https://sh.rustup.rs -sSf --output rustup.sh
sh rustup.sh -y --profile minimal --default-toolchain $TOOLCHAIN
. $HOME/.cargo/env
rustup target add i686-unknown-freebsd
rustc --version --verbose
cargo test --all --all-features --target i686-unknown-freebsd

with $TOOLCHAIN normally set to stable, but under these tests I set it to beta or nightly-xxxx-xx-xx for testing other compilers. This is happening on a x86_64-unknown-freebsd host.

workingjubilee commented 1 month ago

Yep, that's our sigsegv, comrade!

saethlin commented 1 month ago

Minimized:

fn main() {
    std::fs::File::open("Cargo.toml").unwrap().metadata();
}

(any file that exists will do)

saethlin commented 1 month ago

Bisection points to https://github.com/rust-lang/rust/pull/129226 Which notes https://github.com/rust-lang/libc/pull/3723

In particular: https://github.com/rust-lang/libc/pull/3723#discussion_r1620798631

It will always be safe to use a FreeBSD X ABI on a FreeBSD X+1 system. You may be thinking of OpenBSD, which does not provide that guarantee.

So... is it wrong to use a FreeBSD 12 ABI on a FreeBSD 14 system? Because that's what we're apparently trying to do here.

workingjubilee commented 1 month ago

Hello @asomers there seems to be a bit of confusion afoot!

asomers commented 1 month ago

I've reproduced the problem locally, and I'll take a look.

asomers commented 1 month ago

I'm pretty sure that this is a bug in libc. While I work on a patch, could one of you toolchain experts please suggest to me how I might compile an arbitrary crate using a patched version of libc for std?

workingjubilee commented 1 month ago

I'm pretty sure that this is a bug in libc. While I work on a patch, could one of you toolchain experts please suggest to me how I might compile an arbitrary crate using a patched version of libc for std?

It should be sufficient to use [patch.crates-io] in library/Cargo.toml for libc, with presumably overriding to a path or a git repo. That will let you build a patched std, the new path should be reflected in the lock file.

asomers commented 1 month ago

Sorry @workingjubilee but I need more help than that. I also need to know how to build, for example, Tokio using the patched std.

workingjubilee commented 1 month ago

uhh honestly I would just build the entire stage 1 compiler with the patched std and then build tokio with that. I can look up the incantation for using the stage 0 std with an existing nightly if that would take forever, though.

workingjubilee commented 1 month ago

Ah here we go, you want to do this:

./x.py build --stage 0 library
rustup toolchain link stage0 build/host/stage0-sysroot

then cargo +stage0 should work?

asomers commented 1 month ago

@workingjubilee that almost works. But I can't cross-compile from x86_64 to x86. Could you please tell what I must do to enable that? When I try, I get this:

somers@methionine ~/s/r/i386-fbsd-segfault (master)> cargo +stage0 run --target i686-unknown-freebsd
   Compiling i386-fbsd-segfault v0.1.0 (/usr/home/somers/src/rust/i386-fbsd-segfault)
error[E0463]: can't find crate for `std`
  |
  = note: the `i686-unknown-freebsd` target may not be installed
  = help: consider downloading the target with `rustup target add i686-unknown-freebsd`

For more information about this error, try `rustc --explain E0463`.
error: could not compile `i386-fbsd-segfault` (bin "i386-fbsd-segfault") due to 1 previous error
somers@methionine ~/s/r/i386-fbsd-segfault (master) [101]> rustup target add --toolchain stage0 i686-unknown-freebsd
error: error: invalid value 'stage0' for '--toolchain <toolchain>': invalid toolchain name: 'stage0'

For more information, try '--help'.
: invalid toolchain name: 'stage0'
somers@methionine ~/s/r/i386-fbsd-segfault (master) [1]> cargo +stage0 run --target i686-unknown-freebsd -Zbuild-std
error: the `-Z` flag is only accepted on the nightly channel of Cargo, but this is the `beta` channel
See https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information about Rust release channels.

lolbinarycat commented 1 month ago

@asomers you should be able to use -Z build-std to cross-compile the standard library.

asomers commented 1 month ago

@lolbinarycat I tried that, but the compiler thinks it's beta, not nightly, so it won't allow it. Is there a config.toml setting I can use to build it as a nightly compiler instead?

lolbinarycat commented 1 month ago

you can export RUSTC_BOOTSTRAP=1 to allow using nightly features on a beta compiler (yes it's a hack, but it's probably fine for a quick repro)

lolbinarycat commented 1 month ago

also, stage0 is a beta compiler, if you want an actual nightly, you need to build stage1

asomers commented 1 month ago

With help from @lolbinarycat and @workingjubilee I have a working patch in review at https://github.com/rust-lang/libc/pull/3939 . HOWEVER, it only fixes struct stat on the libc-0.2 branch. I'll need to prepare a separate PR for the main branch. Also, I haven't tested on 32-bit architectures other than i386. I'm still trying to get access to some. Finally, I found some additional errors in other structures that require fixing. But they're all pretty obscure structures that the standard library doesn't use.

workingjubilee commented 1 month ago

Oh, the way to do that without -Zbuild-std is to add this to the config.toml:

[build]
target = ["host-target-string", "cross-target-string"]

workingjubilee commented 1 month ago

@asomers Thank you for taking care of this!

apiraino commented 1 month ago

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

asomers commented 1 month ago

The relevant fixes have been merged into libc. So this bug will be fixed if libc makes a new release and rustc incorporates it.

asomers commented 3 weeks ago

libc has released. So if you update rust to use libc 0.2.160 or later this bug should be fixed.

Darksonn commented 3 weeks ago

As this bug has landed in a stable release, it is now causing CI failures in Tokio's CI setup. Please advise on what we should do. Will this get backported to 1.82, or should we freeze our FreeBSD CI to use 1.81.0?

workingjubilee commented 3 weeks ago

@rust-lang/libs, relevant facts:

i686-unknown-freebsd's libstd builds but its filesystem API (std::fs) is made useless/unsound by this bug.
The fix was landed in libc 0.2.160 and then merged as part of https://github.com/rust-lang/rust/pull/131823 thanks to @asomers and @thesummer[^0].
Unfortunately this means it has missed both the 1.83 beta and 1.82 stable releases.
i686-unknown-freebsd is a tier 2 "without host tools" target^1 so "libstd builds, but is ineffectual" is within the bounds of our platform support requirements, thus one can argue there is no requirement to act. However, it would be unusual for us to not beta-backport a soundness fix, tier 2 or no, so I would recommend a beta backport.
The main question is whether we should request a stable backport at the discretion of T-release.

[^0]: I thought it was autumn?

cuviper commented 3 weeks ago

The PR is what needs the backport nomination label -- I've added beta there, but you can add stable too if you want to argue for that. I expect the stable disposition will probably be something like "accepted but does not warrant a point release on its own."

Amanieu commented 2 weeks ago

In the libs meeting we discussed https://github.com/rust-lang/rust/pull/131823#issuecomment-2432788998 we accepted the beta backport but rejected the stable backport due to the large potential impact of a libc bump on other targets.

rust-lang / rust

SIGSEGV with beta/nightly compiler with i686-unknown-freebsd target #130677