Skip to content

refactor: fix definition of LFS64-related types in emscripten#5142

Open
dybucc wants to merge 1 commit into
rust-lang:mainfrom
dybucc:emscripten-largefile64-patch
Open

refactor: fix definition of LFS64-related types in emscripten#5142
dybucc wants to merge 1 commit into
rust-lang:mainfrom
dybucc:emscripten-largefile64-patch

Conversation

@dybucc

@dybucc dybucc commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Description

This PR addresses bit-width representation issues under emscripten concerned with the LFS64 "spec." The current interface we expose unconditionally makes the symbols available, even though the upstream musl fork for emscripten does define the _GNU_SOURCE feature test macro by default. This has been patched by conditionally compiling whenever it is that the gnu_file_offset_bits64 cfg option is detected at build time.

All of the routines concerned with LFS64 were already in a module of their own but were always exposed even though they are not always available in emscripten. According to [1], the _LARGEFILE64_SOURCE feature test macro is considered "deprecated" and the gated functionality should instead be provided by setting _FILE_OFFSET_BITS=64. At present, the libc crate aleady has a cfg for that. Still, the definition in emscripten (and in upstream musl) also requires the _GNU_SOURCE feature test macro to be enabled. This symbol is not made available by default in Linux unless either explicitly defined in source code or as part of the compilation pipeline, and makes available a number of GNU extensions. The libc crate does not have at the time of writing any cfgs gating this functionality, but this PR does not introduce it. Further discussion on this may be necessary.

This patch deprecates all such symbols (both suffixed routines and types) unless the cfg option is set. These changes, though, may prove to be useless. That's why no greater effort has been made into fixing the exposed interface, unless further confirmation from somebody else is provided. Tests don't pass with the module availability changes as there's overlapping definitions between parent modules to the emscripten module, and those in the emscripten module itself. This is caused by the modified link names of a few routines when running under gnu_file_offset_bits64. Were this PR to be viable, further changes will be made to fully fix this.

Sources

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

dybucc commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Style check fails for reasons outside the changes introduced in this PR. The formatting checks seem
to run on a post-macro expanded crate output. When annotating the item produced by the extern_ty
macro with an attribute, this expands with a wrongly indented function block, even though the actual
source code is correctly indented. The output log of the CI job shows this.

@dybucc dybucc force-pushed the emscripten-largefile64-patch branch from 21a8c5d to 9cdfb82 Compare June 4, 2026 14:46
@rustbot

This comment has been minimized.

@dybucc dybucc force-pushed the emscripten-largefile64-patch branch from 9cdfb82 to 4ce5a85 Compare June 9, 2026 07:11
@rustbot

This comment has been minimized.

@tgross35

Copy link
Copy Markdown
Contributor

@rustbot blocked
@SnoozeThis wait 2 months -> remove label S-blocked, add label S-waiting-on-review

Holding off until there is a good alternative to off64_t

@SnoozeThis

Copy link
Copy Markdown

(https://snoozeth.is/FoBvqh9wkR8) I will wait until Wed, 19 Aug 2026 08:32:43 UTC and then add label S-waiting-on-review and remove label S-blocked.

@rustbot claim.

@tgross35

Copy link
Copy Markdown
Contributor

Actually I think I'm missing something - what does this PR fix? I only see it adding deprecation warnings and deleting ino64_t (was that intentional?)

@dybucc

dybucc commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

The ino_t removal was not intentional. If I'm getting things right, PRs are getting freezed because there's yet to be a solution for 32-bit platforms. This should be no issue in emscripten, because the unsuffixed types were already fixed-precision, 64-bit wide types. So anybody who was using the LFS64 types can switch to the unsuffixed types with no regressions.

At the time of opening the PR, I forgot to mention it, but the folks in emscripten do things a bit differently with blkcnt_t. It uses a 32-bit signed integer no matter the platform. This ends up being quite misleading with blkcnt64_t because that type is also 32-bits wide as a consequence of being an alias. So the same applies as in the prior paragraph.

And then this transitively applies to the affected records that use these types.

Finally, there's the fact that the PR itself was opened as a draft to first get confirmation on the changes. If they were viable, then I would go ahead an update all of the top-level unix and unix/linux_like module definitions to avoid having link names changed from using the gnu_file_offset_bits64 cfg.

@dybucc dybucc force-pushed the emscripten-largefile64-patch branch from 4ce5a85 to 334a733 Compare June 19, 2026 10:35
@rustbot

rustbot commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

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.

The definitions for types that were made available under the LFS64 spec
were being exposed even when the right feature test macro for that is
now `FILE_OFFSET_BITS64`.

This has been patched by deprecating the current definition when
building without the existing `gnu_file_offset_bits64` `cfg` option.
Even though the definitions are exposed when both the afore mentioned
macro and the `_GNU_SOURCE` macro are defined, no `cfg` has been added
for the latter.
@dybucc dybucc force-pushed the emscripten-largefile64-patch branch from 334a733 to 145ecb7 Compare June 19, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants