argv[0] in WASIX subprocesses should resolve to the atom rather than the full command#6516
Draft
argv[0] in WASIX subprocesses should resolve to the atom rather than the full command#6516
Conversation
…than the full command
Contributor
There was a problem hiding this comment.
Pull request overview
Updates WASIX command execution so that argv[0] in spawned processes resolves to the underlying atom (via a canonical atom VFS path) instead of the command wrapper, preventing command metadata (notably main_args) from being re-applied on re-exec.
Changes:
- Add a new WASM test that re-execs via
argv[0]and assertsmain_argsare not re-injected. - Inject per-command atoms into a dedicated
/bin/.__atoms/...path and prefer that path forargv[0]/exec_name. - Add
BinaryPackageCommand::atom_vfs_path()to compute the canonical atom VFS path.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/wasix/tests/wasm_tests/proc_exec_command_argv0/main.c | New C harness that re-execs via argv[0] and validates arg behavior. |
| lib/wasix/tests/wasm_tests/proc_exec_command_argv0/build.sh | Build script for the new WASM test binary. |
| lib/wasix/tests/wasm_tests/proc_exec_command_argv0.rs | New Rust integration test building a package with distinct command/atom names and main_args. |
| lib/wasix/tests/wasm_tests/mod.rs | Registers the new test module. |
| lib/wasix/src/state/env.rs | Injects atoms under /bin/.__atoms/... and prefers the atom VFS path for argv[0]. |
| lib/wasix/src/runners/wasi.rs | Prefers atom VFS path for exec name when no explicit annotation exec_name is set. |
| lib/wasix/src/bin_factory/binary_package.rs | Adds atom_vfs_path() helper for canonical atom VFS path derivation. |
Comment on lines
+1390
to
+1395
| // Prefer an explicit exec_name from the annotation; otherwise use | ||
| // the atom's canonical VFS path (e.g. | ||
| // "/bin/.__atoms/wasmer/php/php") so that re-exec via argv[0] finds | ||
| // the raw wasm without triggering prepare_spawn again. | ||
| let argv0 = exec_name.or_else(|| cmd.atom_vfs_path()); | ||
| if let Some(argv0) = argv0 { |
There was a problem hiding this comment.
Overwriting argv[0] with cmd.atom_vfs_path() assumes the atom has been injected at that path. If atom injection is best-effort (or can fail), consider falling back to the previous exec_name/command name behavior unless the atom path is known to exist, to avoid breaking programs that re-exec themselves via argv[0].
Suggested change
| // Prefer an explicit exec_name from the annotation; otherwise use | |
| // the atom's canonical VFS path (e.g. | |
| // "/bin/.__atoms/wasmer/php/php") so that re-exec via argv[0] finds | |
| // the raw wasm without triggering prepare_spawn again. | |
| let argv0 = exec_name.or_else(|| cmd.atom_vfs_path()); | |
| if let Some(argv0) = argv0 { | |
| // Only replace argv[0] when the package explicitly provides an | |
| // exec_name. Falling back to the atom VFS path here is unsafe | |
| // unless injection at that path is known to have succeeded, and a | |
| // missing path can break programs that re-exec themselves via | |
| // argv[0]. | |
| if let Some(argv0) = exec_name { |
Co-authored-by: Copilot <copilot@github.com>
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
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.
No description provided.