Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Darksonn opened this issue Sep 21, 2024 · 29 comments · Fixed by #131823
Closed

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

Darksonn opened this issue Sep 21, 2024 · 29 comments · Fixed by #131823
Labels
C-bug Category: This is a bug. I-libs-nominated Nominated for discussion during a libs team meeting. O-freebsd Operating system: FreeBSD P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Milestone

Comments

@Darksonn
Copy link
Contributor

Darksonn commented Sep 21, 2024

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 tokio-rs/tokio#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 Darksonn added the C-bug Category: This is a bug. label Sep 21, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 21, 2024
@lolbinarycat lolbinarycat added the regression-untriaged Untriaged performance or correctness regression. label Sep 21, 2024
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Sep 21, 2024
@Darksonn
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Member

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

They modded the target.

@saethlin saethlin added the O-freebsd Operating system: FreeBSD label Sep 21, 2024
@Darksonn
Copy link
Contributor Author

Darksonn commented Sep 22, 2024

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.

@Mark-Simulacrum Mark-Simulacrum added this to the 1.82.0 milestone Sep 22, 2024
@workingjubilee
Copy link
Member

Yep, that's our sigsegv, comrade!

@workingjubilee workingjubilee added regression-from-stable-to-beta Performance or correctness regression from stable to beta. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed regression-untriaged Untriaged performance or correctness regression. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. labels Sep 22, 2024
@saethlin
Copy link
Member

Minimized:

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

(any file that exists will do)

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 22, 2024
@saethlin
Copy link
Member

saethlin commented Sep 22, 2024

Bisection points to #129226
Which notes rust-lang/libc#3723

In particular: rust-lang/libc#3723 (comment)

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
Copy link
Member

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

@asomers
Copy link
Contributor

asomers commented Sep 22, 2024

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

@asomers
Copy link
Contributor

asomers commented Sep 22, 2024

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
Copy link
Member

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
Copy link
Contributor

asomers commented Sep 22, 2024

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
Copy link
Member

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
Copy link
Member

workingjubilee commented Sep 22, 2024

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
Copy link
Contributor

asomers commented Sep 23, 2024

@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
Copy link
Contributor

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

@asomers
Copy link
Contributor

asomers commented Sep 23, 2024

@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
Copy link
Contributor

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)

asomers added a commit to asomers/libc that referenced this issue Sep 29, 2024
The original definitions were never correct.  But nobody noticed because
we don't do CI on 32-bit FreeBSD.  The problem is apparent now due to
 rust-lang#3723 , which caused the nightly toolchain to switch to a FreeBSD 12
ABI.

Fixes rust-lang/rust#130677
asomers added a commit to asomers/capsicum-rs that referenced this issue Sep 29, 2024
It's a standard library bug.  Work around it by using an older
toolchain.

rust-lang/rust#130677
asomers added a commit to asomers/libc that referenced this issue Oct 2, 2024
Commit 7437d0a erroneously defined it as "ulong" instead of u64.
Nobody noticed the mistake, probably because it was only tested on
64-bit architectures, where those are equivalent.  But it's a problem
now, after rust-lang#3723 , which switched the standard library to a FreeBSD 12
ABI.

Issue rust-lang/rust#130677
asomers added a commit to asomers/libc that referenced this issue Oct 2, 2024
The original definitions were never correct.  But nobody noticed because
we don't do CI on 32-bit FreeBSD.  The problem is apparent now due to
 rust-lang#3723 , which caused the nightly toolchain to switch to a FreeBSD 12
ABI.

Fixes rust-lang/rust#130677
@asomers
Copy link
Contributor

asomers commented Oct 3, 2024

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
Copy link
Contributor

asomers commented Oct 17, 2024

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

@Darksonn
Copy link
Contributor Author

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 workingjubilee added beta-nominated Nominated for backporting to the compiler in the beta channel. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. stable-nominated Nominated for backporting to the compiler in the stable channel. I-compiler-nominated Nominated for discussion during a compiler team meeting. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Oct 18, 2024
@workingjubilee workingjubilee added I-libs-nominated Nominated for discussion during a libs team meeting. and removed I-compiler-nominated Nominated for discussion during a compiler team meeting. labels Oct 18, 2024
@workingjubilee
Copy link
Member

@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 Bump libc to 0.2.161 #131823 thanks to @asomers and @thesummer1.
  • 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" target2 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.

Footnotes

  1. I thought it was autumn?

  2. https://doc.rust-lang.org/nightly/rustc/platform-support.html

@workingjubilee workingjubilee removed beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Oct 18, 2024
@cuviper
Copy link
Member

cuviper commented Oct 18, 2024

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
Copy link
Member

Amanieu commented Oct 23, 2024

In the libs meeting we discussed #131823 (comment) we accepted the beta backport but rejected the stable backport due to the large potential impact of a libc bump on other targets.

@Amanieu Amanieu closed this as completed Oct 23, 2024
asomers added a commit to asomers/libc that referenced this issue Oct 25, 2024
Add i686-unknown-freebsd to CI.  Run it using 32-bit emulation in a
64-bit environment, with the nightly compiler only.

So as to avoid a repeat of rust-lang/rust#130677
asomers added a commit to asomers/libc that referenced this issue Nov 8, 2024
Add i686-unknown-freebsd to CI.  Run it using 32-bit emulation in a
64-bit environment, with the nightly compiler only.

So as to avoid a repeat of rust-lang/rust#130677
tgross35 pushed a commit to tgross35/rust-libc that referenced this issue Nov 13, 2024
Add i686-unknown-freebsd to CI.  Run it using 32-bit emulation in a
64-bit environment, with the nightly compiler only.

So as to avoid a repeat of rust-lang/rust#130677

(backport <rust-lang#3997>)
(cherry picked from commit ce0a306)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. I-libs-nominated Nominated for discussion during a libs team meeting. O-freebsd Operating system: FreeBSD P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants