Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix "struct stat" on 32-bit FreeBSD 12+ #3939

Merged
merged 3 commits into from
Sep 25, 2024

Conversation

asomers
Copy link
Contributor

@asomers asomers commented Sep 23, 2024

It has always been broken. But we never noticed because we don't run CI on 32-bit FreeBSD, and until #3723 our users always used a FreeBSD 11 ABI.

Fixes rust-lang/rust#130677

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
@rustbot
Copy link
Collaborator

rustbot commented Sep 23, 2024

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

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

Thanks for the fix, can this target main or is it conflict-y? Also, do you have a link to the relevant headers?

@asomers
Copy link
Contributor Author

asomers commented Sep 25, 2024

Thanks for the fix, can this target main or is it conflict-y? Also, do you have a link to the relevant headers?

We'll need to merge it to both branches, but there's a slight conflict with main. As for headers:
https://github.com/freebsd/freebsd-src/blob/ee3da604dd016439850dae77366796313e60f0e0/sys/sys/stat.h#L122
https://github.com/freebsd/freebsd-src/blob/ee3da604dd016439850dae77366796313e60f0e0/sys/sys/stat.h#L157

@tgross35
Copy link
Contributor

Thanks for the links, easy enough.

For future reference I've been preferring PRs to target main first so that branch can't wind up missing anything, then I do periodic backports. But since this is a fix we might as well get it in.

@rustbot label +main-needs-cherry-pick

@rustbot rustbot added the main-needs-cherry-pick This PR was merged against libc-0.2 but does not yet exist in main. label Sep 25, 2024
@tgross35 tgross35 added this pull request to the merge queue Sep 25, 2024
Merged via the queue into rust-lang:libc-0.2 with commit b9e8477 Sep 25, 2024
53 checks passed
@tgross35 tgross35 added main-applied This PR was merged against libc-0.2 and main has since been updated to include it and removed main-needs-cherry-pick This PR was merged against libc-0.2 but does not yet exist in main. labels Oct 16, 2024
AkhilTThomas pushed a commit to AkhilTThomas/libc that referenced this pull request Feb 5, 2025
* Fix the definition of ino_t on 32-bit FreeBSD 12+

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

* Fix the definition of "struct stat" on 32-bit FreeBSD 12+

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

* fixup: fix build on powerpc, powerpc64, and arm
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
main-applied This PR was merged against libc-0.2 and main has since been updated to include it S-waiting-on-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants