newlib: fix definition of `time_t` and `off_t` by dybucc · Pull Request #5132 · rust-lang/libc

dybucc · 2026-06-01T17:25:31Z

Description

This PR addresses bit width mismatches for both time_t and file offset types. This applies to the linux/newlib module.

Upstream and forks for each supported target provide conditional definitions. time_t can be configured in the newlib build script. File offset types require manual intervention in a specific header file. The file containing the default definition will check for a macro that is defined in that specific "override" file. The default definition is a C long.

A number of supported targets were using either one of the above overrides. This was not being taken into account in the libc crate. This patch fixes that by further expanding conditionally compiled type definitions.

Sources

Cygwin's, espressif's and the Vita SDK's newlib forks showing conditional definition of the time_t type and the off_t type. Note Vita SDK's fork contains definitions relative to multiple target systems, but the one under include/sys has taken priority. The definition under include/vita/sys #includes the former to define its off_t type.

https://github.com/cygwin/cygwin/blob/6926523688bab730ae1b56f24ad6fb203b7350af/newlib/libc/include/sys/_types.h#L185
https://github.com/espressif/newlib-esp32/blob/esp-4.5.0/newlib/libc/include/sys/_types.h#L185
https://github.com/search?q=repo%3Avitasdk%2Fnewlib+%2F%5Cb_time_t_%5Cb%2F&type=code
https://github.com/cygwin/cygwin/blob/6926523688bab730ae1b56f24ad6fb203b7350af/newlib/libc/include/sys/_types.h#L100
https://github.com/espressif/newlib-esp32/blob/67537d91e601c40e2b52b9b865518abb7eecc85b/newlib/libc/include/sys/_types.h#L100
https://github.com/vitasdk/newlib/blob/fbb8375870d799758e0b4358d2f1708781aa26a6/newlib/libc/include/sys/_types.h#L100

Sources on devkitARM could not be found online, but they can be found under the installed toolchain's directory in devkitARM/ by running ripgrep with the following command.
```
rg -C 5 -S -uu -g "**/*.h" -e "\b_time_t_\b"
```
Newlib sources documenting the configure build script non-default option for using a c_long for their time_t definition.

https://github.com/cygwin/cygwin/blob/6926523688bab730ae1b56f24ad6fb203b7350af/newlib/README#L351
Cygwin upstream codesearch showing the definition of _off_t, which defines __off_t, and eventually off_t. Note how this is defined in machine/_types.h (as an i64,) and later that is checked in sys/_types.h (falling back to a c_long.)

https://github.com/search?q=repo%3Acygwin%2Fcygwin%20symbol%3A%2F%5Cb_*off_t%5Cb%2F&type=code
espressif's esp-32 fork of newlib, vita's and RTEMS' showing how they don't have a definition in terms an i64 under machine/_types.h.

https://github.com/search?q=repo%3Aespressif%2Fnewlib-esp32+path%3Anewlib%2Flibc%2Finclude+%2F%5Cb_*off_t%5Cb%2F&type=code
https://github.com/search?q=repo%3Avitasdk%2Fnewlib+path%3Anewlib%2Flibc%2Finclude+%2F%5Cb_*off_t%5Cb%2F&type=code
https://gitlab.rtems.org/search?search=path%3Anewlib%2Flibc%2Finclude+extension%3Ah+%22_*off_t%22&nav_source=navbar&project_id=416&group_id=1086&search_code=true&repository_ref=main

Checklist

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

dybucc · 2026-06-02T06:56:53Z

CI seems to be failing for reasons unrelated to the changes introduced in the patch. A rerun should
do it.

dybucc · 2026-06-09T07:32:01Z

CI actually passes. There seems to be an issue with a glob import that is not used, but this has not
been changed in the patch (it's not even part of it, for that matter.) For some reason, rebasing
onto main with dependabot updates has ended up with a warning across all of my open PRs due to
that one (now apparently unused) import.

tgross35 · 2026-06-19T07:55:44Z

I couldn't remember what the possible env+os combinations were for these platforms, here's a quick list.

> $cfg | find 'target_env="newlib"' | get tgt | each {|tgt| $cfg | find $tgt -n | find -nr '(target_env|target_os)' } | flatten
  #               tgt                        cfg
─────────────────────────────────────────────────────────
  0   armv6k-nintendo-3ds            target_env="newlib"
  1   armv6k-nintendo-3ds            target_os="horizon"
  2   armv7-rtems-eabihf             target_env="newlib"
  3   armv7-rtems-eabihf             target_os="rtems"
  4   armv7-sony-vita-newlibeabihf   target_env="newlib"
  5   armv7-sony-vita-newlibeabihf   target_os="vita"
  6   riscv32imac-esp-espidf         target_env="newlib"
  7   riscv32imac-esp-espidf         target_os="espidf"
  8   riscv32imafc-esp-espidf        target_env="newlib"
  9   riscv32imafc-esp-espidf        target_os="espidf"
 10   riscv32imc-esp-espidf          target_env="newlib"
 11   riscv32imc-esp-espidf          target_os="espidf"
 12   xtensa-esp32-espidf            target_env="newlib"
 13   xtensa-esp32-espidf            target_os="espidf"
 14   xtensa-esp32s2-espidf          target_env="newlib"
 15   xtensa-esp32s2-espidf          target_os="espidf"
 16   xtensa-esp32s3-espidf          target_env="newlib"
 17   xtensa-esp32s3-espidf          target_os="espidf"

tgross35

I'm having some difficulty extracting a simplified goal from the top post. Is this the net delta on targets we currently support?

On armv7-rtems, time_t changes from i32 to i64
On armv7-rtems, off_t changes from i64 to c_long, which should be i64->i32
On armv7-vita, off_t changes from c_int to c_long. I think there is no change here?

If this sounds accurate, ping the target maintainer (see https://doc.rust-lang.org/rustc/platform-support/armv7-rtems-eabihf.html) for confirmation. This is generally a good idea when tricky things like this change on less popular targets, there may be config or other nuance that doesn't show up in the source code.

Also please don't call this a refactor, that term usually means cleaning up code without changing behavior - if the user experience changes somehow then it's kind of misleading. (Applies to some of your other PRs as well).

Note this option is "documented" as being meant for end users of the libc crate to enable, but no information on it is provided anywhere in all of the libc crate.

It's mostly intended to be set by espidf build tooling, but it would be good to at least mention it. A PR would be welcome in the crate-level docs, after #5179 adds some structure.

It would also be great to know whether the libc crate considers horizon to be the OS for both the Nintendo Switch and the Nintendo 3DS targets, or only for the Nintendo 3DS target.

aarch64-nintendo-switch-freestanding does not appear to set a target_env, so I would assume the latter.

View changes since this review

tgross35 · 2026-06-19T08:36:20Z

@rustbot author for clarification

rustbot · 2026-06-19T08:36:24Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

dybucc · 2026-06-19T10:55:59Z

The changes here are required to fit the definitions upstream of time_t, which was generally mismatched.

The changes for file offset types are concerned with either

having a mirrored definition, rather an an equivalent one, and
with the same bit width mismatches as with time_t.

I'll open a PR soon with the documentation for ESP-IDF.

@rustbot ready

dybucc · 2026-06-19T10:57:01Z

Hey @thesummer!

I was wondering if I could get confirmation on these changes on the RTEMS target you maintain?

dybucc · 2026-06-19T10:58:14Z

Hey @nikarh @pheki @zetanumbers!

I was wondering if I could get confirmation on these changes on the Sony PS Vita target you maintain?

pheki

Thanks for asking!

Just need a change as the Vita uses 32-bit time_t.

Keeping it on the top-level comment for easy reference, here are the sources I could find (somewhat redundant with the finds in the root PR comment but in vitasdk's fork):

`off_t`

`time_t`

--enable-newlib-long-time_t is set in CMakeLists.txt
In configure:
- newlib_long_time_t is set
- _WANT_USE_LONG_TIME_T is set
In configure.in:
- newlib_long_time_t is set
- _WANT_USE_LONG_TIME_T is set
_TIME_T_ is set
time_t is set

View changes since this review

pheki · 2026-06-19T22:11:07Z

+    if #[cfg(any(
+        target_os = "espidf",
+        target_os = "vita",
+        target_os = "rtems",
+        target_arch = "aarch64"
+    ))] {


For vita, this one seems correct, as by looking at the definitions it seems to be defined as a long. There should also be no change in behavior as both c_int and c_long are aliases to i32 in this platform.

Sources in the top level review comment.

pheki · 2026-06-20T01:02:29Z

+    } else {
+        pub type time_t = i64;


This one looks wrong for the vita, which uses 32-bit time, as --enable-newlib-long-time_t is set when building newlib in vitasdk (currently the only supported SDK).

The correct definition would be c_long (effectively i32).

Sources in the top-level comment.

Thanks. That's fixed now.

tgross35 · 2026-06-20T02:07:38Z

The changes here are required to fit the definitions upstream of time_t, which was generally mismatched.

The changes for file offset types are concerned with either
1. having a mirrored definition, rather an an equivalent one, and
2. with the same bit width mismatches as with `time_t`.

Indeed libc doesn't match every source, but with every change I need to figure out a succinct description of what (1) is changing to look more similar to source without affecting users, vs. (2) is a behavior change of some form. IOW I'm interested in the intent more than the problem list, to make sure I'm on the same page as PR authors.

To help with that, are the summary bullet points at #5132 (review) accurate?

The `newlib` module uses a faulty definition for `time_t`. That type falls back to a 32-bit signed integer definition except in `horizon` and `espidf`. The option to opt out of a 64-bit `time_t` is configurable. There were other supported targets that had it enabled in their build scripts. These were still having `time_t` exposed as 64-bits wide. This patch fixes that.

The `off_t` type was being exposed as a 32-bit signed integer in targets where it was 64-bits wide. The same applies the other way around. This option is configurable. It is not configurable in the build scripts. The default definition is to have a C `long` as `off_t`. This can be altered in another header file. That file defines a macro after defininig the type alias. The macro is then checked in the header file with the default definition. A number of supported targets use this overriding behavior. The `libc` crate is not currently taking them all into consideration. This patch fixes that.

rustbot · 2026-06-21T10:14:10Z

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.

dybucc · 2026-06-21T10:28:27Z

Just updated the PR description. It's hopefully clearer now.

The extent of the changes has now changed.

RTEMS is unaffected.
The changes on vita are as you outlined.
AArch64 had changes to its file offset types.
horizon has a gone from 32-bit to 64-bit time_t.

pheki · 2026-06-21T19:51:07Z

I have some questions, first regarding this:

having a mirrored definition, rather an an equivalent one, and

You kept the vita with pub type time_t = i32;. If the idea is to have a mirrored definition, shouldn't it be c_long?

horizon has a gone from 32-bit to 64-bit time_t.

Wasn't it already 64-bit, as c_longlong is an alias to i64 on all supported platforms?

RTEMS is unaffected.
AArch64 had changes to its file offset types.

It seems like time_t has changed from i32 to i64 for those, no?

dybucc changed the title ~~refactor: fix definition of time_t in Newlib~~ refactor: fix definition of time_t and off_t in Newlib Jun 3, 2026

dybucc force-pushed the newlib-fix-time_t branch 4 times, most recently from 7152f77 to e821035 Compare June 7, 2026 16:34

dybucc marked this pull request as ready for review June 7, 2026 16:35

rustbot added the S-waiting-on-review label Jun 7, 2026

dybucc force-pushed the newlib-fix-time_t branch from e821035 to de697e2 Compare June 9, 2026 07:11