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

fs::metadata() crashes on FreeBSD 12 due to layout change in stat.h #42681

Closed
lukaslueg opened this issue Jun 15, 2017 · 38 comments · Fixed by rust-lang/libc#937
Closed

fs::metadata() crashes on FreeBSD 12 due to layout change in stat.h #42681

lukaslueg opened this issue Jun 15, 2017 · 38 comments · Fixed by rust-lang/libc#937
Labels
C-bug Category: This is a bug. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. O-freebsd Operating system: FreeBSD P-high High priority T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@lukaslueg
Copy link
Contributor

The layout of struct stat used to be this in FreeBSD 11, in FreeBSD 12 it's now this. This causes the FreeBSD-specific implementation of std::fs::metadata() to crash for 1.18 and nightly as of 17/06/15, as it's stack frame get's boiled up.

Can be reproduced via

use std::fs;
fn main() {
    println!("{:?}", fs::metadata("Cargo.toml"));
}

Downstream this appeared in alacritty/alacritty#618

@lukaslueg lukaslueg changed the title fs::metadata() crashes on FreeBSD due to layout change in stat.h fs::metadata() crashes on FreeBSD 12 due to layout change in stat.h Jun 15, 2017
@nagisa nagisa added I-nominated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 15, 2017
@semarie
Copy link
Contributor

semarie commented Jun 16, 2017

I would like to point a pre-RFC discussion about introducing a way in Rust to permit libc to represent differents layouts per OS version. I think there is need to move forward.

@alexcrichton
Copy link
Member

We discussed this at libs triage today and the conclusion was that this is indeed bad! We would be fine in the very near future adding dynamic detection to do the right thing at runtime. In the long term we decided to see how @semarie's rfc plays out.

@alexcrichton alexcrichton added O-openbsd Operating system: OpenBSD O-freebsd Operating system: FreeBSD and removed I-nominated O-openbsd Operating system: OpenBSD labels Jun 25, 2017
@johalun
Copy link

johalun commented Jul 8, 2017

Is there a quick fix for this? This stops me from building a few (probably many) crates..

@johalun
Copy link

johalun commented Jul 13, 2017

Here's a temporary fix for those who can't wait
https://github.com/johalun/rust-ino64
Just re-apply each time you update nightly with rustup.

@Mark-Simulacrum Mark-Simulacrum added C-bug Category: This is a bug. P-high High priority and removed I-wrong labels Jul 27, 2017
@kirillrdy
Copy link

@johalun Thanks for a workaround ! 🍺

@Mark-Simulacrum Mark-Simulacrum added the I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. label Aug 10, 2017
@dumbbell
Copy link
Contributor

Hi!

I prepared a patch to support the two breaking changes in FreeBSD 12.x (see rust-lang/libc#721). I tested them successfully on FreeBSD 10.3 and FreeBSD 12.0, using the testsuites of mio and hyper.

However, I'm not sure my solution around struct kevent is correct and I would love to hear from others. The pull request gives all the details about the patch and what I had in mind.

dumbbell added a commit to dumbbell/libc that referenced this issue Aug 10, 2017
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.
  https://svnweb.freebsd.org/base?view=revision&revision=318736

* Time-related members of `struct kevent` were changed to 64-bit. Again,
  this affects the definition of the same structure in Rust libc.
  https://svnweb.freebsd.org/base?view=revision&revision=320043

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.
@mattmacy
Copy link

@dumbbell there's an ongoing discussion about the fact that Rust has inadvertently baked in UBs on !Linux platforms by creating its own private copies of system headers, that are essentially an arbitrary version. rust-lang/libc#570 (comment)

Cargo needs to grow support for doing the equivalent of parsing uname -K and passing that as target_os_version.

For the time being I've accepted that when compiling I need to just append the following 3 lines to the Cargo.toml of any root crate when building on HEAD:

[patch.crates-io]
mio = { git = "https://github.com/FreeBSDRust/mio" }
libc = { git = "https://github.com/FreeBSDRust/rust-libc" }

@mattmacy
Copy link

Actually - although I managed to get jsonrpc_http_server bits to work yesterday by patching some of the dependent ports and then using [patch.crates-io] overrides, today I tried building netmap_sys with user libs which failed in the gcc crate, presumably because of something in libstd. I tried installing the FreeBSD port, but the cargo in that version wouldn't let me override libc (if that's something only available in nightlies you really need to make it standard) so although it compiled the binary didn't actually work. I've lost too much time on this to consider Rust a viable option for any near-term deadlines and have concluded that Rust really isn't cross-platform outside of tier 1 targets right now.

@valpackett
Copy link
Contributor

valpackett commented Oct 2, 2017

Here's a fun thing. @johalun's amazing method of patching old_fstat.c into an already installed (rustup) std lib messes with…

Mesa OpenGL driver loading. Running glutin or webrender examples compiled with an unpatched compiler, my GPU driver loads fine. With a patched compiler:

libGL error: MESA-LOADER: failed to retrieve device information
[…] libGL error: failed to load driver: amdgpu
[…] libGL: OpenDriver: trying /usr/local/lib/dri/swrast_dri.so

And no, MESA_LOADER_DRIVER_OVERRIDE=radeonsi doesn't help — if it loads the right driver via manual override, that driver can't communicate to the GPU device either:

amdgpu: drmGetDevice2 failed.
do_winsys_init: DRM version is 3.8.0 but this driver is only compatible with 2.12.0 (kernel 3.2) or later.
libGL error: failed to create dri screen
libGL error: failed to load driver: radeonsi

The ports/pkg rustc, compiled normally with the same old_fstat changes patched in at the source level (UPD: used on the bootstrap compiler only; actual rust patch is used instead on the resulting build), does not have this problem. I guess I'll have to resurrect the lang/rust-nightly port…

@valpackett
Copy link
Contributor

valpackett commented Oct 3, 2017

Here's a working nightly Rust toolchain for FreeBSD 12-CURRENT: rustc std cargo rls analysis docs. Don't ask me how I built it :D

(plus fully updated libc)

bdrewery added a commit to bdrewery/libc that referenced this issue Mar 1, 2018
This follows the same method as other platforms like OSX and NetBSD.

This fixes rustup and building from git (once libc is updated for bootstrap)
on FreeBSD12 post-ino64 in freebsd/freebsd-src@f713b08.
It also avoids having to hotpatch the stage0 compiler on FreeBSD12 to
build rust.

The only real pitfall is that this will prevent interaction with inodes that
have an ino_t above the 32-bit limit due to truncation.  On the other hand
Rust won't work at all on 12 without doing this currently.  In general
it should not be a problem for users and if they need 64-bit ino_t they
can use a patched libc, rather than the current state of affairs in
requiring a patched libc to use Rust on 12.

A better, or complementary, approach would be something like proposed in
rust-lang/rfcs#2048 to allow targetting a specific
version of FreeBSD. This would allow Rust to default to this compatibility
mode by targetting FreeBSD10 and still allow targetting FreeBSD12 for 64-bit
ino_t.

Fixes rust-lang/rust#42681
bdrewery added a commit to bdrewery/libc that referenced this issue Mar 1, 2018
This follows the same method as other platforms like OSX and NetBSD.

This fixes rustup and building from git (once libc is updated for bootstrap)
on FreeBSD12 post-ino64 in freebsd/freebsd-src@f713b08.
It also avoids having to hotpatch the stage0 compiler on FreeBSD12 to
build rust.

The only real pitfall is that this will prevent interaction with inodes that
have an ino_t above the 32-bit limit due to truncation.  On the other hand
Rust won't work at all on 12 without doing this currently.  In general
it should not be a problem for users and if they need 64-bit ino_t they
can use a patched libc, rather than the current state of affairs in
requiring a patched libc to use Rust on 12.

A better, or complementary, approach would be something like proposed in
rust-lang/rfcs#2048 to allow targetting a specific
version of FreeBSD. This would allow Rust to default to this compatibility
mode by targetting FreeBSD10 and still allow targetting FreeBSD12 for 64-bit
ino_t.

The symbol versions used were taken from the old version in
freebsd/freebsd-src@f713b08#diff-61a32fcfb7ecd4517665fed591813c57
and
freebsd/freebsd-src@f713b08#diff-7f67ccf8b5f44ff2f54eaab0207abb8d.

Fixes rust-lang/rust#42681
bdrewery added a commit to bdrewery/libc that referenced this issue Mar 1, 2018
This follows the same method as other platforms like OSX and NetBSD.

This fixes rustup and building from git (once libc is updated for bootstrap)
on FreeBSD12 post-ino64 in freebsd/freebsd-src@f713b08.
It also avoids having to hotpatch the stage0 compiler, and HOME/.cargo
libc files on FreeBSD12 to build rust.

The only real pitfall is that this will prevent interaction with inodes that
have an ino_t above the 32-bit limit due to truncation.  On the other hand
Rust won't work at all on 12 without doing this currently.  In general
it should not be a problem for users and if they need 64-bit ino_t they
can use a patched libc, rather than the current state of affairs in
requiring a patched libc to use Rust on 12.

A better, or complementary, approach would be something like proposed in
rust-lang/rfcs#2048 to allow targetting a specific
version of FreeBSD. This would allow Rust to default to this compatibility
mode by targetting FreeBSD10 and still allow targetting FreeBSD12 for 64-bit
ino_t.

The symbol versions used were taken from the old version in
freebsd/freebsd-src@f713b08#diff-61a32fcfb7ecd4517665fed591813c57
and
freebsd/freebsd-src@f713b08#diff-7f67ccf8b5f44ff2f54eaab0207abb8d.

Fixes rust-lang/rust#42681
bdrewery added a commit to bdrewery/libc that referenced this issue Mar 1, 2018
This follows the same method as other platforms like OSX and NetBSD.

This fixes rustup and building from git (once libc is updated for bootstrap)
on FreeBSD12 post-ino64 in freebsd/freebsd-src@f713b08.
It also avoids having to hotpatch the stage0 compiler, and HOME/.cargo
libc files on FreeBSD12 to build rust.

The only real pitfall is that this will prevent interaction with inodes that
have an ino_t above the 32-bit limit due to truncation.  On the other hand
Rust won't work at all on 12 without doing this currently.  In general
it should not be a problem for users and if they need 64-bit ino_t they
can use a patched libc, rather than the current state of affairs in
requiring a patched libc to use Rust on 12.

A better, or complementary, approach would be something like proposed in
rust-lang/rfcs#2048 to allow targetting a specific
version of FreeBSD. This would allow Rust to default to this compatibility
mode by targetting FreeBSD10 and still allow targetting FreeBSD12 for 64-bit
ino_t.

The symbol versions used were taken from the old version in
freebsd/freebsd-src@f713b08#diff-61a32fcfb7ecd4517665fed591813c57
and
freebsd/freebsd-src@f713b08#diff-7f67ccf8b5f44ff2f54eaab0207abb8d.

The scope of functions versioned here differs from other platforms as
not all structs were modified that were on others, such as DIR for
`opendir`, `telldir`, etc..

Fixes rust-lang/rust#42681
bdrewery added a commit to bdrewery/libc that referenced this issue Mar 1, 2018
This follows the same method as other platforms like OSX and NetBSD.

This fixes rustup and building from git (once libc is updated for bootstrap)
on FreeBSD12 post-ino64 in freebsd/freebsd-src@f713b08.
It also avoids having to hotpatch the stage0 compiler, and HOME/.cargo
libc files on FreeBSD12 to build rust.

The only real pitfall is that this will prevent interaction with inodes that
have an ino_t above the 32-bit limit due to truncation.  On the other hand
Rust won't work at all on 12 without doing this currently.  In general
it should not be a problem for users and if they need 64-bit ino_t they
can use a patched libc, rather than the current state of affairs in
requiring a patched libc to use Rust on 12.

A better, or complementary, approach would be something like proposed in
rust-lang/rfcs#2048 to allow targetting a specific
version of FreeBSD. This would allow Rust to default to this compatibility
mode by targetting FreeBSD10 and still allow targetting FreeBSD12 for 64-bit
ino_t.

The symbol versions used were taken from the old version in
freebsd/freebsd-src@f713b08#diff-61a32fcfb7ecd4517665fed591813c57
and
freebsd/freebsd-src@f713b08#diff-7f67ccf8b5f44ff2f54eaab0207abb8d.

The scope of functions versioned here differs from other platforms as
not all structs were modified that were on others, such as DIR for
`opendir`, `telldir`, etc.  Only functions using dirent, stat, glob_t,
and dev_t need the changes.

Fixes rust-lang/rust#42681
bdrewery added a commit to bdrewery/libc that referenced this issue Mar 1, 2018
This follows the same method as other platforms like OSX and NetBSD.

This will fix rustup and building from git (once libc is updated for bootstrap)
on FreeBSD12 post-ino64 in freebsd/freebsd-src@f713b08.
It also avoids having to hotpatch the stage0 compiler, and HOME/.cargo
libc files on FreeBSD12 to build rust.

The only real pitfall is that this will prevent interaction with inodes that
have an ino_t above the 32-bit limit due to truncation.  On the other hand
Rust won't work at all on 12 without doing this currently.  In general
it should not be a problem for users and if they need 64-bit ino_t they
can use a patched libc, rather than the current state of affairs in
requiring a patched libc to use Rust on 12.

A better, or complementary, approach would be something like proposed in
rust-lang/rfcs#2048 to allow targetting a specific
version of FreeBSD. This would allow Rust to default to this compatibility
mode by targetting FreeBSD10 and still allow targetting FreeBSD12 for 64-bit
ino_t.

The symbol versions used were taken from the old version in
freebsd/freebsd-src@f713b08#diff-61a32fcfb7ecd4517665fed591813c57
and
freebsd/freebsd-src@f713b08#diff-7f67ccf8b5f44ff2f54eaab0207abb8d.

The scope of functions versioned here differs from other platforms as
not all structs were modified that were on others, such as DIR for
`opendir`, `telldir`, etc.  Only functions using dirent, stat, glob_t,
and dev_t need the changes.

Fixes rust-lang/rust#42681
bors added a commit to rust-lang/libc that referenced this issue Mar 2, 2018
Use pre-ino64 FreeBSD symbols to resolve binary compatibility.

This follows the same method as other platforms like OSX and NetBSD.

This will fix rustup and building from git (once libc is updated for bootstrap)
on FreeBSD12 post-ino64 in freebsd/freebsd-src@f713b08.
It also avoids having to hotpatch the stage0 compiler, and HOME/.cargo
libc files on FreeBSD12 to build rust.

The only real pitfall is that this will prevent interaction with inodes that
have an ino_t above the 32-bit limit due to truncation.  On the other hand
Rust won't work at all on 12 without doing this currently.  In general
it should not be a problem for users and if they need 64-bit ino_t they
can use a patched libc, rather than the current state of affairs in
requiring a patched libc to use Rust on 12.

A better, or complementary, approach would be something like proposed in
rust-lang/rfcs#2048 to allow targetting a specific
version of FreeBSD. This would allow Rust to default to this compatibility
mode by targetting FreeBSD10 and still allow targetting FreeBSD12 for 64-bit
ino_t.

The symbol versions used were taken from the old version in
freebsd/freebsd-src@f713b08#diff-61a32fcfb7ecd4517665fed591813c57
and
freebsd/freebsd-src@f713b08#diff-7f67ccf8b5f44ff2f54eaab0207abb8d.

The scope of functions versioned here differs from other platforms as
not all structs were modified that were on others, such as DIR for
`opendir`, `telldir`, etc.  Only functions using dirent, stat, glob_t,
and dev_t need the changes.

Fixes rust-lang/rust#42681
@bdrewery
Copy link
Contributor

bdrewery commented Mar 2, 2018

Technically this is not fixed yet as I need to update the submodule and the bootstrap needs to get the fix. Will be a few days.

bdrewery added a commit to bdrewery/rust that referenced this issue Mar 8, 2018
bdrewery added a commit to bdrewery/rust that referenced this issue Mar 12, 2018
bors added a commit that referenced this issue Mar 14, 2018
@bdrewery
Copy link
Contributor

The last piece for this should be updating the src/stage0.txt beta used. I am testing that now.

kennytm added a commit to kennytm/rust that referenced this issue Mar 20, 2018
…xcrichton

Update beta to version with fixed FreeBSD support from rust-lang#49023.

Fixes rust-lang#42681

r? @alexcrichton
swills pushed a commit to swills/freebsd-ports that referenced this issue Mar 21, 2018
The ABI patch and bootstrap patching are no longer needed on head after
fixes fully upstreamed in rust-lang/rust#42681.


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@465189 35697150-7ecd-e111-bb59-0022644237b5
uqs pushed a commit to freebsd/freebsd-ports that referenced this issue Mar 21, 2018
The ABI patch and bootstrap patching are no longer needed on head after
fixes fully upstreamed in rust-lang/rust#42681.


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@465189 35697150-7ecd-e111-bb59-0022644237b5
uqs pushed a commit to freebsd/freebsd-ports that referenced this issue Mar 21, 2018
The ABI patch and bootstrap patching are no longer needed on head after
fixes fully upstreamed in rust-lang/rust#42681.
Jehops pushed a commit to Jehops/freebsd-ports-legacy that referenced this issue Mar 21, 2018
The ABI patch and bootstrap patching are no longer needed on head after
fixes fully upstreamed in rust-lang/rust#42681.


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@465189 35697150-7ecd-e111-bb59-0022644237b5
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-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. O-freebsd Operating system: FreeBSD P-high High priority T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.