Skip to content

Deprecate windows time64_t #5032

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.

Sign up for GitHub

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

Open
dybucc wants to merge 1 commit into
base: main
Choose a base branch
from
+29 −18
Open

Deprecate windows time64_t #5032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions libc-test/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,6 @@ fn test_cygwin(target: &str) {
fn test_windows(target: &str) {
assert!(target.contains("windows"));
let gnu = target.contains("gnu");
let i686 = target.contains("i686");

let mut cfg = ctest_cfg();

Expand Down Expand Up @@ -818,19 +817,9 @@ fn test_windows(target: &str) {
cfg.skip_alias(move |alias| match alias.ident() {
"SSIZE_T" if !gnu => true,
"ssize_t" if !gnu => true,
// FIXME(windows): The size and alignment of this type are incorrect
"time_t" if gnu && i686 => true,
_ => false,
});

cfg.skip_struct(move |struct_| {
match struct_.ident() {
// FIXME(windows): The size and alignment of this struct are incorrect
"timespec" if gnu && i686 => true,
_ => false,
}
});

cfg.skip_const(move |constant| {
match constant.ident() {
// FIXME(windows): API error:
Expand Down
22 changes: 22 additions & 0 deletions libc-test/tests/windows_time.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//! Ensures Windows `time`-related routines align with `libc`'s interface. By
//! default, both MSVC and GNU (under `mingw`) expose 64-bit symbols. `libc`
//! also does that, but there's been slight inconsistencies in the past.

#![cfg(windows)]

/// Ensures a 64-bit write is performed on values that should always be 64 bits.
/// This may fail if
/// (1) the routine links with its 32-bit variant. This only happens if
/// `_USE_32BIT_TIME_T` is defined. In theory, this should not be
/// possible when working with Rust's `libc`.
/// (2) Or `time_t` is 32-bits, and a 64-bit write overwrites both array items.
/// This should neither be possible unless the above macro is defined.
/// Support for non-64-bit values in `libc` should thus be non-existent.
#[test]
fn test_64_bit_store() {
let mut time_values: [libc::time_t; 2] = [123, 456];
let ptr = time_values.as_mut_ptr();
unsafe { libc::time(ptr) };
assert!(time_values[0] != 123);
assert_eq!(time_values[1], 456);
}
14 changes: 7 additions & 7 deletions src/windows/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,7 @@ pub type clock_t = i32;

pub type errno_t = c_int;

cfg_if! {
if #[cfg(all(target_arch = "x86", target_env = "gnu"))] {
pub type time_t = i32;
} else {
pub type time_t = i64;
}
}
pub type time_t = i64;
Copy link
Copy Markdown
Contributor

@tgross35 tgross35 Apr 10, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

View changes since the review

Have you run any tests before and after this change for whether things work? I'm wondering about this group of functions where it seems like we're already linking the time64 version in some cases

libc/src/windows/mod.rs

Lines 393 to 413 in ff0e616

pub fn ctime(sourceTime: *const time_t) -> *mut c_char;
pub fn difftime(timeEnd: time_t, timeStart: time_t) -> c_double;
#[link_name = "_gmtime64_s"]
pub fn gmtime_s(destTime: *mut tm, srcTime: *const time_t) -> c_int;
#[link_name = "_get_daylight"]
pub fn get_daylight(hours: *mut c_int) -> errno_t;
#[link_name = "_get_dstbias"]
pub fn get_dstbias(seconds: *mut c_long) -> errno_t;
#[link_name = "_get_timezone"]
pub fn get_timezone(seconds: *mut c_long) -> errno_t;
#[link_name = "_get_tzname"]
pub fn get_tzname(
p_return_value: *mut size_t,
time_zone_name: *mut c_char,
size_in_bytes: size_t,
index: c_int,
) -> errno_t;
#[link_name = "_localtime64_s"]
pub fn localtime_s(tmDest: *mut tm, sourceTime: *const time_t) -> crate::errno_t;
#[link_name = "_time64"]
pub fn time(destTime: *mut time_t) -> time_t;
, and am thinking that we might have a bug with the current (before this PR) impl.

To test this you could use the libc from main and pass a pointer to a [time_t; 2] to the time function, both initialized to 0x1234abcd or whatever. If only one of them gets overwritten with the current time, there isn't a bug and things are working despite the link name. If one gets the current time and the other gets zeroed, then we have a bug.

This PR would fix it but we can't backport because of the breakage, so we'll need configure the link_name attributes behind not(x86+gnu).

(I realize that's rather confusing, let me know if anything needs clarification)

Copy link
Copy Markdown
Contributor Author

@dybucc dybucc Apr 10, 2026

Choose a reason for hiding this comment

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

Regarding the first question on tests, I have run the existing libc test suite
both before and after the changes, though admittedly I did not run it prior to
making the changes to src/windows/mod.rs, but without skipping time_t in
build.rs.

I'm not sure what do you mean when you say those routines may be faulty in the
current main worktree. From what I could gather of the Windows CRT docs (e.g.
[1], as all routines explain things similarly in this respect,) there
shouldn't be any issues, considering the non-suffixed variants are just wrappers
for the suffixed, 64-bit variants. So linking the 64-bit symbol with a type that
is also 64-bits wide now should be fine. Still, I understand there may be some
concerns with GNU on Windows, as in those cases, the expected time_t (prior to
this PR) was 32-bits wide, so that could be an issue.

Either way, I'll get back in a few hours once I actually run your test.

Copy link
Copy Markdown
Contributor Author

@dybucc dybucc Apr 11, 2026

Choose a reason for hiding this comment

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

There may be a bug in the current main worktree. Running on Windows x86 GNU,
with a two-element array of 32-bit time_t, a 64-bit store is being performed
on the pointer's pointee that gets passed to one of these 64-bit-linked routines
(tested it out with time() in libc, which links by default on all
targets/environments to _time64()) as one of the elements is zeroed after the
call.

I would personally say the bug is not in linking these routines by default to
their 64-bit variants, but rather in the fact that we assume a 32-bit time_t
on Windows GNU x86. By default, Rust's libc behaves correctly under MSVC, as
time_t is always 64-bit wide unless USE_32BIT_TIME_T is defined, no matter
the target triple, and in libc we opted out of adding support for that feature
test macro. On the other hand, when running under mingw with GNU in x86, we
always define time_t to be 32-bit wide. This, according to [1] and [2], is
incorrect, as behavior in mingw is exactly like that in MSVC when it comes to
bit-width; Namely, define USE_32BIT_TIME_T if you want 32-bit time_t and
non-suffixed variants to default to their 32-bit variants, otherwise, it's
64-bit time_t all the way through.

Running the test with the changes in this PR solves the issues, simply because
there's no 32-bit time_t exposed on Rust's libc under Windows anymore.

Copy link
Copy Markdown
Contributor

@tgross35 tgross35 Apr 13, 2026

Choose a reason for hiding this comment

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

Would you be able to add that test to libc-test/tests?

Copy link
Copy Markdown
Contributor Author

@dybucc dybucc Apr 14, 2026

Choose a reason for hiding this comment

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

Done.


pub type off_t = i32;
pub type dev_t = u32;
Expand All @@ -34,6 +28,12 @@ extern_ty! {
pub enum timezone {}
}

#[deprecated(
since = "1.0.0",
note = "This time-related value, among others, is part of the shift \
towards using a single, 64-bit-sized `time_t`. See #PENDING for \
discussion."
)]
pub type time64_t = i64;

pub type SOCKET = crate::uintptr_t;
Expand Down
Loading