diff --git a/CHANGELOG.md b/CHANGELOG.md index 26e2ea8f4..4710ee2fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - **Perf** When tasks miss the cache, `vp run` no longer waits for the disk after saving each task's result, so the run finishes sooner. If the computer loses power or the operating system crashes during `vp run`, the cache entries that the run saved can be lost, and those tasks run again next time instead of hitting the cache ([#871](https://github.com/voidzero-dev/vite-task/pull/871)). - **Fixed** Running `vp run` in a package that sits inside a workspace folder but isn't listed in its `pnpm-workspace.yaml` `packages` or `package.json` `workspaces` now runs that package's own tasks, instead of treating the enclosing workspace as its root. When both configurations exist, `pnpm-workspace.yaml` takes precedence ([#859](https://github.com/voidzero-dev/vite-task/pull/859)). +- **Fixed** On Linux and macOS, programs using automatic file-access tracking no longer abort when they call `execl`, `execlp`, or `execle` with 32 or more arguments ([#699](https://github.com/voidzero-dev/vite-task/issues/699), [#865](https://github.com/voidzero-dev/vite-task/pull/865)). - **Fixed** A mistyped or misplaced setting in a task now gets an error that names it. For example, `cache: { outputs: [...] }` now fails with an `unknown field` error that names `outputs` and lists the accepted settings, instead of `data did not match any variant of untagged enum UserTaskDefinition` ([vite-plus#2635](https://github.com/voidzero-dev/vite-plus/issues/2635), [#815](https://github.com/voidzero-dev/vite-task/pull/815)). - **Fixed** In cached tasks on Linux and macOS, programs that pass `-1` as a directory descriptor to calls such as `openat` no longer abort. For example, `ps -p ` from procps-ng 4.0.6 or later aborted when the process had already exited ([#862](https://github.com/voidzero-dev/vite-task/pull/862)). - **Fixed** `vp run` no longer panics before running a script that contains non-ASCII characters, such as `echo こんにちは`, and no longer cuts off such commands where it displays them ([#861](https://github.com/voidzero-dev/vite-task/pull/861)). diff --git a/crates/fspy/tests/execl_many_args.rs b/crates/fspy/tests/execl_many_args.rs new file mode 100644 index 000000000..a560649db --- /dev/null +++ b/crates/fspy/tests/execl_many_args.rs @@ -0,0 +1,95 @@ +//! `execl`, `execlp`, and `execle` take a variable number of arguments. The +//! interposer stores them in a stack array for fewer than 32 arguments and a +//! heap allocation otherwise. A call with 32 or more arguments used to write +//! the NULL terminator past the end of that allocation, aborting the program. +//! +//! `exec` replaces the process, so the call runs in a forked child; the parent +//! waits for the child and checks that the program ran successfully. +#![cfg(unix)] + +mod test_utils; + +use std::{ffi::CString, io::Error}; + +use test_log::test; + +#[test(tokio::test)] +async fn execl_with_many_arguments() -> anyhow::Result<()> { + let program = which::which("true")?; + track_fn!(program.to_str().unwrap().to_owned(), |program: String| { + let program = CString::new(program).unwrap(); + let args: Vec = (0..40) + .map( + |i| if i == 0 { program.clone() } else { CString::new(format!("arg{i}")).unwrap() }, + ) + .collect(); + + // SAFETY: the tracked test process is single-threaded here, and the + // child and parent only call async-signal-safe functions below. + let pid = unsafe { libc::fork() }; + assert!(pid >= 0, "fork failed: {}", Error::last_os_error()); + + if pid == 0 { + // SAFETY: every pointer is a valid NUL-terminated C string and the + // argument list is NULL-terminated. + unsafe { + libc::execl( + program.as_ptr(), + args[0].as_ptr(), + args[1].as_ptr(), + args[2].as_ptr(), + args[3].as_ptr(), + args[4].as_ptr(), + args[5].as_ptr(), + args[6].as_ptr(), + args[7].as_ptr(), + args[8].as_ptr(), + args[9].as_ptr(), + args[10].as_ptr(), + args[11].as_ptr(), + args[12].as_ptr(), + args[13].as_ptr(), + args[14].as_ptr(), + args[15].as_ptr(), + args[16].as_ptr(), + args[17].as_ptr(), + args[18].as_ptr(), + args[19].as_ptr(), + args[20].as_ptr(), + args[21].as_ptr(), + args[22].as_ptr(), + args[23].as_ptr(), + args[24].as_ptr(), + args[25].as_ptr(), + args[26].as_ptr(), + args[27].as_ptr(), + args[28].as_ptr(), + args[29].as_ptr(), + args[30].as_ptr(), + args[31].as_ptr(), + args[32].as_ptr(), + args[33].as_ptr(), + args[34].as_ptr(), + args[35].as_ptr(), + args[36].as_ptr(), + args[37].as_ptr(), + args[38].as_ptr(), + args[39].as_ptr(), + core::ptr::null::(), + ); + libc::_exit(127); + } + } + + let mut status = 0; + // SAFETY: `pid` is the child process created above, and `status` is + // writable storage for the wait status. + let waited = unsafe { libc::waitpid(pid, &raw mut status, 0) }; + assert_eq!(waited, pid, "waitpid failed: {}", Error::last_os_error()); + assert!(libc::WIFEXITED(status), "child did not exit normally: {status}"); + assert_eq!(libc::WEXITSTATUS(status), 0); + }) + .await?; + + Ok(()) +} diff --git a/crates/fspy/tests/test_utils/mod.rs b/crates/fspy/tests/test_utils/mod.rs index 4ffa8f6d1..f47bb6837 100644 --- a/crates/fspy/tests/test_utils/mod.rs +++ b/crates/fspy/tests/test_utils/mod.rs @@ -23,6 +23,11 @@ pub use vt_subprocess_test::command_for_fn; /// # Panics /// /// Panics if the path was not accessed, or was accessed in another way. +#[expect( + clippy::allow_attributes, + reason = "allow attribute required for conditionally-used helper" +)] +#[allow(dead_code, reason = "not all test files use this helper")] #[track_caller] pub fn assert_contains<'a>( accesses: &'a PathAccessIterable, diff --git a/crates/fspy_preload_unix/src/interceptions/spawn/exec/with_argv.rs b/crates/fspy_preload_unix/src/interceptions/spawn/exec/with_argv.rs index c3aba13ea..8f1971a95 100644 --- a/crates/fspy_preload_unix/src/interceptions/spawn/exec/with_argv.rs +++ b/crates/fspy_preload_unix/src/interceptions/spawn/exec/with_argv.rs @@ -31,14 +31,14 @@ pub unsafe fn with_argv( stack.as_mut_slice() } else if argc < 4096 { // TODO: Use ARG_MAX, not this hardcoded constant - // SAFETY: requesting a heap allocation of the correct size for argc pointers - let ptr = unsafe { libc::malloc(argc * mem::size_of::<*const c_char>()) }; + // SAFETY: requesting a heap allocation of the correct size for argc + 1 pointers, including the NULL terminator + let ptr = unsafe { libc::malloc((argc + 1) * mem::size_of::<*const c_char>()) }; if ptr.is_null() { Error::ENOMEM.set(); return -1; } - // SAFETY: ptr is non-null (checked above), properly aligned, and points to argc elements worth of allocated memory - unsafe { slice::from_raw_parts_mut(ptr.cast::>(), argc) } + // SAFETY: ptr is non-null (checked above), properly aligned, and points to argc + 1 elements worth of allocated memory + unsafe { slice::from_raw_parts_mut(ptr.cast::>(), argc + 1) } } else { Error::E2BIG.set(); return -1;