Add new cfg gnu_file_offset_bits64 corresponding to _FILE_OFFSET_BITS=64 #4345

snogge · 2025-03-20T14:18:44Z

Description

Add a new cfg gnu_file_offset_bits64 corresponding to
-D_FILE_OFFSET_BITS=64.
The cfg can also be enabled by setting the environment variable
RUST_LIBC_UNSTABLE_GNU_FILE_OFFSET_BITS64=1.

This is the second part of what was #3175 .

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

I've tested this as widely as possible, but I have not been able to
run the sparc tests. mips and powerpc tests have been run on rust
1.85 using the yocto project, but it's not a fully compatible
environment for some reason.

Less commands makes for a cleaner `set -x` log. And it is more efficient.

Variables set with `env` in the matrix never propagated into the environment. Add a step in test_tier1 and test_tier2 that reads the env context from the matrix and adds the variables to the environment used by later steps.

The `Create I artifacts` step is always run, whether earlier steps succeeds or not. But the upload step would only run if all preceeding steps wer successfull. Add a conditional to always run except if artifact creation failed.

rustbot · 2025-03-20T14:18:50Z

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

tgross35

I haven't checked the type changes in detail but this looks pretty good to me. Left a handful of comments mostly related to configuration.

Thank you so much for working on this!

tgross35 · 2025-03-22T03:12:56Z

build.rs

+                let target_env = env::var("CARGO_CFG_TARGET_ENV").unwrap();
+                let target_os = env::var("CARGO_CFG_TARGET_OS").unwrap();
+                let target_ptr_width = env::var("CARGO_CFG_TARGET_POINTER_WIDTH").unwrap();
+                let target_arch = env::var("CARGO_CFG_TARGET_ARCH").unwrap();


Nit, could you move these out of this block and toward the top of main? Something else may want to use them.

unwrap_or_default() may be better too, I doubt anybody is building this without Cargo but it's probably good to be careful just in case.

tgross35 · 2025-03-22T03:13:45Z

build.rs

+        Ok(val) => {
+            if val == "1" {


Suggested change

Ok(val) => {

if val == "1" {

Ok(val) => if val == "1" {

Nit to remove an indent level. The next arm needs to change to Ok(_v) | Err(_e).

tgross35 · 2025-03-22T03:15:35Z

build.rs

+                    && target_arch != "riscv32"
+                    && target_arch != "x86_64"


x86_64 won't ever get matched here since target_ptr_width is checked right? And then, what is special about riscv32?

It would probably be good to add a riscv32 test in CI since unfortunately we don't have one yet (not here of course)

x86_64-unknown-linux-gnux32 have 32bit pointer but always 64 bit time_t.
riscv32 also always have 64-bit time_t.

tgross35 · 2025-03-22T03:17:18Z

.github/workflows/ci.yaml

+              RUST_LIBC_UNSTABLE_GNU_FILE_OFFSET_BITS64: 1
+            artifact-tag: offset-bits64
+          # - target: powerpc-unknown-linux-gnu
+          # - target: powerpc-unknown-linux-gnu


Could you keep the FIXME(ppc): SIGILL running tests ... note?

Oops, I meant to include that.

tgross35 · 2025-03-22T03:19:26Z

ci/run-docker.sh

-        --volume "$(pwd)":/checkout:ro \
-        --volume "$(pwd)"/target:/checkout/target \
+        --volume "$PWD":/checkout:ro \
+        --volume "$PWD"/target:/checkout/target \


Is this just style (fine if so) or did some platform not work with PWD?

Not just style, running the script under set -x will give prints for the execution of the $(pwd) subprocess.
At the moment I was having a hard time getting the CI to do what I wanted and the extra prints were annoying.

tgross35 · 2025-03-22T03:21:05Z

libc-test/build.rs

+        Ok(val) => {
+            if val == "1" {
+                if target.contains("gnu")


Suggested change

Ok(val) => {

if val == "1" {

if target.contains("gnu")

Ok(val) if val == "1" && target.contains("gnu") && ...

Same nit as above

tgross35 · 2025-03-22T03:22:15Z

src/unix/linux_like/linux/arch/mips/mod.rs

+        target_env = "gnu",
+        gnu_file_offset_bits64
+    ))] {
+        pub const RLIM_INFINITY: crate::rlim_t = !0;


Is this meant to be the same as the above block? If so, the conditions could be merged.

The conditions tend to be complicated, but I'll push an updated variant.

tgross35 · 2025-03-22T03:23:20Z

src/unix/linux_like/linux/gnu/b32/arm/mod.rs

+        // #[cfg(not(gnu_file_offset_bits64))]
+        // __st_ino: crate::ino_t,
+        // #[cfg(gnu_file_offset_bits64)]
+        __st_ino: c_ulong,


What is the reason for this to be commented out?

This was included by mistake. I'll remove it.

tgross35 · 2025-03-22T03:25:43Z

build.rs

+    println!("cargo:rerun-if-env-changed=RUST_LIBC_UNSTABLE_GNU_FILE_OFFSET_BITS64");
+    match env::var("RUST_LIBC_UNSTABLE_GNU_FILE_OFFSET_BITS64") {


I think we may want to use RUST_LIBC_UNSTABLE_GNU_FILE_OFFSET_BITS and then have it be set to =32 or =64. Reason being, I think in 1.0 we will default to 64-bit but need to provide an environment variable to use 32.

~~Also, doesn't this need to set linux_time_bits64 as well?~~ edit: nope, mixing my time and files

tgross35 · 2025-03-22T03:28:35Z

src/unix/mod.rs

+    if #[cfg(not(target_env = "gnu"))] {
+        missing! {
+            #[cfg_attr(feature = "extra_traits", derive(Debug))]
+            pub enum fpos_t {} // FIXME(unix): fill this out with a struct
+        }
+    }


Is this not provided at all for glibc? Or only with 64-bit time.

It was only provided as an enum before, this PR provides it for gnu/glibc. The definition for glibc is in src/unix/linux_like/linux/gnu/mod.rs.
The fpos_t definition in src/unix/mod.rs and fpos64_t definition in src/unix/linux_like/linux/mod.rs are now only for non-gnu libcs.

Add new jobs for i686 in test_tier1 and arm and powerpc in test_tier2 where RUST_LIBC_UNSTABLE_GNU_FILE_OFFSET_BITS=64. Use artifact-tag to avoid artifact name collisions.

Set the basic types correctly for gnu_file_offset_bits64 (_FILE_OFFSET_BITS=64).

gnu_file_offset_bits64 means _FILE_OFFSET_BITS=64.

When _FILE_OFFSET_BITS=64, glibc redirects some function calls to 64 bit versions. These symbols are sometimes the public LFS variants, sometimes hidden variants.

Like mips, the stat struct will become different once support for gnu_file_offset_bits64 is added.

Like mips and powerpc, the stat struct will become different once support for gnu_file_offset_bits64 is added.

Change the __padX members in b32/mod.rs from short to uint even though they are actually unsigned short in C. Using unsigned int will give the same alignment, and make the struct equivalent to stat64 when gnu_file_offset_bits64 is set.

Struct stat and stat64 needs to match when gnu_file_offset_bits64 is set.

The __f_unused field should be the same in statvfs and statvfs64 (where it was already included) as can be seen in https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/bits/statvfs.h;h=1aed2f54aa86e43ac1c1d3a33197b3232be76580;hb=HEAD

snogge · 2025-03-24T13:30:16Z

@rustbot ready

snogge added 3 commits March 12, 2025 23:03

ci: Use $PWD instead of $(pwd) in run-docker

6fbaf53

Less commands makes for a cleaner `set -x` log. And it is more efficient.

ci: Add matrix env variables to the environment

a2b2bb0

Variables set with `env` in the matrix never propagated into the environment. Add a step in test_tier1 and test_tier2 that reads the env context from the matrix and adds the variables to the environment used by later steps.

ci: Always upload successfully created artifacts

899fb69

The `Create I artifacts` step is always run, whether earlier steps succeeds or not. But the upload step would only run if all preceeding steps wer successfull. Add a conditional to always run except if artifact creation failed.

rustbot assigned tgross35 Mar 20, 2025

rustbot added A-CI Area: CI-related items O-arm O-gnu O-linux O-linux-like O-mips O-sparc O-unix O-x86 S-waiting-on-review labels Mar 20, 2025

tgross35 requested changes Mar 22, 2025

View reviewed changes

rustbot added S-waiting-on-author and removed S-waiting-on-review labels Mar 22, 2025

tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Mar 22, 2025

snogge added 11 commits March 24, 2025 14:23

gnu: build settings for _FILE_OFFSET_BITS=64

7e684c4

gnu: Set up the CI for _FILE_OFFSET_BITS=64

c85a253

Add new jobs for i686 in test_tier1 and arm and powerpc in test_tier2 where RUST_LIBC_UNSTABLE_GNU_FILE_OFFSET_BITS=64. Use artifact-tag to avoid artifact name collisions.

gnu: Handle basic file types for 32bit with _FILE_OFFSET_BITS=64

5234c5d

Set the basic types correctly for gnu_file_offset_bits64 (_FILE_OFFSET_BITS=64).

gnu: Update F_GETLK for gnu_file_offset_bits64

19ac1c2

gnu_file_offset_bits64 means _FILE_OFFSET_BITS=64.

gnu: Update F_SETLK and F_SETLKW for gnu_file_offset_bits64

3ab45a3

gnu: Set RLIM_INFINITY for mips with gnu_file_offset_bits64

a7272c8

gnu: Use _FILE_OFFSET_BITS=64 versions of glibc symbols

304076e

When _FILE_OFFSET_BITS=64, glibc redirects some function calls to 64 bit versions. These symbols are sometimes the public LFS variants, sometimes hidden variants.

gnu powerpc: Use a separate stat struct for powerpc

bce5501

Like mips, the stat struct will become different once support for gnu_file_offset_bits64 is added.

gnu sparc: Use a separate stat struct for 32bit powerpc

f4ccd52

Like mips and powerpc, the stat struct will become different once support for gnu_file_offset_bits64 is added.

gnu: Adapt struct stat for gnu_file_offset_bits64

8d49d3a

Change the __padX members in b32/mod.rs from short to uint even though they are actually unsigned short in C. Using unsigned int will give the same alignment, and make the struct equivalent to stat64 when gnu_file_offset_bits64 is set.

gnu: Adapt stat64 for gnu_file_offset_bits64

374cd9d

snogge added 7 commits March 24, 2025 14:24

gnu: Correct the struct stat64 padding for 32bit mips

40e9b2b

gnu: Correct struct stat64 for sparc

fed7cba

Struct stat and stat64 needs to match when gnu_file_offset_bits64 is set.

gnu: Add missing f_flags field to struct statfs for sparc

6885c2a

gnu: Add proper structs for fpos_t and fpos64_t

a8e6918

gnu: Update struct aiocb for gnu_file_offset_bits64

0533a56

gnu: Adapt struct flock on mips for gnu_file_offset_bits64

cf9c316

snogge force-pushed the gnu-file-offset-64 branch from aba1e20 to cf9c316 Compare March 24, 2025 13:24

rustbot added S-waiting-on-review and removed S-waiting-on-author labels Mar 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new cfg gnu_file_offset_bits64 corresponding to _FILE_OFFSET_BITS=64 #4345

Add new cfg gnu_file_offset_bits64 corresponding to _FILE_OFFSET_BITS=64 #4345

snogge commented Mar 20, 2025 •

edited

Loading

rustbot commented Mar 20, 2025

tgross35 left a comment

tgross35 Mar 22, 2025

snogge Mar 24, 2025

tgross35 Mar 22, 2025

snogge Mar 24, 2025

tgross35 Mar 22, 2025

snogge Mar 24, 2025

tgross35 Mar 22, 2025

snogge Mar 24, 2025

tgross35 Mar 22, 2025

snogge Mar 24, 2025

tgross35 Mar 22, 2025

snogge Mar 24, 2025

tgross35 Mar 22, 2025

snogge Mar 24, 2025

tgross35 Mar 22, 2025

snogge Mar 24, 2025

tgross35 Mar 22, 2025

tgross35 Mar 22, 2025 •

edited

Loading

tgross35 Mar 22, 2025

snogge Mar 24, 2025

snogge commented Mar 24, 2025

		println!("cargo:rerun-if-env-changed=RUST_LIBC_UNSTABLE_GNU_FILE_OFFSET_BITS64");
		match env::var("RUST_LIBC_UNSTABLE_GNU_FILE_OFFSET_BITS64") {

Add new cfg gnu_file_offset_bits64 corresponding to _FILE_OFFSET_BITS=64 #4345

Are you sure you want to change the base?

Add new cfg gnu_file_offset_bits64 corresponding to _FILE_OFFSET_BITS=64 #4345

Conversation

snogge commented Mar 20, 2025 • edited Loading

Description

Sources

Checklist

rustbot commented Mar 20, 2025

tgross35 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgross35 Mar 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snogge commented Mar 24, 2025

snogge commented Mar 20, 2025 •

edited

Loading

tgross35 Mar 22, 2025 •

edited

Loading