Skip to content

Conversation

@xbjfk
Copy link
Contributor

@xbjfk xbjfk commented May 26, 2025

Description

This change includes time64 support for applicable architectures (x86, arm, mips and powerpc). This is based on the previous PRs to this repo as well as the musl changelog from 1.1.24 -> 1.2. It can be enabled with the environment variable
RUST_LIBC_UNSTABLE_MUSL_TIME64 only when musl_v1_2_3 is enabled and the architecture is supported.

A lot of structures, especially ones with mixed endian became excessively complicated, so I used cfg_if to separate them. It looks like you can only nest s! {} in cfg_if! {}, but not vice versa.

As a note, I'm considering removing musl_not_time64, and just keeping the old logic of allowing deprecated for function definitions involving time_t as it introduces necessary complexity.

When libc 1.0 is released, I believe the best path would be to remove the musl_v1_2_3 feature, making it unconditionally enabled, keeping musl_time64 which will expand to (musl && time64_arch).

Tested through QEMU for all architectures.

Sources

Sources are located on each commit, in the form of upstream commits

Checklist

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see rust-lang/libc#3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot label +stable-nominated

@rustbot rustbot added A-CI Area: CI-related items O-arm O-linux O-linux-like O-mips O-musl O-unix O-x86 S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels May 26, 2025
@xbjfk xbjfk changed the title musl: time64 musl: 64-bit time support May 26, 2025
@xbjfk xbjfk mentioned this pull request May 26, 2025
5 tasks
@xbjfk
Copy link
Contributor Author

xbjfk commented May 26, 2025

Hmm, seems like the style checker isn't happy with mixing s! {} and cfg_if! {}, and putting cfg_if! {} inside an s! {} block does not seem to work either. I'm not sure what the best course of action is regarding this.

@xbjfk
Copy link
Contributor Author

xbjfk commented May 30, 2025

Since #4433 seems close to merging, I can rebase on top of it once merged.

@xbjfk xbjfk force-pushed the musl-time64 branch 6 times, most recently from 8b32bb3 to e91f182 Compare June 3, 2025 08:26
@xbjfk
Copy link
Contributor Author

xbjfk commented Jun 3, 2025

Rebased on top of the GNU changes :)

@tgross35
Copy link
Contributor

tgross35 commented Jun 3, 2025

Thanks! Sorry it's taken me a while, I'll try to look at this very soon

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! Left a few comments here.

For the style check, are the changes it is suggesting possible? If not, that will probably have to be updated unfortunately (hopefully we will be able to reorganize things at some point so it's less annoying...)

@xbjfk
Copy link
Contributor Author

xbjfk commented Jun 15, 2025

Thanks for the review! I'll get around to implementing the changes hopefully in the next few days. I'll try figure out how to pass the style lints - but it might need a new file.

After this merges and when 1.0 is closer to release feel free to ping me on GitHub and I can help fully complete the transition for these changes in the 1.0 branch on the musl side :).

@xbjfk xbjfk force-pushed the musl-time64 branch 2 times, most recently from 9b5ca40 to f669296 Compare June 23, 2025 11:51
@tgross35
Copy link
Contributor

tgross35 commented Nov 2, 2025

Okay, I've addressed most comments - unfortunately, it seems like there is an issue with compiling for hexagon - but it looks like it still appears on main.

Fyi we're skipping hexagon now, should be picked up if you rebase

@tgross35
Copy link
Contributor

@xbjfk gentle ping here, would you be able to take a look at this? If you're able to rebase I can probably apply at least a few of the patches from this series, to reduce the remaining surface area.

@rustbot

This comment has been minimized.

@xbjfk xbjfk force-pushed the musl-time64 branch 2 times, most recently from c20d0e7 to b2c6f8e Compare December 2, 2025 13:18
@xbjfk
Copy link
Contributor Author

xbjfk commented Dec 2, 2025

Thanks for the nudge! Have rebased and applied most comments. Still need to confirm why input_event and make sure to double check and add missing Paddings (only did it near merge conflicts). Also need to find a nice way to handle sched_param on emscripten without failing - might just need to duplicate the struct

src/new/mod.rs Outdated
} else if #[cfg(target_os = "emscripten")] {
mod emscripten;
pub(crate) use emscripten::*;
pub use musl::sched::sched_util;
Copy link
Contributor

Choose a reason for hiding this comment

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

For the test failure: drop this bit...

Comment on lines 112 to 115
#[cfg(not(any( target_env = "musl", target_os = "emscripten", target_env = "ohos")))]
pub struct sched_param {
pub sched_priority: c_int,
#[cfg(any(target_env = "musl", target_os = "emscripten", target_env = "ohos"))]
pub sched_ss_low_priority: c_int,
#[cfg(any(target_env = "musl", target_os = "emscripten", target_env = "ohos"))]
pub sched_ss_repl_period: crate::timespec,
#[cfg(any(target_env = "musl", target_os = "emscripten", target_env = "ohos"))]
pub sched_ss_init_budget: crate::timespec,
#[cfg(any(target_env = "musl", target_os = "emscripten", target_env = "ohos"))]
pub sched_ss_max_repl: c_int,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

... and restore these fields, but with only #[cfg(target_os = "emscripten")] (update the cfg(not) of course.

Or create a src/new/emscripten/sched.rs file and copy the deprecated version from musl there.

@tgross35
Copy link
Contributor

tgross35 commented Dec 3, 2025

Still need to confirm why input_event and make sure to double check and add missing Paddings (only did it near merge conflicts). Also need to find a nice way to handle sched_param on emscripten without failing - might just need to duplicate the struct

Oh, sorry, that's most of what I just left comments about 😆 I think this is very close to ready though

@tgross35
Copy link
Contributor

tgross35 commented Dec 3, 2025

Sorry for the conflicts here - more padding changes in #4862, and a build config update in 5c4255f.

xbjfk added 5 commits December 4, 2025 12:05
The structures are the same - and this follows the upstream definition
On musl at least, blkcnt64_t == blkcnt_t == i64 and
ino_t == ino64_t == u64
This feature is enabled with independently from musl_v1_2_3 to support
time64.

Defining this feature makes this roughly equivalent to upstream
commit bminor/musl@f12bd8e.
This corresponds to upstream commit bminor/musl@1febd21 (most symbols)
and bminor/musl@22daaea (only dlsym)
@rustbot
Copy link
Collaborator

rustbot commented Dec 4, 2025

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

A bunch of properties were removed upstream and set to reserved.

This matches upstream commit bminor/musl@827aa8f and bminor/musl@2d69fcf
@xbjfk xbjfk force-pushed the musl-time64 branch 2 times, most recently from d15a765 to 81dcc04 Compare December 4, 2025 13:20
xbjfk added 8 commits December 4, 2025 13:36
Change time_t type to i64
Change struct stat, msqid_ds and shmid_ds to reflect

This commit follows upstream bminor/musl@3814333 and bminor/musl@d6dcfe4
It also implements a fix from bminor/musl@0fbd7d6
This is primarily based on a small part of bminor/musl@3814333.

This also integrates bminor/musl@3c02bac, which update MSG_STAT,
SEM_STAT, SEM_STAT_ANY. These are based on the value of IPC_STAT,
however we can just use `cfg` as it is effectively the same.
Namely, this allows `target_endian` as well as adds a constant array
where certain configs are explicitly allowed (e.g. musl32_time64).
Unfortunately does not seem to handle cfg_if well
@xbjfk
Copy link
Contributor Author

xbjfk commented Dec 4, 2025

Okay - I should have resolved everything. I added a commit to change stat's inlined timespec to simply timespec and changed the tests to account for that. Note the naming is different - st_xtim instead of st_xtime. It seems migration to timespec was done in POSIX 2008 - man 3 stat has more info

I also reverted the commit which disabled time related tests. I'll fix any test failures hopefully tomorrow.

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

Labels

A-CI Area: CI-related items O-arm O-linux O-linux-like O-mips O-musl O-powerpc O-riscv O-unix O-x86 S-waiting-on-author stable-nominated This PR should be considered for cherry-pick to libc's stable release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants