-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WASI: path_open regression in Rust 1.51 when using LTO #82758
Comments
Those PRs look like a possible candidates: #79578, WebAssembly/wasi-libc#214. Perhaps there is a bug in my code, but it works with Rust 1.50, and the preopens are populated correctly even in Rust 1.51 (otherwise it wouldn't reach |
I think I found the culprit. This happens only in the release builds with LTO enabled, where
|
Confirmed: if I "use" use libc;
extern "C" {
fn __wasilibc_find_relpath_alloc(
path: *const libc::c_char,
abs_prefix: *mut *const libc::c_char,
relative_path: *mut *const libc::c_char,
relative_path_len: libc::size_t,
can_realloc: libc::c_int,
) -> libc::c_int;
}
fn main() {
unsafe {
let _ = __wasilibc_find_relpath_alloc(
0 as *const libc::c_char,
0 as *mut *const libc::c_char,
0 as *mut *const libc::c_char,
0,
0,
);
}
let _ = std::fs::File::open("/proxy-wasm-sandbox/regression.txt");
} |
@PiotrSikora can you clarify how you're building your project? Are you using an external libc or the one shipped with libstd? |
|
Just to clarify, this does not happen with LTO disabled? |
Ok I believe that this is fixed in #82804. The |
Correct.
Thanks! I verified that #82804 fixes the issue for me. |
Assigning @rustbot label -I-prioritize +P-high |
This commit fixes an issue pointed out in rust-lang#82758 where LTO changed the behavior of a program. It turns out that LTO was not at fault here, it simply uncovered an existing bug. The bindings to `__wasilibc_find_relpath` assumed that the relative portion of the path returned was always contained within thee input `buf` we passed in. This isn't actually the case, however, and sometimes the relative portion of the path may reference a sub-portion of the input string itself. The fix here is to use the relative path pointer coming out of `__wasilibc_find_relpath` as the source of truth. The `buf` used for local storage is discarded in this function and the relative path is copied out unconditionally. We might be able to get away with some `Cow`-like business or such to avoid the extra allocation, but for now this is probably the easiest patch to fix the original issue.
std: Fix a bug on the wasm32-wasi target opening files This commit fixes an issue pointed out in rust-lang#82758 where LTO changed the behavior of a program. It turns out that LTO was not at fault here, it simply uncovered an existing bug. The bindings to `__wasilibc_find_relpath` assumed that the relative portion of the path returned was always contained within thee input `buf` we passed in. This isn't actually the case, however, and sometimes the relative portion of the path may reference a sub-portion of the input string itself. The fix here is to use the relative path pointer coming out of `__wasilibc_find_relpath` as the source of truth. The `buf` used for local storage is discarded in this function and the relative path is copied out unconditionally. We might be able to get away with some `Cow`-like business or such to avoid the extra allocation, but for now this is probably the easiest patch to fix the original issue.
std: Fix a bug on the wasm32-wasi target opening files This commit fixes an issue pointed out in rust-lang#82758 where LTO changed the behavior of a program. It turns out that LTO was not at fault here, it simply uncovered an existing bug. The bindings to `__wasilibc_find_relpath` assumed that the relative portion of the path returned was always contained within thee input `buf` we passed in. This isn't actually the case, however, and sometimes the relative portion of the path may reference a sub-portion of the input string itself. The fix here is to use the relative path pointer coming out of `__wasilibc_find_relpath` as the source of truth. The `buf` used for local storage is discarded in this function and the relative path is copied out unconditionally. We might be able to get away with some `Cow`-like business or such to avoid the extra allocation, but for now this is probably the easiest patch to fix the original issue.
std: Fix a bug on the wasm32-wasi target opening files This commit fixes an issue pointed out in rust-lang#82758 where LTO changed the behavior of a program. It turns out that LTO was not at fault here, it simply uncovered an existing bug. The bindings to `__wasilibc_find_relpath` assumed that the relative portion of the path returned was always contained within thee input `buf` we passed in. This isn't actually the case, however, and sometimes the relative portion of the path may reference a sub-portion of the input string itself. The fix here is to use the relative path pointer coming out of `__wasilibc_find_relpath` as the source of truth. The `buf` used for local storage is discarded in this function and the relative path is copied out unconditionally. We might be able to get away with some `Cow`-like business or such to avoid the extra allocation, but for now this is probably the easiest patch to fix the original issue.
This commit fixes an issue pointed out in rust-lang#82758 where LTO changed the behavior of a program. It turns out that LTO was not at fault here, it simply uncovered an existing bug. The bindings to `__wasilibc_find_relpath` assumed that the relative portion of the path returned was always contained within thee input `buf` we passed in. This isn't actually the case, however, and sometimes the relative portion of the path may reference a sub-portion of the input string itself. The fix here is to use the relative path pointer coming out of `__wasilibc_find_relpath` as the source of truth. The `buf` used for local storage is discarded in this function and the relative path is copied out unconditionally. We might be able to get away with some `Cow`-like business or such to avoid the extra allocation, but for now this is probably the easiest patch to fix the original issue.
Discussed in weekly rustc triage meeting. We believe this was fixed by #82804. Closing as fixed. |
Code
I tried this code:
I expected to see this happen:
This is executed inside Envoy Proxy with preopen dir
/proxy-wasm-sandbox
, but hopefully the output is readable:The memory address and size passed in the call (
path_open(.., 1114320, 14, ..)
) correctly point to aregression.txt
.Instead, this happened:
The memory address and size passed in the call (
path_open(.., 1, 0, ..)
) are wrong.Version it worked on
It most recently worked on: Rust 1.50
rustc --version --verbose
:Version with regression
rustc --version --verbose
:@rustbot modify labels: +regression-from-stable-to-beta -regression-untriaged
The text was updated successfully, but these errors were encountered: