rust-lang / libc

Raw bindings to platform APIs for Rust
https://docs.rs/libc
Apache License 2.0
2.12k stars 1.04k forks source link

Tracking issue for libc 1.0 #3248

Open joshtriplett opened 1 year ago

joshtriplett commented 1 year ago

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).

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! :tada:
bossmc commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 11 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 9 months ago

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 commented 9 months ago

You should make 2 separate PRs.

safinaskar commented 9 months ago

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: https://github.com/rust-lang/libc/issues/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 commented 9 months ago

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

safinaskar commented 9 months ago

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

Amanieu commented 9 months ago

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

snogge commented 8 months ago

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

safinaskar commented 7 months ago

No pull requests was accepted in one whole month. So, our 1.0 work is stalled

Xuanwo commented 7 months ago

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 commented 3 months ago

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 commented 1 week ago

One other notable thing I think we should definitely do is make all padding or reserved fields MaybeUninit, as discussed here https://github.com/rust-lang/libc/issues/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)].