Add userspace file utilities (stat, cp, mv, rm, mkdir, rmdir, touch) and /bin/uname#913
Merged
Add userspace file utilities (stat, cp, mv, rm, mkdir, rmdir, touch) and /bin/uname#913
Conversation
Add eight new base system programs following the cat/ls pattern from PR #909: stat, cp, mv, rm, mkdir, rmdir, touch, and uname. Each is a standalone crate in base/<name>/ with its own syscalls.rs shim and builds via the build_userspace_std_bin() helper. All binaries are wired into the ext2 rootfs across all four extras assembly sites (run ext2, run default, smoke, sh_test). Closes #904 Closes #905 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds eight standalone Rust userspace utilities (stat, cp, mv, rm, mkdir, rmdir, touch, uname) each under base// with Cargo manifests, implementations, and small C‑ABI syscall shims; xtask is updated to build and install these binaries into the ext2 rootfs at /bin/. ChangesUserspace File Utilities
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
base/rm/src/syscalls.rs (1)
1-50: 💤 Low valueConsider extracting duplicated syscall shims to a shared crate.
This file is identical to
syscalls.rsin touch, mv, rmdir, mkdir, cp, stat, and uname. While duplication may be intentional for standalone-Z build-stdbuilds, a sharedvibix_syscallscrate could reduce maintenance burden if future syscalls are added.🤖 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 `@base/rm/src/syscalls.rs` around lines 1 - 50, Multiple binaries duplicate the same syscall shim implementation (SYS_CLOSE, __errno_location, raw1, cvt, close); extract this common logic into a shared crate (e.g., vibix_syscalls) and have each binary depend on it. Move constants and unsafe functions (SYS_CLOSE, __errno_location extern, raw1, cvt) and the no_mangle close wrapper into the new crate, make the functions pub(crate) or pub as needed, update callers to import close (or call via the crate) and remove the duplicated file from each repo target; ensure visibility and ABI (extern "C" no_mangle) are preserved so -Z build-std builds keep the same symbols.xtask/src/main.rs (1)
1784-1842: ⚡ Quick winExtract the base
/bin/*build/install list into one helper to avoid drift.The same utility build + extras mapping is now duplicated across multiple call sites. Centralizing it will prevent “added in one path, missing in another” regressions on future utility additions.
♻️ Refactor sketch
+fn build_base_utils_extras() -> R<Vec<(PathBuf, &'static str)>> { + Ok(vec![ + (build_userspace_sh()?, "/bin/sh"), + (build_userspace_cat()?, "/bin/cat"), + (build_userspace_ls()?, "/bin/ls"), + (build_userspace_stat()?, "/bin/stat"), + (build_userspace_cp()?, "/bin/cp"), + (build_userspace_mv()?, "/bin/mv"), + (build_userspace_rm()?, "/bin/rm"), + (build_userspace_mkdir()?, "/bin/mkdir"), + (build_userspace_rmdir()?, "/bin/rmdir"), + (build_userspace_touch()?, "/bin/touch"), + (build_userspace_uname()?, "/bin/uname"), + ]) +} ... - let sh_bin = build_userspace_sh()?; - ... - let extras: Vec<(&Path, &str)> = vec![ ... ]; + let built = build_base_utils_extras()?; + let extras: Vec<(&Path, &str)> = + built.iter().map(|(p, dst)| (p.as_path(), *dst)).collect();🤖 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 `@xtask/src/main.rs` around lines 1784 - 1842, The duplicated build-and-extras mapping (calls to build_userspace_init/sh/cat/ls/stat/cp/mv/rm/mkdir/rmdir/touch/uname and the extras Vec used with ext2_image::build_with_extras or other branches) should be extracted into a single helper (e.g., a function like collect_userspace_binaries or build_userspace_extras) that returns the built Paths and the Vec<(&Path, &str)> extras; replace the repeated blocks that call build_userspace_* and construct extras in both the Some("ext2") branch and the None branch with calls to this helper and pass its results into ext2_image::build_with_extras(&workspace_root(), Some(&init_bin), &extras, true) or the corresponding call sites, ensuring init_bin is included/returned by the helper if needed.
🤖 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 `@base/mkdir/src/syscalls.rs`:
- Around line 19-30: The inline asm blocks that perform the "syscall" (the
assembly snippet using asm!("syscall", inlateout("rax") nr => ret, ...
options(nostack, preserves_flags),) ) incorrectly declare preserves_flags;
remove preserves_flags from the options list in that asm invocation in
base/mkdir/src/syscalls.rs (and the same syscall asm in the other listed
base/*/src/syscalls.rs files) so the options are just options(nostack) to
reflect that syscall clobbers RFLAGS.
In `@base/stat/src/main.rs`:
- Around line 10-31: The current stat_path function prints only size/type and
hardcodes inode/links as "unknown"; update stat_path to extract and print the
required v1 fields by importing std::os::unix::fs::MetadataExt and using the
MetadataExt methods on meta (e.g., meta.ino(), meta.nlink(), meta.mode(),
meta.atime(), meta.atime_nsec(), meta.mtime(), meta.mtime_nsec()) to populate
inode, nlink, mode (format mode in octal), and timestamps (combine seconds +
nanoseconds into a readable timestamp or RFC3339-like string), keeping existing
size (meta.len()) and file_type logic; replace the hardcoded "Inode: unknown"
and "Links: unknown" lines with the real values and add lines for "Mode:" and
the access/modification times derived from meta.atoi()/mtime methods so the
function meets the v1 contract.
In `@base/uname/src/main.rs`:
- Around line 10-18: The current argument handling (using env::args() -> args
and the `all` boolean) accepts any extra args; change it to only accept either
no operands or exactly one operand equal to "-a". Validate argv shape by
checking args.len(): allow len == 1 (no args) or len == 2 with args[1] == "-a";
for any other case write an error to stderr (use eprintln!) and exit with a
non-zero code (std::process::exit(1)). Keep the existing output behavior
(println! "vibix..." when "-a", otherwise "vibix") and remove the permissive
any(|a| a == "-a") logic.
---
Nitpick comments:
In `@base/rm/src/syscalls.rs`:
- Around line 1-50: Multiple binaries duplicate the same syscall shim
implementation (SYS_CLOSE, __errno_location, raw1, cvt, close); extract this
common logic into a shared crate (e.g., vibix_syscalls) and have each binary
depend on it. Move constants and unsafe functions (SYS_CLOSE, __errno_location
extern, raw1, cvt) and the no_mangle close wrapper into the new crate, make the
functions pub(crate) or pub as needed, update callers to import close (or call
via the crate) and remove the duplicated file from each repo target; ensure
visibility and ABI (extern "C" no_mangle) are preserved so -Z build-std builds
keep the same symbols.
In `@xtask/src/main.rs`:
- Around line 1784-1842: The duplicated build-and-extras mapping (calls to
build_userspace_init/sh/cat/ls/stat/cp/mv/rm/mkdir/rmdir/touch/uname and the
extras Vec used with ext2_image::build_with_extras or other branches) should be
extracted into a single helper (e.g., a function like collect_userspace_binaries
or build_userspace_extras) that returns the built Paths and the Vec<(&Path,
&str)> extras; replace the repeated blocks that call build_userspace_* and
construct extras in both the Some("ext2") branch and the None branch with calls
to this helper and pass its results into
ext2_image::build_with_extras(&workspace_root(), Some(&init_bin), &extras, true)
or the corresponding call sites, ensuring init_bin is included/returned by the
helper if needed.
🪄 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: 0979b03d-009a-444d-a31d-f17b8eb6e60a
📒 Files selected for processing (25)
base/cp/Cargo.tomlbase/cp/src/main.rsbase/cp/src/syscalls.rsbase/mkdir/Cargo.tomlbase/mkdir/src/main.rsbase/mkdir/src/syscalls.rsbase/mv/Cargo.tomlbase/mv/src/main.rsbase/mv/src/syscalls.rsbase/rm/Cargo.tomlbase/rm/src/main.rsbase/rm/src/syscalls.rsbase/rmdir/Cargo.tomlbase/rmdir/src/main.rsbase/rmdir/src/syscalls.rsbase/stat/Cargo.tomlbase/stat/src/main.rsbase/stat/src/syscalls.rsbase/touch/Cargo.tomlbase/touch/src/main.rsbase/touch/src/syscalls.rsbase/uname/Cargo.tomlbase/uname/src/main.rsbase/uname/src/syscalls.rsxtask/src/main.rs
Comment on lines
+19
to
+30
| asm!( | ||
| "syscall", | ||
| inlateout("rax") nr => ret, | ||
| inlateout("rdi") a0 => _, | ||
| lateout("rcx") _, | ||
| lateout("r11") _, | ||
| lateout("rdx") _, | ||
| lateout("rsi") _, | ||
| lateout("r8") _, | ||
| lateout("r9") _, | ||
| lateout("r10") _, | ||
| options(nostack, preserves_flags), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Rust x86_64 inline assembly, does the syscallinstruction preserve flags, and isoptions(preserves_flags)valid aroundasm!("syscall", ...)?
💡 Result:
No, the syscall instruction does not preserve all flags: it saves RFLAGS to R11 but then modifies RFLAGS by masking bits according to the IA32_FMASK MSR (e.g., clearing IF, TF, DF, AC, IOPL, NT in Linux).[1][2] On return via SYSRET, RFLAGS is restored from R11 (with RF and VM cleared).[3] Thus, the asm block as a whole preserves the relevant status flags (CF, PF, AF, ZF, SF, OF in EFLAGS).[4][5] However, options(preserves_flags) is not valid around asm!("syscall", ...) by itself. Rust requires that if preserves_flags is specified, the asm code must restore the covered flags upon exit (e.g., by explicitly saving/restoring them).[4][5] Since syscall alone does not restore flags (SYSRET does, but that's in kernel code outside the asm block), using preserves_flags without explicit flag save/restore in user-space asm would be incorrect and is typically omitted in examples.[6][7][8] Standard syscall asm uses options(nostack) instead, explicitly clobbering rcx/r11/rax.[9][7][8]
Citations:
- 1: http://www.felixcloutier.com/x86/SYSCALL.html
- 2: https://kernel-internals.org/arch/x86/syscall-entry/
- 3: http://www.felixcloutier.com/x86/SYSRET.html
- 4: https://doc.rust-lang.org/1.59.0/reference/inline-assembly.html
- 5: https://rust-lang.github.io/rfcs/2873-inline-asm.html
- 6: https://fluxsec.red/implementing-syscall-hooking-rust
- 7: https://blog.rust-lang.org/inside-rust/2020/06/08/new-inline-asm/
- 8: https://shadow.github.io/docs/rust/src/linux_syscall/arch/x86_64/syscall_asm.rs.html
- 9: asm: Are input registers guaranteed to be written in left-to-right order? rust-lang/reference#1353
🏁 Script executed:
#!/bin/bash
# Find all syscalls.rs files and check for preserves_flags usage
find . -name "syscalls.rs" -type f | head -20Repository: dburkart/vibix
Length of output: 389
🏁 Script executed:
#!/bin/bash
# Check the specific file and location mentioned in the review
if [ -f "base/mkdir/src/syscalls.rs" ]; then
echo "=== base/mkdir/src/syscalls.rs lines 19-30 ==="
sed -n '19,30p' "base/mkdir/src/syscalls.rs"
fiRepository: dburkart/vibix
Length of output: 475
🏁 Script executed:
#!/bin/bash
# Search for all occurrences of preserves_flags in syscall blocks
rg "preserves_flags" --type rust -B 3 -A 1Repository: dburkart/vibix
Length of output: 50370
🏁 Script executed:
#!/bin/bash
# Find all base/*/src/syscalls.rs files that contain preserves_flags with syscall
for file in base/*/src/syscalls.rs; do
if grep -q "preserves_flags" "$file"; then
echo "=== $file ==="
grep -c "preserves_flags" "$file"
fi
doneRepository: dburkart/vibix
Length of output: 447
🏁 Script executed:
#!/bin/bash
# Show one full syscall block from a sibling to confirm same pattern
echo "=== Example from base/sh/src/syscalls.rs ==="
head -35 base/sh/src/syscalls.rs | tail -15Repository: dburkart/vibix
Length of output: 512
🏁 Script executed:
#!/bin/bash
# Verify one of the sibling files has the same issue
echo "=== base/cat/src/syscalls.rs syscall block with preserves_flags ==="
grep -B 10 -A 2 "preserves_flags" base/cat/src/syscalls.rs | head -15Repository: dburkart/vibix
Length of output: 499
🏁 Script executed:
#!/bin/bash
# Count total syscall blocks with preserves_flags across all base utils
total=0
for file in base/*/src/syscalls.rs; do
count=$(grep -c "preserves_flags" "$file")
if [ "$count" -gt 0 ]; then
total=$((total + count))
fi
done
echo "Total preserves_flags occurrences in base/*/src/syscalls.rs: $total"Repository: dburkart/vibix
Length of output: 122
Drop preserves_flags from all syscall blocks in base utility syscalls.rs files.
The syscall instruction on x86_64 clobbers RFLAGS, but these asm blocks declare options(nostack, preserves_flags), misleading LLVM into assuming the condition flags survive. This can cause miscompilation around the wrapper functions. Remove preserves_flags from all 11 affected files: base/{cat, cp, ls, mkdir, mv, rm, rmdir, sh, stat, touch, uname}/src/syscalls.rs.
Example fix for base/mkdir/src/syscalls.rs
asm!(
"syscall",
inlateout("rax") nr => ret,
inlateout("rdi") a0 => _,
lateout("rcx") _,
lateout("r11") _,
lateout("rdx") _,
lateout("rsi") _,
lateout("r8") _,
lateout("r9") _,
lateout("r10") _,
- options(nostack, preserves_flags),
+ options(nostack),
);📝 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
| asm!( | |
| "syscall", | |
| inlateout("rax") nr => ret, | |
| inlateout("rdi") a0 => _, | |
| lateout("rcx") _, | |
| lateout("r11") _, | |
| lateout("rdx") _, | |
| lateout("rsi") _, | |
| lateout("r8") _, | |
| lateout("r9") _, | |
| lateout("r10") _, | |
| options(nostack, preserves_flags), | |
| asm!( | |
| "syscall", | |
| inlateout("rax") nr => ret, | |
| inlateout("rdi") a0 => _, | |
| lateout("rcx") _, | |
| lateout("r11") _, | |
| lateout("rdx") _, | |
| lateout("rsi") _, | |
| lateout("r8") _, | |
| lateout("r9") _, | |
| lateout("r10") _, | |
| options(nostack), | |
| ); |
🤖 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 `@base/mkdir/src/syscalls.rs` around lines 19 - 30, The inline asm blocks that
perform the "syscall" (the assembly snippet using asm!("syscall",
inlateout("rax") nr => ret, ... options(nostack, preserves_flags),) )
incorrectly declare preserves_flags; remove preserves_flags from the options
list in that asm invocation in base/mkdir/src/syscalls.rs (and the same syscall
asm in the other listed base/*/src/syscalls.rs files) so the options are just
options(nostack) to reflect that syscall clobbers RFLAGS.
Comment on lines
+10
to
+31
| fn stat_path(path: &str) -> u8 { | ||
| let meta = match fs::symlink_metadata(path) { | ||
| Ok(m) => m, | ||
| Err(e) => { | ||
| eprintln!("stat: {path}: {e}"); | ||
| return 1; | ||
| } | ||
| }; | ||
|
|
||
| let file_type = if meta.is_dir() { | ||
| "directory" | ||
| } else if meta.is_symlink() { | ||
| "symbolic link" | ||
| } else { | ||
| "regular file" | ||
| }; | ||
|
|
||
| println!(" File: {path}"); | ||
| println!(" Size: {}", meta.len()); | ||
| println!(" Type: {file_type}"); | ||
| println!("Inode: unknown"); | ||
| println!("Links: unknown"); |
There was a problem hiding this comment.
stat still misses the required metadata fields.
The v1 contract calls for size, mode, inode, nlink, and timestamps. Right now this only prints size/type, hardcodes inode and links as unknown, and omits mode/timestamps entirely, so callers still cannot use this as the promised stat replacement.
🤖 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 `@base/stat/src/main.rs` around lines 10 - 31, The current stat_path function
prints only size/type and hardcodes inode/links as "unknown"; update stat_path
to extract and print the required v1 fields by importing
std::os::unix::fs::MetadataExt and using the MetadataExt methods on meta (e.g.,
meta.ino(), meta.nlink(), meta.mode(), meta.atime(), meta.atime_nsec(),
meta.mtime(), meta.mtime_nsec()) to populate inode, nlink, mode (format mode in
octal), and timestamps (combine seconds + nanoseconds into a readable timestamp
or RFC3339-like string), keeping existing size (meta.len()) and file_type logic;
replace the hardcoded "Inode: unknown" and "Links: unknown" lines with the real
values and add lines for "Mode:" and the access/modification times derived from
meta.atoi()/mtime methods so the function meets the v1 contract.
- stat: use MetadataExt to print mode (octal), inode, and nlink instead of hardcoded "unknown" values - uname: validate arguments; reject unsupported flags with usage error - syscalls.rs: drop preserves_flags from all new crates since the syscall instruction clobbers RFLAGS Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
base/stat/src/main.rs (1)
28-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
statstill omits the required timestamps.Lines 28-33 now cover size, type, mode, inode, and link count, but the v1 contract for this utility still includes timestamps. Without at least access/modify times, this is not yet the promised
statreplacement from issue#904.Suggested minimal follow-up
println!(" Size: {}", meta.len()); println!(" Type: {file_type}"); println!(" Mode: {:o}", meta.mode()); println!(" Inode: {}", meta.ino()); println!(" Links: {}", meta.nlink()); + println!("Access: {}.{:09}", meta.atime(), meta.atime_nsec()); + println!("Modify: {}.{:09}", meta.mtime(), meta.mtime_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 `@base/stat/src/main.rs` around lines 28 - 33, The output is missing required timestamps (access/modify) from the v1 stat contract; update the printing block that uses the Metadata instance named meta (the same meta used in println!(" Size: {}", meta.len()) etc.) to also fetch meta.accessed() and meta.modified(), convert those SystemTime values to a readable string (or fallback to a placeholder on Err), and include two new println lines (e.g., "Access: ..." and "Modify: ...") alongside the existing File/Size/Type/Mode/Inode/Links prints so the tool emits access and modify timestamps.
🤖 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 `@base/cp/src/syscalls.rs`:
- Around line 15-49: Duplicate and diverging syscall shim (raw1, cvt, close)
across crates must be centralized: extract unsafe fn raw1, unsafe fn cvt, and
the extern "C" close shim (and any SYS_* and __errno_location uses) into a
single shared module or small helper crate and update all utilities to import
that module (or have the build system inject the same file) so there's a single
authoritative implementation; when moving, standardize the inline asm options to
include preserves_flags (use options(nostack, preserves_flags)) and keep the ABI
and symbol names (raw1, cvt, close, SYS_CLOSE, __errno_location) unchanged so
callers stay compatible.
---
Duplicate comments:
In `@base/stat/src/main.rs`:
- Around line 28-33: The output is missing required timestamps (access/modify)
from the v1 stat contract; update the printing block that uses the Metadata
instance named meta (the same meta used in println!(" Size: {}", meta.len())
etc.) to also fetch meta.accessed() and meta.modified(), convert those
SystemTime values to a readable string (or fallback to a placeholder on Err),
and include two new println lines (e.g., "Access: ..." and "Modify: ...")
alongside the existing File/Size/Type/Mode/Inode/Links prints so the tool emits
access and modify timestamps.
🪄 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: e9d53fe8-a836-4343-b463-8c9b03d02ddc
📒 Files selected for processing (10)
base/cp/src/syscalls.rsbase/mkdir/src/syscalls.rsbase/mv/src/syscalls.rsbase/rm/src/syscalls.rsbase/rmdir/src/syscalls.rsbase/stat/src/main.rsbase/stat/src/syscalls.rsbase/touch/src/syscalls.rsbase/uname/src/main.rsbase/uname/src/syscalls.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- base/stat/src/syscalls.rs
Comment on lines
+15
to
+49
| #[inline(always)] | ||
| unsafe fn raw1(nr: u64, a0: u64) -> i64 { | ||
| let ret: i64; | ||
| unsafe { | ||
| asm!( | ||
| "syscall", | ||
| inlateout("rax") nr => ret, | ||
| inlateout("rdi") a0 => _, | ||
| lateout("rcx") _, | ||
| lateout("r11") _, | ||
| lateout("rdx") _, | ||
| lateout("rsi") _, | ||
| lateout("r8") _, | ||
| lateout("r9") _, | ||
| lateout("r10") _, | ||
| options(nostack), | ||
| ); | ||
| } | ||
| ret | ||
| } | ||
|
|
||
| /// Convert raw syscall return to C convention: on error set errno, return -1. | ||
| #[inline] | ||
| unsafe fn cvt(r: i64) -> i64 { | ||
| if r < 0 { | ||
| unsafe { *__errno_location() = (-r) as i32 }; | ||
| -1 | ||
| } else { | ||
| r | ||
| } | ||
| } | ||
|
|
||
| #[no_mangle] | ||
| pub unsafe extern "C" fn close(fd: i32) -> i32 { | ||
| unsafe { cvt(raw1(SYS_CLOSE, fd as u64)) as i32 } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how many duplicated syscall shims exist and whether they are identical.
fd -a 'syscalls\.rs$' base | sort | while read -r file; do
printf '%s %s\n' "$(sha256sum "$file" | cut -d' ' -f1)" "$file"
done | sort
printf '\n=== raw1 / close definitions across base/*/src/syscalls.rs ===\n'
rg -n -C2 'unsafe fn raw1|pub unsafe extern "C" fn close' base --glob '*/src/syscalls.rs'Repository: dburkart/vibix
Length of output: 1178
🏁 Script executed:
# First, let's look at the structure of a few syscalls.rs files
echo "=== Content of base/cp/src/syscalls.rs ==="
head -60 base/cp/src/syscalls.rs
echo -e "\n=== Content of base/mkdir/src/syscalls.rs ==="
head -60 base/mkdir/src/syscalls.rs
echo -e "\n=== Content of base/cat/src/syscalls.rs ==="
head -60 base/cat/src/syscalls.rsRepository: dburkart/vibix
Length of output: 3858
🏁 Script executed:
# Extract the raw1 function's options() call from all syscalls.rs files to check for variations
echo "=== Checking asm! options across all base/*/src/syscalls.rs ==="
for file in base/*/src/syscalls.rs; do
echo "--- $file ---"
grep -A 8 'asm!' "$file" | grep 'options('
done
# Also count total lines in each file to see if they differ in content
echo -e "\n=== File line counts ==="
wc -l base/*/src/syscalls.rs | sort -k2Repository: dburkart/vibix
Length of output: 872
🏁 Script executed:
# Extract the asm! block with options() more carefully
echo "=== Comparing asm! options() calls ==="
for file in base/*/src/syscalls.rs; do
echo "--- $file ---"
sed -n '19,29p' "$file" | grep -E '(asm!|options)'
done
# Let's also see the full asm! block for each
echo -e "\n=== Full asm! blocks (lines 19-29) ==="
for file in base/*/src/syscalls.rs; do
echo "--- $file ---"
sed -n '19,29p' "$file"
doneRepository: dburkart/vibix
Length of output: 4536
🏁 Script executed:
# Look further down to find the options() lines
echo "=== Looking for options() lines in asm! blocks ==="
for file in base/*/src/syscalls.rs; do
echo "--- $file ---"
sed -n '27,32p' "$file" | grep -E 'options|^[[:space:]]*\);'
done
# Also show wider context to catch the exact lines
echo -e "\n=== Exact options() lines with context ==="
rg 'options\(' base --glob '*/src/syscalls.rs' -B1 -A1Repository: dburkart/vibix
Length of output: 964
Consolidate duplicated syscall shims—inconsistencies have already emerged across copies.
The raw1/cvt/close shim is now duplicated across 10 crates. More critically, these copies have already diverged: cat and ls use options(nostack, preserves_flags) in their asm! blocks, while cp, mkdir, mv, rm, rmdir, stat, touch, and uname use only options(nostack). This inconsistency is exactly the maintainability risk this structure creates—fixes and updates land unevenly and go unnoticed.
Move this shim into a single shared module or helper crate, or have the build system inject it consistently, so ABI-sensitive code like syscall assembly stays synchronized across all utilities.
🤖 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 `@base/cp/src/syscalls.rs` around lines 15 - 49, Duplicate and diverging
syscall shim (raw1, cvt, close) across crates must be centralized: extract
unsafe fn raw1, unsafe fn cvt, and the extern "C" close shim (and any SYS_* and
__errno_location uses) into a single shared module or small helper crate and
update all utilities to import that module (or have the build system inject the
same file) so there's a single authoritative implementation; when moving,
standardize the inline asm options to include preserves_flags (use
options(nostack, preserves_flags)) and keep the ABI and symbol names (raw1, cvt,
close, SYS_CLOSE, __errno_location) unchanged so callers stay compatible.
Owner
Author
|
The syscall shim consolidation is a good follow-up but out of scope for this PR -- it would touch pre-existing crates (cat, ls, sh) and change the build architecture. Deferred to future work. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #904
Closes #905
Summary
stat,cp,mv,rm,mkdir,rmdir,touch, andunamebase/<name>/with its ownsyscalls.rsshim, built via thebuild_userspace_std_bin()helperUtilities
statfs::symlink_metadata()cpcp <src> <dst>)fs::copy()mvmv <src> <dst>)fs::rename()rmrm <path>...)fs::remove_file()mkdirmkdir <path>...)fs::create_dir()rmdirrmdir <path>...)fs::remove_dir()touchOpenOptions::new().create(true).write(true).open()uname-afor fullTest plan
cargo xtask build— passescargo xtask smoke— all 43 markers presentcargo fmt --all -- --check— cleancargo xtask ext2-image— hash updated and verified