Skip to content

Implement sys/fs/vibix.rs and sys/process/vibix.rs in std fork#867

Merged
dburkart merged 3 commits into
mainfrom
m854-std-fs-process
May 5, 2026
Merged

Implement sys/fs/vibix.rs and sys/process/vibix.rs in std fork#867
dburkart merged 3 commits into
mainfrom
m854-std-fs-process

Conversation

@dburkart
Copy link
Copy Markdown
Owner

@dburkart dburkart commented May 5, 2026

Summary

  • Implements Phase 2 of RFC 0009: std::fs and std::process for vibix
  • Adds vibix_abi::fs module with syscall wrappers for all filesystem operations (openat, read, write, fstat, lseek, getdents64, close, unlink, mkdir, rmdir, rename, link, symlink, readlink, chmod/fchmod, chown/fchown/lchown, utimensat, ftruncate, fsync, dup, fcntl, access, getcwd, chdir)
  • Adds vibix_abi::process module with syscall wrappers (fork, execve, exit, exit_group, wait4, kill, getpid)
  • Implements sys/fs/vibix.rs: File, OpenOptions, FileAttr, FilePermissions, FileType, ReadDir, DirEntry, DirBuilder, FileTimes
  • Implements sys/process/vibix.rs: Command::spawn via fork+execve, Process::wait via wait4, ExitStatus with Linux wait status decoding, getpid
  • Adds sys/time/vibix.rs with SystemTime supporting new/to_timespec for filesystem timestamps
  • Adds os::vibix module with OsStrExt/OsStringExt byte-based traits
  • File locking and Instant::now() are stubbed (require additional kernel support)

Closes #854

Test plan

  • cargo xtask build -- kernel compiles cleanly
  • __CARGO_TESTS_ONLY_SRC_ROOT=library cargo +nightly build -Z build-std=std,core,alloc,panic_abort -Z json-target-spec --target x86_64-unknown-vibix.json -- std compiles cleanly for vibix (only downstream crates needing restricted_std gate fail, which is expected for a non-upstreamed target)

🤖 Generated with Claude Code

Adds Phase 2 std support for vibix: filesystem and process operations.

- vibix_abi: Add fs module with syscall wrappers (openat, read, write,
  fstat, lseek, getdents64, close, unlink, mkdir, rmdir, rename, link,
  symlink, readlink, chmod, fchmod, chown, fchown, lchown, utimensat,
  ftruncate, fsync, dup, fcntl, access, getcwd, chdir)
- vibix_abi: Add process module with syscall wrappers (fork, execve,
  exit, exit_group, wait4, kill, getpid)
- std: Implement sys/fs/vibix.rs with File, OpenOptions, FileAttr,
  FilePermissions, FileType, ReadDir, DirEntry, DirBuilder, FileTimes
- std: Implement sys/process/vibix.rs with Command::spawn via
  fork+execve, Process::wait via wait4, ExitStatus, getpid
- std: Add sys/time/vibix.rs with SystemTime supporting new/to_timespec
  for filesystem timestamps
- std: Add os/vibix module with OsStrExt/OsStringExt traits
- std: Wire vibix into fs/mod.rs, process/mod.rs, time/mod.rs cfg_select

Verified: kernel builds (cargo xtask build), std builds for
x86_64-unknown-vibix target with -Z build-std.

Closes #854

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

Warning

Rate limit exceeded

@dburkart has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 2 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 50bf0aeb-fc88-45a7-8cad-5446e80e1a02

📥 Commits

Reviewing files that changed from the base of the PR and between 2806a2d and a3a8738.

📒 Files selected for processing (3)
  • library/std/src/sys/fs/vibix.rs
  • library/std/src/sys/process/vibix.rs
  • userspace/vibix_abi/src/fs.rs
📝 Walkthrough

Walkthrough

This PR implements Rust std support for the vibix operating system by adding platform-specific implementations for filesystem operations, process management, and time handling, coupled with userspace ABI syscall wrappers that bridge to the kernel.

Changes

Vibix Platform Support Implementation

Layer / File(s) Summary
ABI Foundation
userspace/vibix_abi/src/lib.rs, userspace/vibix_abi/src/fs.rs, userspace/vibix_abi/src/process.rs
Export fs and process modules; define Linux x86_64 syscall numbers, constants (open flags, seek modes, stat modes, dirent types), and ABI-compatible data structures (Stat, Dirent64, Timespec); provide unsafe syscall wrappers for file ops (openat, read, write, fstat, stat, lstat, lseek, getdents64, unlink, mkdir, rmdir, rename, link, symlink, readlink, chmod, fchmod, chown, utimensat, ftruncate, fsync, dup, fcntl, access, getcwd, chdir, ioctl), and process ops (fork, execve, exit, exit_group, wait4, kill, getpid), with constants SIGKILL and WNOHANG.
Sys Implementations
library/std/src/sys/fs/vibix.rs
Implement File (fd wrapper with drop-close), FileAttr, FilePermissions, FileType, FileTimes; ReadDir with dynamic getdents64 buffer and entry parsing; DirEntry with path/name/type resolution; OpenOptions with access-mode and creation-mode validation; DirBuilder; free functions for readdir, unlink, rename, chmod, utimensat, rmdir, remove_dir_all, readlink, symlink, link, stat, lstat, canonicalize (with symlink traversal), copy, exists, plus File I/O ops (open, read, write, seek, fsync, truncate, set_permissions, set_times) and vectored I/O fallbacks.
Sys Implementations
library/std/src/sys/process/vibix.rs
Implement Command with program/args/env/cwd, Stdio with variants, spawn() via fork() + optional chdir + execve with exit code 127 on failure; Process with id(), kill() (SIGKILL), blocking wait() loop via wait4, non-blocking try_wait() with WNOHANG; ExitStatus parsing from wait status; ExitStatusError with NonZero<i32> code; ExitCode with SUCCESS/FAILURE constants; CommandArgs iterator and debug formatting.
Sys Implementations
library/std/src/sys/time/vibix.rs
Define Instant wrapping Duration with checked arithmetic (checked_sub_instant, checked_add_duration, checked_sub_duration); SystemTime with tv_sec/tv_nsec, UNIX_EPOCH constant, new() with nanosecond validation, sub_time() for duration difference, checked add/subtract with carry/borrow normalization, and to_timespec() conversion (note: now() panics).
Module Wiring
library/std/src/sys/fs/mod.rs, library/std/src/sys/process/mod.rs, library/std/src/sys/time/mod.rs
Add cfg_select! arms for target_os = "vibix" to route filesystem, process, and time implementations to vibix modules respectively.
Std Exports
library/std/src/os/mod.rs, library/std/src/os/vibix/mod.rs, library/std/src/os/vibix/ffi.rs
Export new vibix module in os with #[cfg(target_os = "vibix")]; define os::vibix as stable module exporting ffi submodule; re-export OsStrExt and OsStringExt from shared Unix os_str module for FFI extensions.

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant Cmd as Command
    participant Spawn as spawn()
    participant Fork as fork()
    participant Child as Child Process
    participant Execve as execve()
    participant Kernel as Kernel
    participant Parent as Parent Process
    participant Wait as wait4()
    
    App->>Cmd: new(program)
    App->>Cmd: arg(arg), env, cwd
    App->>Spawn: spawn()
    Spawn->>Fork: fork()
    Fork->>Kernel: SYS_FORK
    Kernel-->>Fork: returns pid
    Fork-->>Parent: parent gets pid
    Fork-->>Child: child gets 0
    
    Note over Child: child process
    Child->>Child: chdir if needed
    Child->>Execve: execve(program, argv, envp)
    Execve->>Kernel: SYS_EXECVE
    alt execve succeeds
        Kernel-->>Execve: never returns
    else execve fails
        Execve->>Child: returns error
        Child->>Child: exit(127)
    end
    
    Note over Parent: parent process
    Parent-->>Spawn: returns Process(pid)
    Spawn-->>Cmd: returns Process
    App->>App: work...
    
    App->>Wait: Process::wait()
    Wait->>Kernel: SYS_WAIT4(pid, ...)
    Kernel-->>Wait: returns (pid, status)
    Wait->>Wait: extract exit code from status
    Wait-->>App: returns ExitStatus
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

  • #854: This PR directly implements the core work items of RFC 0009 — adding sys/fs/vibix.rs, sys/process/vibix.rs, sys/time/vibix.rs, and cfg_select! branches for vibix platform support.
  • #851: This PR adds vibix-target std modules and sys implementations that depend on the std fork prepared in that issue.
  • #858: This PR implements vibix-specific std and userspace ABI syscall wrappers that are prerequisites for vibix_libc and pjdfstest integration.

Possibly related PRs

  • dburkart/vibix#862: Directly related — that PR establishes the vibix_abi crate root with core syscall/alloc/errno/stdio modules; this PR extends it with fs and process modules.
  • dburkart/vibix#847: Directly related — that PR merged RFC 0009 which proposes bringing Rust std to vibix; this PR realizes that RFC through userspace ABI and std platform implementations.
  • dburkart/vibix#775: Related — this PR's fs ABI wrappers (fsync, read, write) complement that PR's kernel-side syscall implementations.

Suggested labels

needs-human-review

Poem

🐰 A hop and a skip through vibix land,
With fork and execve, a process so grand,
Files opened wide, directories read,
Syscalls now wired where once none led!
Rust std blooms on a new OS ground,
Platform support found!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 49.40% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly reflects the main changes: implementing sys/fs/vibix.rs and sys/process/vibix.rs modules as the core objective.
Description check ✅ Passed The description comprehensively covers the PR objectives, lists all implemented modules (vibix_abi fs/process, sys/fs/vibix, sys/process/vibix, sys/time/vibix, os/vibix), and documents stubs and test results.
Linked Issues check ✅ Passed The PR implements all core requirements from issue #854: sys/fs/vibix.rs with File/OpenOptions/FileAttr/ReadDir/DirEntry types and syscall operations, sys/process/vibix.rs with Command/Process via fork+execve+wait4, sys/time/vibix.rs with SystemTime, cfg_select branches for vibix, and syscall wrappers in vibix_abi for both fs and process operations.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing Phase 2 of RFC 0009: the PR adds vibix platform support to std fs/process/time/os modules without unrelated modifications to other platforms or subsystems.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch m854-std-fs-process

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🧹 Nitpick comments (1)
library/std/src/sys/process/vibix.rs (1)

271-275: 💤 Low value

Prefer implementing From instead of Into.

Idiomatic Rust prefers implementing From<ExitStatusError> for ExitStatus rather than Into<ExitStatus> for ExitStatusError, as From automatically provides Into.

Suggested change
-impl Into<ExitStatus> for ExitStatusError {
-    fn into(self) -> ExitStatus {
-        ExitStatus(self.0)
+impl From<ExitStatusError> for ExitStatus {
+    fn from(e: ExitStatusError) -> ExitStatus {
+        ExitStatus(e.0)
     }
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@library/std/src/sys/process/vibix.rs` around lines 271 - 275, Replace the
manual Into implementation with the idiomatic From implementation: implement
From<ExitStatusError> for ExitStatus by adding fn from(err: ExitStatusError) ->
ExitStatus { ExitStatus(err.0) } (remove or replace the existing impl
Into<ExitStatus> for ExitStatusError). This ensures From is provided (and Into
is derived) for the types ExitStatusError and ExitStatus.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@library/std/src/sys/fs/vibix.rs`:
- Around line 677-710: The canonicalize function currently pushes raw symlink
targets which can leave "."/".." unresolved, miss nested symlinks, and fail to
detect cycles; modify canonicalize (and its use of lstat/readlink/result) to
fully resolve symlink targets by splitting the target into components and
processing those components immediately (handling Normal, CurDir, ParentDir,
RootDir) instead of a single push, re-checking each resolved component with
lstat to catch nested symlinks, and maintain a loop-detection mechanism (e.g., a
counter or a set of visited (dev,ino) from lstat) to prevent infinite recursion;
ensure absolute targets replace result and relative targets are joined then
normalized as you continue iterating remaining components of the original path.
- Around line 543-559: The readdir function currently opens a directory fd (via
vfs::openat) and may return early when cvt(getdents64(...))? fails, leaking the
fd; fix this by ensuring the fd is always closed on all exit paths—e.g., after
obtaining fd in readdir, create a small RAII guard (e.g., a local CloseDir or
FDGuard with Drop that calls unsafe { vfs::close(fd) }) or manually close the fd
in each error/early-return branch, then move the guard or disable it only when
you intentionally keep the descriptor; update references in readdir around
cvt(unsafe { vfs::getdents64(... ) }) and the existing unsafe { vfs::close(fd) }
so the descriptor is reliably closed on error.
- Around line 408-410: The truncate and seek methods cast u64 offsets to i64
unsafely; add explicit bounds checks in pub fn truncate(&self, size: u64) and
the seek implementation (the method that calls vfs::llseek/vfs::ftruncate) that
return Err(io::Error::new(io::ErrorKind::InvalidInput, "...")) when the
requested size/offset > i64::MAX instead of casting, and only perform the unsafe
vfs::ftruncate/vfs::llseek call after confirming the value fits in i64 to avoid
negative/wrapped values being passed to the kernel.
- Around line 205-224: You're creating a reference to vfs::Dirent64 from a u8
buffer (buf.as_ptr() as *const vfs::Dirent64) which can be misaligned and UB;
instead, copy/read the bytes into an aligned local vfs::Dirent64 value and use
that: validate there's enough bytes for size_of::<vfs::Dirent64>() then use
core::ptr::read_unaligned or memcpy into a local variable (e.g., let dirent =
core::ptr::read_unaligned(buf.as_ptr() as *const vfs::Dirent64) or copy into a
local vfs::Dirent64) and use dirent.d_reclen, leaving the rest of the logic
(name_offset, name_bytes, name_len, OsString::from_vec) unchanged so you never
hold an aligned reference into the u8 buffer.

In `@library/std/src/sys/process/vibix.rs`:
- Around line 130-133: The code currently sets envp to a NULL-only array (envp:
[*const u8; 1] = [core::ptr::null()]) so child processes get no environment;
update vibix.rs to either (A) add a prominent comment/TODO above the envp
declaration and function handling (mentioning Command::env_mut and that
environment passthrough is not implemented) and/or create/link an issue/issue ID
to track implementing full env support, or (B) implement proper env construction
by iterating the process/command environment entries and building a
NULL-terminated array of C strings for envp (ensuring lifetime/storage for the
pointers), replacing the hardcoded envp; reference envp and Command::env_mut in
your change so reviewers can find the spot.
- Around line 334-338: The try_wait implementation currently treats any nonzero
return from vproc::wait4 as meaning this process exited; change it to verify the
returned PID matches self.pid before returning an ExitStatus. Specifically, in
try_wait (which calls vproc::wait4), compare ret (the PID returned by wait4) to
self.pid as i32 and only return Ok(Some(ExitStatus(status))) when they match; if
ret == 0 return Ok(None) as before, and if ret != self.pid return Ok(None) (or
loop/handle accordingly) so you don't return another child's status.
- Around line 140-143: The code silently accepts NUL-containing paths and
ignores chdir failures; change the self.cwd handling to first attempt
CString::new(dir.as_bytes()) and if it returns Err, return an error (do not use
unwrap_or_default), then call unsafe vibix_abi::fs::chdir with the CString
pointer and check its return value — if chdir returns non-zero, convert that
into an Err (e.g., via io::Error::last_os_error() or mapping the returned errno)
and propagate/return it so the caller knows the working-directory change failed
instead of proceeding to execve.

In `@library/std/src/sys/time/vibix.rs`:
- Around line 78-95: The check functions checked_add_duration and
checked_sub_duration cast Duration::as_secs() (u64) to i64 directly which can
overflow; modify both functions to attempt a fallible conversion (e.g.
i64::try_from(other.as_secs()) or similar) and return None if the conversion
fails, then use the converted i64 for arithmetic with tv_sec and the existing
nsec logic (tv_nsec and other.subsec_nanos()) so any durations > i64::MAX
correctly produce None instead of wrapping to negative values.

---

Nitpick comments:
In `@library/std/src/sys/process/vibix.rs`:
- Around line 271-275: Replace the manual Into implementation with the idiomatic
From implementation: implement From<ExitStatusError> for ExitStatus by adding fn
from(err: ExitStatusError) -> ExitStatus { ExitStatus(err.0) } (remove or
replace the existing impl Into<ExitStatus> for ExitStatusError). This ensures
From is provided (and Into is derived) for the types ExitStatusError and
ExitStatus.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6d7b4d00-7c09-47b5-917a-78a88b2839e9

📥 Commits

Reviewing files that changed from the base of the PR and between 145e884 and 2806a2d.

📒 Files selected for processing (12)
  • library/std/src/os/mod.rs
  • library/std/src/os/vibix/ffi.rs
  • library/std/src/os/vibix/mod.rs
  • library/std/src/sys/fs/mod.rs
  • library/std/src/sys/fs/vibix.rs
  • library/std/src/sys/process/mod.rs
  • library/std/src/sys/process/vibix.rs
  • library/std/src/sys/time/mod.rs
  • library/std/src/sys/time/vibix.rs
  • userspace/vibix_abi/src/fs.rs
  • userspace/vibix_abi/src/lib.rs
  • userspace/vibix_abi/src/process.rs

Comment thread library/std/src/sys/fs/vibix.rs
Comment thread library/std/src/sys/fs/vibix.rs
Comment thread library/std/src/sys/fs/vibix.rs
Comment on lines +677 to +710
pub fn canonicalize(path: &Path) -> io::Result<PathBuf> {
// vibix does not have realpath; do a simple absolute-path resolution.
let mut result = if path.is_absolute() {
PathBuf::new()
} else {
crate::env::current_dir()?
};
for component in path.components() {
match component {
crate::path::Component::RootDir => {
result.push("/");
}
crate::path::Component::ParentDir => {
result.pop();
}
crate::path::Component::CurDir => {}
crate::path::Component::Normal(c) => {
result.push(c);
// Check that the path component exists and resolve symlinks.
let meta = lstat(&result)?;
if meta.file_type().is_symlink() {
let target = readlink(&result)?;
result.pop();
if target.is_absolute() {
result = target;
} else {
result.push(target);
}
}
}
crate::path::Component::Prefix(_) => unreachable!(),
}
}
Ok(result)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

This canonicalize implementation can return non-canonical paths.

When a symlink target contains ./.. or multiple path components, the code just pushes the raw target and continues iterating the original path. That leaves relative segments unresolved, misses nested symlinks, and never detects loops.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@library/std/src/sys/fs/vibix.rs` around lines 677 - 710, The canonicalize
function currently pushes raw symlink targets which can leave "."/".."
unresolved, miss nested symlinks, and fail to detect cycles; modify canonicalize
(and its use of lstat/readlink/result) to fully resolve symlink targets by
splitting the target into components and processing those components immediately
(handling Normal, CurDir, ParentDir, RootDir) instead of a single push,
re-checking each resolved component with lstat to catch nested symlinks, and
maintain a loop-detection mechanism (e.g., a counter or a set of visited
(dev,ino) from lstat) to prevent infinite recursion; ensure absolute targets
replace result and relative targets are joined then normalized as you continue
iterating remaining components of the original path.

Comment on lines +130 to +133
// Build environment (inherit current env for now, with modifications).
// For simplicity, pass a minimal envp with just a NULL terminator.
// Full env support would require iterating current env + applying changes.
let envp: [*const u8; 1] = [core::ptr::null()];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Environment variables are not passed to child process.

The envp is hardcoded to [null], meaning child processes receive no environment variables regardless of what was configured via Command::env_mut(). The comment acknowledges this limitation, but it may cause unexpected behavior for programs that depend on environment variables.

Consider documenting this limitation prominently or tracking it for future implementation.

Would you like me to open an issue to track implementing full environment variable passthrough?

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@library/std/src/sys/process/vibix.rs` around lines 130 - 133, The code
currently sets envp to a NULL-only array (envp: [*const u8; 1] =
[core::ptr::null()]) so child processes get no environment; update vibix.rs to
either (A) add a prominent comment/TODO above the envp declaration and function
handling (mentioning Command::env_mut and that environment passthrough is not
implemented) and/or create/link an issue/issue ID to track implementing full env
support, or (B) implement proper env construction by iterating the
process/command environment entries and building a NULL-terminated array of C
strings for envp (ensuring lifetime/storage for the pointers), replacing the
hardcoded envp; reference envp and Command::env_mut in your change so reviewers
can find the spot.

Comment thread library/std/src/sys/process/vibix.rs
Comment thread library/std/src/sys/process/vibix.rs
Comment on lines +20 to +24
pub fn now() -> Instant {
// vibix doesn't have clock_gettime wired yet; panic for now.
// Phase 3 will add CLOCK_MONOTONIC support.
panic!("Instant::now() not yet supported on vibix")
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Don’t expose std::time::*::now() as a panic path.

Once Vibix is routed to this backend, any crate calling Instant::now() or SystemTime::now() aborts at runtime. For public std APIs this is a real behavior break, so this should stay on an unsupported path until a clock source exists.

Also applies to: 56-59

Comment on lines +78 to +95
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
let mut secs = self.tv_sec.checked_add(other.as_secs() as i64)?;
let mut nsec = self.tv_nsec + other.subsec_nanos() as i64;
if nsec >= 1_000_000_000 {
nsec -= 1_000_000_000;
secs = secs.checked_add(1)?;
}
Some(SystemTime { tv_sec: secs, tv_nsec: nsec })
}

pub fn checked_sub_duration(&self, other: &Duration) -> Option<SystemTime> {
let mut secs = self.tv_sec.checked_sub(other.as_secs() as i64)?;
let mut nsec = self.tv_nsec - other.subsec_nanos() as i64;
if nsec < 0 {
nsec += 1_000_000_000;
secs = secs.checked_sub(1)?;
}
Some(SystemTime { tv_sec: secs, tv_nsec: nsec })
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, let's check if the file exists and examine the code
find . -name "vibix.rs" -type f

Repository: dburkart/vibix

Length of output: 319


🏁 Script executed:

# Also search for the specific file path mentioned
ls -la library/std/src/sys/time/vibix.rs 2>/dev/null || echo "File not found at expected path"
git ls-files | grep -i vibix

Repository: dburkart/vibix

Length of output: 1116


🏁 Script executed:

# Search for Duration-related code in the sys/time directory
git ls-files library/std/src/sys/time/ | head -20

Repository: dburkart/vibix

Length of output: 434


🏁 Script executed:

# Read the file to see the actual code at lines 78-95
cat -n library/std/src/sys/time/vibix.rs | sed -n '70,100p'

Repository: dburkart/vibix

Length of output: 1377


🏁 Script executed:

# Also read the entire file to understand the SystemTime struct and context
wc -l library/std/src/sys/time/vibix.rs

Repository: dburkart/vibix

Length of output: 96


🏁 Script executed:

# Let's look at the entire SystemTime struct to understand the types
cat -n library/std/src/sys/time/vibix.rs | head -30

Repository: dburkart/vibix

Length of output: 1223


🏁 Script executed:

# Let's also check if there are any tests for these functions
grep -n "checked_add_duration\|checked_sub_duration" library/std/src/sys/time/vibix.rs

Repository: dburkart/vibix

Length of output: 388


🏁 Script executed:

python3 << 'EOF'
# Verify overflow behavior
i64_max = 9223372036854775807
i64_min = -9223372036854775808

# Example: Duration with seconds > i64::MAX
duration_secs = 10223372036854775808

# What happens when we cast to i64?
# In Rust, casting u64 to i64 is bitwise reinterpretation
# For values > i64::MAX, the high bit (sign bit) is set
# This creates a negative number

# The wrapped value
import ctypes
wrapped = ctypes.c_int64(duration_secs).value
print(f"Duration seconds (u64): {duration_secs}")
print(f"After cast to i64: {wrapped}")
print(f"i64::MAX: {i64_max}")
print(f"i64::MIN: {i64_min}")

# Example scenario
systime_sec = 1000
result = ctypes.c_int64(systime_sec + wrapped).value
print(f"\nExample: SystemTime with tv_sec={systime_sec}")
print(f"Adding Duration with as_secs()={duration_secs}")
print(f"Wrapped cast value: {wrapped}")
print(f"Result of checked_add: {result}")
print(f"Expected behavior: Should return None (overflow)")
EOF

Repository: dburkart/vibix

Length of output: 428


Add bounds check for duration casting to prevent silent overflow.

Lines 79 and 89 cast Duration::as_secs() (u64) directly to i64 without bounds checking. Durations exceeding i64::MAX silently wrap to negative values, causing these checked_* functions to return incorrect timestamps instead of None. For example, a duration of 10223372036854775808 seconds wraps to -8223372036854775808, producing a bogus result from checked_add rather than properly indicating overflow.

Proposed fix
     pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
+        if other.as_secs() > i64::MAX as u64 {
+            return None;
+        }
         let mut secs = self.tv_sec.checked_add(other.as_secs() as i64)?;
         let mut nsec = self.tv_nsec + other.subsec_nanos() as i64;
         if nsec >= 1_000_000_000 {
             nsec -= 1_000_000_000;
             secs = secs.checked_add(1)?;
         }
         Some(SystemTime { tv_sec: secs, tv_nsec: nsec })
     }

     pub fn checked_sub_duration(&self, other: &Duration) -> Option<SystemTime> {
+        if other.as_secs() > i64::MAX as u64 {
+            return None;
+        }
         let mut secs = self.tv_sec.checked_sub(other.as_secs() as i64)?;
         let mut nsec = self.tv_nsec - other.subsec_nanos() as i64;
         if nsec < 0 {
             nsec += 1_000_000_000;
             secs = secs.checked_sub(1)?;
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
let mut secs = self.tv_sec.checked_add(other.as_secs() as i64)?;
let mut nsec = self.tv_nsec + other.subsec_nanos() as i64;
if nsec >= 1_000_000_000 {
nsec -= 1_000_000_000;
secs = secs.checked_add(1)?;
}
Some(SystemTime { tv_sec: secs, tv_nsec: nsec })
}
pub fn checked_sub_duration(&self, other: &Duration) -> Option<SystemTime> {
let mut secs = self.tv_sec.checked_sub(other.as_secs() as i64)?;
let mut nsec = self.tv_nsec - other.subsec_nanos() as i64;
if nsec < 0 {
nsec += 1_000_000_000;
secs = secs.checked_sub(1)?;
}
Some(SystemTime { tv_sec: secs, tv_nsec: nsec })
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
if other.as_secs() > i64::MAX as u64 {
return None;
}
let mut secs = self.tv_sec.checked_add(other.as_secs() as i64)?;
let mut nsec = self.tv_nsec + other.subsec_nanos() as i64;
if nsec >= 1_000_000_000 {
nsec -= 1_000_000_000;
secs = secs.checked_add(1)?;
}
Some(SystemTime { tv_sec: secs, tv_nsec: nsec })
}
pub fn checked_sub_duration(&self, other: &Duration) -> Option<SystemTime> {
if other.as_secs() > i64::MAX as u64 {
return None;
}
let mut secs = self.tv_sec.checked_sub(other.as_secs() as i64)?;
let mut nsec = self.tv_nsec - other.subsec_nanos() as i64;
if nsec < 0 {
nsec += 1_000_000_000;
secs = secs.checked_sub(1)?;
}
Some(SystemTime { tv_sec: secs, tv_nsec: nsec })
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@library/std/src/sys/time/vibix.rs` around lines 78 - 95, The check functions
checked_add_duration and checked_sub_duration cast Duration::as_secs() (u64) to
i64 directly which can overflow; modify both functions to attempt a fallible
conversion (e.g. i64::try_from(other.as_secs()) or similar) and return None if
the conversion fails, then use the converted i64 for arithmetic with tv_sec and
the existing nsec logic (tv_nsec and other.subsec_nanos()) so any durations >
i64::MAX correctly produce None instead of wrapping to negative values.

claude added 2 commits May 5, 2026 06:38
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix critical UB: use read_unaligned for Dirent64 from u8 buffer to
  avoid misaligned reference (the buffer may not be 8-byte aligned)
- Fix readdir fd leak: ensure directory fd is closed even if getdents64
  fails mid-loop
- Fix truncate/seek: add bounds check for u64 > i64::MAX before casting
- Fix try_wait: verify returned pid matches expected child pid
- Fix process chdir: properly handle NUL bytes in path and check chdir
  return value, exit child on failure

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dburkart dburkart merged commit a335c0b into main May 5, 2026
15 checks passed
@dburkart dburkart deleted the m854-std-fs-process branch May 5, 2026 06:44
@coderabbitai coderabbitai Bot mentioned this pull request May 6, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement sys/fs/vibix.rs and sys/process/vibix.rs in the std fork

2 participants