Skip to content

Commit

Permalink
Work around a negative timestamps bug on macOS. (#1305)
Browse files Browse the repository at this point in the history
* Work around a negative timestamps bug on macOS.

As described [here], macOS can sometimes return `timespec` values
with negative `tv_nsecs` values. Adjust `timespec` values as needed
to ensure that `tv_nsec` is in 0..1_000_000_000.

[here]: rust-lang/rust#108277 (comment)

* Make st_mtime etc. signed.

* Define our own `Stat` on NetBSD, fix types for Apple.

* More stat layout checks.

* powerp64 defines a stat64 but doesn't use it.

* Make `st_size` signed on s390x.
  • Loading branch information
sunfishcode authored Feb 1, 2025
1 parent 40733be commit 912db92
Show file tree
Hide file tree
Showing 12 changed files with 393 additions and 176 deletions.
58 changes: 47 additions & 11 deletions src/backend/libc/fs/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,9 +611,15 @@ pub(crate) fn stat(path: &CStr) -> io::Result<Stat> {
)
)))]
unsafe {
#[cfg(test)]
assert_eq_size!(Stat, c::stat);

let mut stat = MaybeUninit::<Stat>::uninit();
ret(c::stat(c_str(path), stat.as_mut_ptr()))?;
Ok(stat.assume_init())
ret(c::stat(c_str(path), stat.as_mut_ptr().cast()))?;
let stat = stat.assume_init();
#[cfg(apple)]
let stat = fix_negative_stat_nsecs(stat);
Ok(stat)
}
}

Expand Down Expand Up @@ -651,9 +657,15 @@ pub(crate) fn lstat(path: &CStr) -> io::Result<Stat> {
)
)))]
unsafe {
#[cfg(test)]
assert_eq_size!(Stat, c::stat);

let mut stat = MaybeUninit::<Stat>::uninit();
ret(c::lstat(c_str(path), stat.as_mut_ptr()))?;
Ok(stat.assume_init())
ret(c::lstat(c_str(path), stat.as_mut_ptr().cast()))?;
let stat = stat.assume_init();
#[cfg(apple)]
let stat = fix_negative_stat_nsecs(stat);
Ok(stat)
}
}

Expand Down Expand Up @@ -687,14 +699,20 @@ pub(crate) fn statat(dirfd: BorrowedFd<'_>, path: &CStr, flags: AtFlags) -> io::
)
)))]
unsafe {
#[cfg(test)]
assert_eq_size!(Stat, c::stat);

let mut stat = MaybeUninit::<Stat>::uninit();
ret(c::fstatat(
borrowed_fd(dirfd),
c_str(path),
stat.as_mut_ptr(),
stat.as_mut_ptr().cast(),
bitflags_bits!(flags),
))?;
Ok(stat.assume_init())
let stat = stat.assume_init();
#[cfg(apple)]
let stat = fix_negative_stat_nsecs(stat);
Ok(stat)
}
}

Expand Down Expand Up @@ -1443,9 +1461,15 @@ pub(crate) fn fstat(fd: BorrowedFd<'_>) -> io::Result<Stat> {
)
)))]
unsafe {
#[cfg(test)]
assert_eq_size!(Stat, c::stat);

let mut stat = MaybeUninit::<Stat>::uninit();
ret(c::fstat(borrowed_fd(fd), stat.as_mut_ptr()))?;
Ok(stat.assume_init())
ret(c::fstat(borrowed_fd(fd), stat.as_mut_ptr().cast()))?;
let stat = stat.assume_init();
#[cfg(apple)]
let stat = fix_negative_stat_nsecs(stat);
Ok(stat)
}
}

Expand Down Expand Up @@ -1850,17 +1874,17 @@ fn stat64_to_stat(s64: c::stat64) -> io::Result<Stat> {
st_size: s64.st_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blksize: s64.st_blksize.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blocks: s64.st_blocks.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_atime: bitcast!(i64::from(s64.st_atime)),
st_atime: i64::from(s64.st_atime),
st_atime_nsec: s64
.st_atime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_mtime: bitcast!(i64::from(s64.st_mtime)),
st_mtime: i64::from(s64.st_mtime),
st_mtime_nsec: s64
.st_mtime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_ctime: bitcast!(i64::from(s64.st_ctime)),
st_ctime: i64::from(s64.st_ctime),
st_ctime_nsec: s64
.st_ctime_nsec
.try_into()
Expand Down Expand Up @@ -2553,6 +2577,18 @@ pub(crate) fn fremovexattr(fd: BorrowedFd<'_>, name: &CStr) -> io::Result<()> {
}
}

/// See [`crate::timespec::fix_negative_nsec`] for details.
#[cfg(apple)]
fn fix_negative_stat_nsecs(mut stat: Stat) -> Stat {
stat.st_atime_nsec =
crate::timespec::fix_negative_nsecs(&mut stat.st_atime, stat.st_atime_nsec as _) as _;
stat.st_mtime_nsec =
crate::timespec::fix_negative_nsecs(&mut stat.st_mtime, stat.st_mtime_nsec as _) as _;
stat.st_ctime_nsec =
crate::timespec::fix_negative_nsecs(&mut stat.st_ctime, stat.st_ctime_nsec as _) as _;
stat
}

#[test]
fn test_sizes() {
#[cfg(linux_kernel)]
Expand Down
36 changes: 35 additions & 1 deletion src/backend/libc/fs/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ pub enum FlockOperation {
///
/// [`statat`]: crate::fs::statat
/// [`fstat`]: crate::fs::fstat
#[cfg(not(any(linux_like, target_os = "hurd")))]
#[cfg(not(any(linux_like, target_os = "hurd", target_os = "netbsd")))]
pub type Stat = c::stat;

/// `struct stat` for use with [`statat`] and [`fstat`].
Expand Down Expand Up @@ -921,6 +921,40 @@ pub struct Stat {
pub st_ino: u64,
}

/// `struct stat` for use with [`statat`] and [`fstat`].
///
/// [`statat`]: crate::fs::statat
/// [`fstat`]: crate::fs::fstat
// NetBSD's `st_mtime_nsec` is named `st_mtimensec` so we declare our own
// `Stat` so that we can be consistent with other platforms.
#[cfg(target_os = "netbsd")]
#[derive(Debug, Copy, Clone)]
#[allow(missing_docs)]
#[repr(C)]
pub struct Stat {
pub st_dev: c::dev_t,
pub st_mode: c::mode_t,
pub st_ino: c::ino_t,
pub st_nlink: c::nlink_t,
pub st_uid: c::uid_t,
pub st_gid: c::gid_t,
pub st_rdev: c::dev_t,
pub st_atime: c::time_t,
pub st_atime_nsec: c::c_long,
pub st_mtime: c::time_t,
pub st_mtime_nsec: c::c_long,
pub st_ctime: c::time_t,
pub st_ctime_nsec: c::c_long,
pub st_birthtime: c::time_t,
pub st_birthtime_nsec: c::c_long,
pub st_size: c::off_t,
pub st_blocks: c::blkcnt_t,
pub st_blksize: c::blksize_t,
pub st_flags: u32,
pub st_gen: u32,
pub st_spare: [u32; 2],
}

/// `struct statfs` for use with [`statfs`] and [`fstatfs`].
///
/// [`statfs`]: crate::fs::statfs
Expand Down
18 changes: 15 additions & 3 deletions src/backend/libc/time/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,10 @@ pub(crate) fn clock_gettime(id: ClockId) -> Timespec {
as_libc_timespec_mut_ptr(&mut timespec),
))
.unwrap();
timespec.assume_init()
let timespec = timespec.assume_init();
#[cfg(apple)]
let timespec = fix_negative_timespec_nsecs(timespec);
timespec
}
}

Expand Down Expand Up @@ -220,8 +223,10 @@ pub(crate) fn clock_gettime_dynamic(id: DynamicClockId<'_>) -> io::Result<Timesp
id as c::clockid_t,
as_libc_timespec_mut_ptr(&mut timespec),
))?;

Ok(timespec.assume_init())
let timespec = timespec.assume_init();
#[cfg(apple)]
let timespec = fix_negative_timespec_nsecs(timespec);
Ok(timespec)
}
}

Expand Down Expand Up @@ -476,3 +481,10 @@ fn timerfd_gettime_old(fd: BorrowedFd<'_>) -> io::Result<Itimerspec> {
},
})
}

/// See [`crate::timespec::fix_negative_nsecs`] for details.
#[cfg(apple)]
fn fix_negative_timespec_nsecs(mut ts: Timespec) -> Timespec {
ts.tv_nsec = crate::timespec::fix_negative_nsecs(&mut ts.tv_sec, ts.tv_nsec as _) as _;
ts
}
18 changes: 9 additions & 9 deletions src/backend/linux_raw/fs/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,11 +730,11 @@ fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {
st_size: x.stx_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blksize: x.stx_blksize.into(),
st_blocks: x.stx_blocks.into(),
st_atime: bitcast!(i64::from(x.stx_atime.tv_sec)),
st_atime: i64::from(x.stx_atime.tv_sec),
st_atime_nsec: x.stx_atime.tv_nsec.into(),
st_mtime: bitcast!(i64::from(x.stx_mtime.tv_sec)),
st_mtime: i64::from(x.stx_mtime.tv_sec),
st_mtime_nsec: x.stx_mtime.tv_nsec.into(),
st_ctime: bitcast!(i64::from(x.stx_ctime.tv_sec)),
st_ctime: i64::from(x.stx_ctime.tv_sec),
st_ctime_nsec: x.stx_ctime.tv_nsec.into(),
st_ino: x.stx_ino.into(),
})
Expand All @@ -754,17 +754,17 @@ fn stat_to_stat(s64: linux_raw_sys::general::stat64) -> io::Result<Stat> {
st_size: s64.st_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blksize: s64.st_blksize.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blocks: s64.st_blocks.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_atime: bitcast!(i64::from(s64.st_atime)),
st_atime: i64::from(s64.st_atime),
st_atime_nsec: s64
.st_atime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_mtime: bitcast!(i64::from(s64.st_mtime)),
st_mtime: i64::from(s64.st_mtime),
st_mtime_nsec: s64
.st_mtime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_ctime: bitcast!(i64::from(s64.st_ctime)),
st_ctime: i64::from(s64.st_ctime),
st_ctime_nsec: s64
.st_ctime_nsec
.try_into()
Expand All @@ -786,17 +786,17 @@ fn stat_to_stat(s: linux_raw_sys::general::stat) -> io::Result<Stat> {
st_size: s.st_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blksize: s.st_blksize.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blocks: s.st_blocks.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_atime: bitcast!(i64::from(s.st_atime)),
st_atime: i64::from(s.st_atime),
st_atime_nsec: s
.st_atime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_mtime: bitcast!(i64::from(s.st_mtime)),
st_mtime: i64::from(s.st_mtime),
st_mtime_nsec: s
.st_mtime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_ctime: bitcast!(i64::from(s.st_ctime)),
st_ctime: i64::from(s.st_ctime),
st_ctime_nsec: s
.st_ctime_nsec
.try_into()
Expand Down
29 changes: 14 additions & 15 deletions src/backend/linux_raw/fs/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,15 +584,14 @@ pub enum FlockOperation {
///
/// [`statat`]: crate::fs::statat
/// [`fstat`]: crate::fs::fstat
// On 32-bit, and mips64, Linux's `struct stat64` has a 32-bit `st_mtime` and
// friends, so we use our own struct, populated from `statx` where possible, to
// avoid the y2038 bug.
// On 32-bit with `struct stat64` and mips64 with `struct stat`, Linux's
// `st_mtime` and friends are 32-bit, so we use our own struct, populated from
// `statx` where possible, to avoid the y2038 bug.
#[cfg(any(
target_pointer_width = "32",
target_arch = "mips64",
target_arch = "mips64r6"
))]
#[repr(C)]
#[derive(Debug, Copy, Clone)]
#[allow(missing_docs)]
#[non_exhaustive]
Expand Down Expand Up @@ -636,11 +635,11 @@ pub struct Stat {
pub st_size: ffi::c_long,
pub st_blksize: ffi::c_long,
pub st_blocks: ffi::c_long,
pub st_atime: ffi::c_ulong,
pub st_atime: ffi::c_long,
pub st_atime_nsec: ffi::c_ulong,
pub st_mtime: ffi::c_ulong,
pub st_mtime: ffi::c_long,
pub st_mtime_nsec: ffi::c_ulong,
pub st_ctime: ffi::c_ulong,
pub st_ctime: ffi::c_long,
pub st_ctime_nsec: ffi::c_ulong,
pub(crate) __unused: [ffi::c_long; 3],
}
Expand Down Expand Up @@ -698,7 +697,7 @@ pub struct Stat {
pub(crate) __unused4: ffi::c_uint,
pub(crate) __unused5: ffi::c_uint,
}
// This follows `stat64`.
// This follows `stat`. powerpc64 defines a `stat64` but it's not used.
#[repr(C)]
#[derive(Debug, Copy, Clone)]
#[allow(missing_docs)]
Expand All @@ -715,11 +714,11 @@ pub struct Stat {
pub st_size: ffi::c_long,
pub st_blksize: ffi::c_ulong,
pub st_blocks: ffi::c_ulong,
pub st_atime: ffi::c_ulong,
pub st_atime: ffi::c_long,
pub st_atime_nsec: ffi::c_ulong,
pub st_mtime: ffi::c_ulong,
pub st_mtime: ffi::c_long,
pub st_mtime_nsec: ffi::c_ulong,
pub st_ctime: ffi::c_ulong,
pub st_ctime: ffi::c_long,
pub st_ctime_nsec: ffi::c_ulong,
pub(crate) __unused4: ffi::c_ulong,
pub(crate) __unused5: ffi::c_ulong,
Expand All @@ -739,12 +738,12 @@ pub struct Stat {
pub st_gid: ffi::c_uint,
pub(crate) __pad1: ffi::c_uint,
pub st_rdev: ffi::c_ulong,
pub st_size: ffi::c_ulong,
pub st_atime: ffi::c_ulong,
pub st_size: ffi::c_long, // Linux has `c_ulong` but we make it signed.
pub st_atime: ffi::c_long,
pub st_atime_nsec: ffi::c_ulong,
pub st_mtime: ffi::c_ulong,
pub st_mtime: ffi::c_long,
pub st_mtime_nsec: ffi::c_ulong,
pub st_ctime: ffi::c_ulong,
pub st_ctime: ffi::c_long,
pub st_ctime_nsec: ffi::c_ulong,
pub st_blksize: ffi::c_ulong,
pub st_blocks: ffi::c_long,
Expand Down
Loading

0 comments on commit 912db92

Please sign in to comment.