fix: use PathBuf instead of String for guest binary and crash dump paths#1652
fix: use PathBuf instead of String for guest binary and crash dump paths#1652midsterx wants to merge 1 commit into
Conversation
Signed-off-by: Midhush Karthic <mimosk25@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes filesystem path handling in Hyperlight’s host/guest loading and crash dump code by replacing String-backed paths with PathBuf, improving correctness for non-UTF8 paths and reducing lossy conversions at API boundaries.
Changes:
- Update
GuestBinary::FilePath(and related callers) to usePathBufrather thanString. - Update crashdump plumbing to carry
PathBufthrough the API (generate_crashdump_to_dir,HYPERLIGHT_CORE_DUMP_DIR, file path formatting). - Update tests, examples, benches, and fuzz targets to pass
PathBufpaths; add a Unix-only regression test for loading from a non-UTF8 filename.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/hyperlight_testing/src/lib.rs | Add *_as_pathbuf helpers; keep *_as_string wrappers for compatibility. |
| src/hyperlight_host/tests/wit_test.rs | Switch test guest path acquisition to PathBuf. |
| src/hyperlight_host/tests/snapshot_goldens/fixtures.rs | Use PathBuf for fixture guest binary paths. |
| src/hyperlight_host/tests/sandbox_host_tests.rs | Update sandbox construction to pass PathBuf guest paths. |
| src/hyperlight_host/tests/common/mod.rs | Return PathBuf guest paths for shared test helpers. |
| src/hyperlight_host/src/sandbox/uninitialized.rs | Change GuestBinary::FilePath to PathBuf; update canonicalization and add non-UTF8 path test. |
| src/hyperlight_host/src/sandbox/uninitialized_evolve.rs | Update evolve tests to use PathBuf guest paths. |
| src/hyperlight_host/src/sandbox/snapshot/mod.rs | Pass PathBuf into ExeInfo::from_file during snapshot creation. |
| src/hyperlight_host/src/sandbox/snapshot/file_tests.rs | Update snapshot tests and crashdump directory calls to PathBuf/&Path. |
| src/hyperlight_host/src/sandbox/outb.rs | Update outb tests to use PathBuf guest paths. |
| src/hyperlight_host/src/sandbox/mod.rs | Update sandbox module tests to use PathBuf guest paths. |
| src/hyperlight_host/src/sandbox/initialized_multi_use.rs | Change generate_crashdump_to_dir to accept Into<PathBuf> and plumb into crashdump generator. |
| src/hyperlight_host/src/metrics/mod.rs | Update metrics tests to use PathBuf guest paths. |
| src/hyperlight_host/src/mem/mgr.rs | Update memory manager tests to use PathBuf guest paths. |
| src/hyperlight_host/src/mem/exe.rs | Change ExeInfo::from_file to take &Path; update tests accordingly. |
| src/hyperlight_host/src/hypervisor/mod.rs | Update hypervisor tests to use PathBuf guest paths. |
| src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs | Remove string filename extraction; pass Option<PathBuf> into crashdump context. |
| src/hyperlight_host/src/hypervisor/gdb/mod.rs | Update GDB mem access tests to use PathBuf guest paths. |
| src/hyperlight_host/src/hypervisor/crashdump.rs | Store binary path as PathBuf; use var_os and PathBuf end-to-end for output paths. |
| src/hyperlight_host/examples/tracing/main.rs | Use PathBuf guest path helper in example. |
| src/hyperlight_host/examples/tracing-otlp/main.rs | Use PathBuf guest path helper in example. |
| src/hyperlight_host/examples/tracing-chrome/main.rs | Use PathBuf guest path helper in example. |
| src/hyperlight_host/examples/metrics/main.rs | Use PathBuf guest path helper in example. |
| src/hyperlight_host/examples/map-file-cow-test/main.rs | Pass PathBuf into GuestBinary::FilePath. |
| src/hyperlight_host/examples/logging/main.rs | Use PathBuf guest path helper in example. |
| src/hyperlight_host/examples/hello-world/main.rs | Pass PathBuf into GuestBinary::FilePath. |
| src/hyperlight_host/examples/guest-debugging/main.rs | Pass PathBuf into GuestBinary::FilePath for debug example/tests. |
| src/hyperlight_host/examples/func_ctx/main.rs | Use PathBuf guest path helper in example. |
| src/hyperlight_host/examples/crashdump/main.rs | Update crashdump example functions to take &Path / use PathBuf throughout. |
| src/hyperlight_host/benches/benchmarks.rs | Update benchmarks to pass PathBuf guest paths. |
| fuzz/fuzz_targets/host_print.rs | Switch fuzz harness guest path acquisition to PathBuf. |
| fuzz/fuzz_targets/host_call.rs | Switch fuzz harness guest path acquisition to PathBuf. |
| fuzz/fuzz_targets/guest_trace.rs | Switch fuzz harness guest path acquisition to PathBuf. |
| fuzz/fuzz_targets/guest_call.rs | Switch fuzz harness guest path acquisition to PathBuf. |
| CHANGELOG.md | Document breaking API change: guest/crashdump path APIs now use PathBuf. |
|
LGTM @ludfjig for a quick look |
There was a problem hiding this comment.
LGTM. @jprendes for quick look :) The only concern I might have that we perhaps want to consider pairing this break with others at the same time to lessen pain for users (but not a huge deal, the update-path is straight forward)
jprendes
left a comment
There was a problem hiding this comment.
I think we could take impl AsRef<Path> wherever we take a a String or &str (or Option<impl AsRef<Path>>, so that we stay backwards compatible.
Currently, we do not allow non-UTF-8 paths for loading guest binaries and crash dumps. This PR replaces
String-backed filesystem paths withPathBuf, thus allowing non-UTF-8 paths.Resolves #1518