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

Tracking issue for libc 1.0 #3248

Open
2 of 19 tasks
joshtriplett opened this issue May 15, 2023 · 17 comments
Open
2 of 19 tasks

Tracking issue for libc 1.0 #3248

joshtriplett opened this issue May 15, 2023 · 17 comments

Comments

@joshtriplett
Copy link
Member

joshtriplett commented May 15, 2023

This issue tracks changes we want to make in libc 1.0.

(Note that this tracking issue previously referred to libc 0.3, this should now be assumed to refer to 1.0 instead)

64-bit time_t and off_t

On some 32-bit targets, it is unclear whether time_t should be 32-bit or 64-bit since in C this depends on what pre-processor macros are defined when the C standard library headers are included (e.g. _FILE_OFFSET_BITS, _TIME_BITS on glibc) or what version of the libc headers are used (e.g. musl 1.2 changed time_t to always be 64-bit). This was achieved without breaking ABI compatibility by giving the new 64-bit API separate symbol names (clock_gettime64) and having the header files transparently redirect the old function names to the new symbols. The old symbols remain available for programs built against old versions of the libc headers.

This makes it difficult for the libc crate since we can only have one definition of off_t and time_t, and yet we need our definitions of those types to match the ones used in any C code we link against.

Breaking changes

Most of these are mistakes that were made early on which cannot be fixed without making a breaking change. libc 1.0 is a good opportunity to make these changes.

Policy

These don't involve any code changes, but instead require a decision for how to handle breaking changes moving forward.

Anonymous structs and unions

Some C types are defined using anonymous structs and unions, such as sigaction or siginfo. To properly expose these, we need support for anonymous structs and unions in Rust. This feature is part of RFC 2102, but the implementation is still a work in progress.

Trait implementations

RFC 2235 added implementations of Eq, Hash and Debug for all libc types. However it turns out that this is not appropriate for many libc types, especially in the presence of padding, unions and extensible structs.

We would like to un-accept this RFC for libc 1.0 and completely remove Eq and Hash trait implementations for all types. It is still an open question whether we want to keep Debug trait implementations, but we are tending towards removing them as well.

Testing

We use ctest2 to check that the struct and function definitions in libc match those in C header files. However ctest2 is based on an old fork of the Rust parser, which doesn't handle new syntax well. It might be worth rewriting it or replacing it with a more modern tool (possibly based on bindgen/cbindgen).

  • Rewrite ctest2 (which is used on libc-test) to support newer edition and syntax

Implementation plan

  1. Fork a libc-0.2 branch from master on which development for libc 0.2 will continue (done).
  2. All PRs should target main.
  3. Anyone that would like a nonbreaking feature in a sooner release should comment @rustbot label +stable-nominated. After merge to main, this PR will be considered for cherry pick to libc-0.2.
  4. Once all desired features for libc 1.0 are ready, we can made a release! 🎉
@bossmc
Copy link
Contributor

bossmc commented May 17, 2023

On the LFS64 symbol removal point (#2935) the current PR is attempting to achieve rust crate API compatibility (by aliasing LFS64 things to their non-LFS64 equivalents), hiding the effective ABI break in musl libc (it's not a real break since those symbols never "really" existed, but they existed enough for the libc crate to wind up depending on them 😞). If 0.3 is going to be a breaking change we could do a simpler change where we simply stop all the LFS64 functions and types from the libc crate (which is arguably closer to the musl libc API).

My expectation was that we'd merge the API-compatible change in a 0.2.x (and maybe then deprecate the aliases?) and remove them "later" in a 0.3 release, but if 0.3 is imminent, maybe we just drop the items completely?

Note that the rust libc crate has to do "something" here to work with musl 1.2.4+ (@wesleywiser moved rustc's self-contained version to 1.2.3 since that's ABI-compatible with 1.1.24 but 1.2.4 breaks the crate's use of the ABI).

@joshtriplett
Copy link
Member Author

joshtriplett commented May 20, 2023

@bossmc Figuring out how to deal with time64 has been one of the two biggest blocking issues for this. (The other is having unnamed fields of struct and union types, for which there's an accepted RFC that still needs compiler support.)

We could just try to match the API of musl as closely as possible when building for musl, but that just pushes the problem onto whoever uses libc.

Personally, I would like to minimize the degree to which people have to write a pile of cfgs in order to use the 64-bit functions. libc::foo64 seems preferable to #[cfg(...)] use libc::foo as foo64; #[cfg(not(...))] use libc::foo64; (with corresponding complexity for other functions and types).

Is there some reasonable way we could generally make the same function names and types available on every platform that has 64-bit support, so that people only need cfg if they want to support platforms that don't have 64-bit support?

@lvella
Copy link

lvella commented Jun 21, 2023

So, the changes for 64 bits version of time and file offset function is just for musl? Can't 32 bit glibc be included in the package?

@JohnTitor
Copy link
Member

Fork a libc-0.2 branch from master on which development for libc 0.2 will continue.

Created the libc-0.2 branch. Once the musl 1.2 PR (or any other breaking changes) gets merged, I'll update the bors config and docs.

@ydirson
Copy link

ydirson commented Jan 15, 2024

Fork a libc-0.2 branch from master on which development for libc 0.2 will continue.

Created the libc-0.2 branch. Once the musl 1.2 PR (or any other breaking changes) gets merged, I'll update the bors config and docs.

Great to see 0.3 starting to take shape!
Is there an ETA for the breakage-candidates to land in main branch, and for a 0.3.0? Not asking for a firm date, my question is really "should I invest time to get a feature-guarded version of #3367 / #3201 into 0.2, or is 0.3 just right off the corner?"

@Amanieu
Copy link
Member

Amanieu commented Jan 15, 2024

I expect it will take at least a few months until 0.3 is ready. As you can see from the list at the top of this issue, there is still quite a lot of work to do.

@Xeonacid
Copy link
Contributor

Xeonacid commented Feb 18, 2024

Will PR submitted to libc-0.2 get merged into main for 0.3? Should I make two PRs basing both libc-0.2 and main if I want it available for both 0.2 and 0.3?

@Amanieu
Copy link
Member

Amanieu commented Feb 18, 2024

You should make 2 separate PRs.

@safinaskar
Copy link

According to both RFC 1242 and RFC 3119 and also https://blog.rust-lang.org/2017/02/06/roadmap.html , mature crates, which are part of "official Rust", should not have 0.x version numbers. So, please, release 1.0, not 0.3. Previous discussion: #547 . We should take this chance and release 1.0, not 0.3. libc is mature enough.

Later we can release 2.0, etc, if needed.

If there exist previous discussion, where you decided to release 0.3, not 1.0, then I'm sorry. The discussion is not linked from first comment

@safinaskar
Copy link

Obviously, next libc release should use semver trick ( https://github.com/dtolnay/semver-trick ), because libc types widely used in public interfaces

@safinaskar
Copy link

Also, we possibly should wait for extern types ( rust-lang/rust#43467 ). Many libc types are good candidates, for example, FILE ( https://docs.rs/libc/0.2.153/libc/enum.FILE.html )

@Amanieu Amanieu changed the title Tracking issue for libc 0.3 Tracking issue for libc 1.0 Feb 27, 2024
@Amanieu
Copy link
Member

Amanieu commented Feb 27, 2024

I've updated the tracking issue to talk about libc 1.0 instead of 0.3.

@snogge
Copy link
Contributor

snogge commented Mar 18, 2024

I have submitted #3223 and #3175 for 64bit time_t on GNU libc.

@safinaskar

This comment was marked as off-topic.

@Xuanwo
Copy link

Xuanwo commented Apr 9, 2024

It seems the reviewer for this repository is overwhelmed: there are 53 pull requests awaiting review, and almost all of them are for @JohnTitor. So do you need help? I'm willing to help push forward.

@tgross35
Copy link
Contributor

tgross35 commented Aug 13, 2024

Our branches unfortunately seem to have been diverging - some PRs target main, some target libc-0.2, and some both. This isn't very sustainable - it means the same reviews are happening in multiple places, and we are building up to either a nontrivial merge or some surprise regressions whenever 1.0 releases (e.g. there are some soundness fixes in 0.2 but not main that don't apply).

I assume the original intent was to merge libc-0.2 into main occasionally. But that isn't a very clean merge at this point and the workflow doesn't really line up with the existing duplicate PRs. I think we are better off keeping main correct and backporting on a case-by-case basis.

So, please always target main with new PRs. If the PR is nonbreaking, comment @rustbot label +stable-nominated to indicate we may want it in a release before 1.0. After merge, it will then be cherry picked to the 0.2 branch (I have been doing these in batches and intend to continue to do so), assuming it applies cleanly, in which case the label will be changed to stable-applied.

I have updated documentation to reflect this.

(If the difference between the branches get more minimal, we can probably switch back to merge-up flow. It is just different enough now that taking individual commits is easier to do correctly than a full merge, and I don't really see it getting easier with the 1.0 refactoring.)

@tgross35
Copy link
Contributor

One other notable thing I think we should definitely do is make all padding or reserved fields MaybeUninit, as discussed here #1453. This ensures the data can't be read by accident either on the user side or on our side via e.g. #[derive(PartialEq)].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests