Skip to content

Use pre-ino64 FreeBSD symbols to resolve binary compatibility. #937

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

Merged
merged 1 commit into from
Mar 2, 2018

Conversation

bdrewery
Copy link
Contributor

@bdrewery bdrewery commented Mar 2, 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

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
@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@bdrewery
Copy link
Contributor Author

bdrewery commented Mar 2, 2018

FYI @dumbbell @asomers

Copy link
Contributor

@asomers asomers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This covers every function in FreeBSD's libc that was changed in 12, and that is already bound by the libc crate.

@alexcrichton
Copy link
Member

@bors: r+

Awesome, thanks @bdrewery!

FWIW when we can I think it'd be fine to add things like stat64 and such like we have on Linux. Although those symbols probably don't actually exist in FreeBSD we could probably get away with pretending they do in this crate (using #[link_name])

@bors
Copy link
Contributor

bors commented Mar 2, 2018

📌 Commit 78f9322 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Mar 2, 2018

⌛ Testing commit 78f9322 with merge 68d4848...

bors added a commit that referenced this pull request 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
@bors
Copy link
Contributor

bors commented Mar 2, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 68d4848 to master...

@bors bors merged commit 78f9322 into rust-lang:master Mar 2, 2018
@bdrewery bdrewery deleted the freebsd-ino64-compat branch March 2, 2018 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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