uucore: fix OsStr byte conversions on wasip2 and wasip3 - #13755
uucore: fix OsStr byte conversions on wasip2 and wasip3#13755GamePad64 wants to merge 2 commits into
Conversation
`target_os = "wasi"` is true for p1, p2 and p3, but `std::os::wasi` is unstable on p2 and not compiled at all on p3. The helpers gating on `target_env = "p1"` therefore misbehaved on the other two: - `os_str_from_bytes`, `os_string_from_vec` and `os_string_to_vec` took the UTF-8-validating path and rejected byte strings that are valid on WASI, even though `os_str_as_bytes` already returned raw bytes there. - `path_ends_with_terminator` gated its `as_encoded_bytes` branch to exactly p2, so on p3 it used `as_bytes` with no `OsStrExt` in scope and did not compile. Use the `OsStr::as_encoded_bytes` family instead: stable since 1.74 and uniform across p1/p2/p3. On WASI `OsStr` is byte-based, so `from_encoded_bytes_unchecked` is sound. unix and Windows are unchanged. Verified with a non-UTF-8 round-trip built for wasm32-wasip2 and run under wasmtime: rejected before, preserved after. wasip3 is not compile-tested, as std does not yet build for that target.
Assert that `os_string_from_vec`/`os_string_to_vec` and
`os_str_from_bytes`/`os_str_as_bytes` round-trip non-UTF-8 bytes on
byte-oriented platforms, which is unix and every WASI environment.
Built for wasm32-wasip2 and run under wasmtime, they fail before the
preceding commit and pass after it:
before: Unable to transform bytes into OsStr
after: 2 passed
On unix they pass either way and guard against a future regression.
Merging this PR will degrade performance by 18.29%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | df_with_path |
571.4 µs | 699.4 µs | -18.29% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing GamePad64:uucore-wasi-non-p1-osstr (d8e25d8) with main (21d4e96)
Footnotes
-
46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
| #[cfg(target_os = "wasi")] | ||
| // SAFETY: on WASI `OsString` is a plain byte string with no encoding | ||
| // invariant, so every byte sequence is a valid `OsString`. | ||
| return Ok(unsafe { OsString::from_encoded_bytes_unchecked(vec) }); |
There was a problem hiding this comment.
can we avoid the unsafe ?
| return Ok(Cow::Borrowed( | ||
| // SAFETY: on WASI `OsStr` is a plain byte string with no encoding | ||
| // invariant, so every byte sequence is a valid `OsStr`. | ||
| unsafe { OsStr::from_encoded_bytes_unchecked(bytes) }, |
|
please add tests showing that the issue is fixed |
aec25de to
d8e25d8
Compare
|
Once the |
3 participants
target_os = "wasi"is true for p1, p2 and p3, butstd::os::wasiis unstable on p2 and not compiled at all on p3. The helpers gating ontarget_env = "p1"therefore misbehaved on the other two:os_str_from_bytes,os_string_from_vecandos_string_to_vectook the UTF-8-validating path and rejected byte strings that are valid on WASI, even thoughos_str_as_bytesalready returned raw bytes there.path_ends_with_terminatorgated itsas_encoded_bytesbranch to exactly p2, so on p3 it usedas_byteswith noOsStrExtin scope and did not compile.Use the
OsStr::as_encoded_bytesfamily instead: stable since 1.74 and uniform across p1/p2/p3. On WASIOsStris byte-based, sofrom_encoded_bytes_uncheckedis sound. unix and Windows are unchanged.Verified with a non-UTF-8 round-trip built for wasm32-wasip2 and run under wasmtime: rejected before, preserved after. wasip3 is not compile-tested, as std does not yet build for that target.