Skip to content
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: Cannot open paths with nightly >= 2021-03-11 when linked with LLD 11.1 #85840

Closed
abbec opened this issue May 30, 2021 · 18 comments
Closed
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections O-wasm Target: WASM (WebAssembly), http://webassembly.org/ P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Milestone

Comments

@abbec
Copy link

abbec commented May 30, 2021

I have code opening a path like std::fs::read("/absolute/path.txt") in WASI. All the surrounding code stays the same but when I upgrade nightly from 2021-03-10 to 2021-03-11 it does not work anymore.

Version details:

# opening a path with leading "/" does _NOT_ work
rustc 1.52.0-nightly (f98721f88 2021-03-10)
binary: rustc
commit-hash: f98721f886ab52d32d622ad0a46216ad03f3e525
commit-date: 2021-03-10
host: x86_64-apple-darwin
release: 1.52.0-nightly
# opening a path with leading "/" _DOES_ work
rustc 1.52.0-nightly (3a5d45f68 2021-03-09)
binary: rustc
commit-hash: 3a5d45f68cadc8fff4fbb557780f92b403b19c19
commit-date: 2021-03-09
host: x86_64-apple-darwin
release: 1.52.0-nightly
LLVM version: 12.0.0

Looking at the history, the only thing I can see that looks WASI-related is this but I have no idea here and any help would be appreciated!

EDIT: This description turned out to be somewhat incorrect, see below for a good repro case.

@abbec abbec added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels May 30, 2021
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. labels May 30, 2021
@LeSeulArtichaut LeSeulArtichaut added O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 30, 2021
@abbec
Copy link
Author

abbec commented May 30, 2021

I was wrong, actually it does not work at all on nightly 2021-03-11 and onwards (I tested nightly 2021-05-11 with the same result), with or without the leading "/":

failed to find a pre-opened file descriptor through which "relative/path.txt" could be opened

and

failed to find a pre-opened file descriptor through which "/absolute/path.txt" could be opened

whereas on nightly 2021-03-10 the same code works fine both with and without the leading "/".

@apiraino
Copy link
Contributor

apiraino commented May 31, 2021

hi @abbec thanks for filing a report. Do you imply that even a snippet like this returns that error?

use std::fs;
fn repro() {
    fs::read("/absolute/path.txt").unwrap();
}
fn main() {
    repro();
}

Can you attach a small but fully actionable and reproducible snippet (along with the command to compile)? There might be more to investigate and that would be useful. Thanks!

@abbec

This comment has been minimized.

@abbec

This comment has been minimized.

@abbec

This comment has been minimized.

@apiraino
Copy link
Contributor

Which command and rustc flags do you use to compile the sources? Also, do you get an actual error stack? If yes, can you expand that error with RUST_BACKTRACE=full? Are you able to test against the latest LLVM 12?

@abbec

This comment has been minimized.

@abbec

This comment has been minimized.

@abbec
Copy link
Author

abbec commented May 31, 2021

Yeah, it does work with LLVM 12 but not with LLVM 11.1.

REPRO CASE:

use std::fs;

fn repro() {
    let s = fs::read("/absolute/path.txt").unwrap();
    println!("{:#?}", s);
}

fn main() {
    repro();
}

Run this

$ rustup toolchain install nightly-2021-03-11
$ rustup default nightly-2021-03-11
$ rustup target add wasm32-wasi
$ export CARGO_TARGET_WASM32_WASI_LINKER=/path/to/llvm-11/bin/lld
$ cargo build --target wasm32-wasi
$ echo "YES" > path.txt
$ wasmtime run --mapdir=absolute::. target/wasm32-wasi/debug/rust-repro.wasm
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: Other, error: "failed to find a pre-opened file descriptor through which \"/absolute/path.txt\" could be opened" }', src/main.rs:4:44

The same thing works fine with

$ rustup toolchain install nightly-2021-03-10
$ rustup default nightly-2021-03-10
$ rustup target add wasm32-wasi
$ export CARGO_TARGET_WASM32_WASI_LINKER=/path/to/llvm-11/bin/lld
$ cargo build --target wasm32-wasi
$ echo "YES" > path.txt
$ wasmtime run --mapdir=absolute::. target/wasm32-wasi/debug/rust-repro.wasm
[
    89,
    69,
    83,
    10,
]

@abbec abbec changed the title WASI: Behavior change opening "absolute" paths WASI: Cannot open paths with nightly >= 2021-03-11 when linked with LLD 11.1 May 31, 2021
@apiraino
Copy link
Contributor

apiraino commented Jun 2, 2021

great work @abbec for creating a reproducible example.

pinging ICE breakers to try to identify where this started (need to be triaged with both LLVM installed), so also adding LLVM label

@rustbot ping icebreakers-cleanup-crew

@rustbot label +A-LLVM

@apiraino apiraino added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Jun 2, 2021
@rustbot
Copy link
Collaborator

rustbot commented Jun 2, 2021

@rustbot rustbot added the ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections label Jun 2, 2021
@rustbot
Copy link
Collaborator

rustbot commented Jun 2, 2021

@pietroalbini pietroalbini added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Jun 14, 2021
@pietroalbini pietroalbini added this to the 1.52.0 milestone Jun 14, 2021
@pietroalbini
Copy link
Member

This seems to have been introduced in 1.52.0 nightly given the affected version. Relabeling as a stable-to-stable regression.

@joshtriplett
Copy link
Member

cc @alexcrichton

@m-ou-se m-ou-se removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 23, 2021
@m-ou-se m-ou-se added the P-medium Medium priority label Jun 23, 2021
@alexcrichton
Copy link
Member

@abbec could you describe a bit why you're linking with a non-Rust LLD, and specifically an older-than-LLVM-12 LLD?

Given your range of nightlies for the regression this seems most likely due to #82924 which I could imagine relies on features and/or bug fixes in LLVM 12 to work.

In general the wasm targets right now are intended to work best with the LLD that Rust ships itself, and otherwise if you're looking to interop with C/C++ code it's recommended to say pretty up-to-date because there's still a good deal of feature development and bug fixes happening.

@abbec
Copy link
Author

abbec commented Jun 23, 2021

Yeah to be fair, this issue popped up in passing and I am now using LLVM 12 which works fine.

Reason for using an external linker is a custom libc and other C libraries.

@alexcrichton
Copy link
Member

Ah ok, if that's the case I'd recommend closing this because the wasm target and toolchain are undergoing enough changes that not a lot of effort is put into making all versions of objects work with past and future toolchains. This will naturally evolve over time as wasm stabilizes, but at this time I think the guarantee of Rust is that if you want to intermix Rust & C on wasm you need to have matching LLVM versions.

@apiraino
Copy link
Contributor

ok, closing this as per Alex's suggestion. @abbec please feel free to action this issue again in case something relevant happens again. Thanks again for your time on this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections O-wasm Target: WASM (WebAssembly), http://webassembly.org/ P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants