Stream: t-compiler/wg-prioritization/alerts

Topic: #82758 WASI: path_open regression in Rust 1.51


triagebot (Mar 04 2021 at 11:32, on Zulip):

@WG-prioritization/alerts issue #82758 has been requested for prioritization.

Procedure

Yuki Okushi (Mar 05 2021 at 10:02, on Zulip):

looks like it's painful for WASI users but it happens with --release

Yuki Okushi (Mar 05 2021 at 10:03, on Zulip):

I'd go for P-medium but I'm fine with P-high if someone has concerns

apiraino (Mar 05 2021 at 10:50, on Zulip):

right, my understanding here is that compiling for --release with optimizations enabled breaks loading files from the filesystem, which looks a bad enough regression on beta waiting to land on stable in ~3 weeks, mitigated only by the fact that wasm32-wasi is a Tier 2 target.

Given this (perhaps limited context) I'd like to assign a P-high and hopefully someone investigate this, but if P-medium is more appropriate, ok for that.

Yuki Okushi (Mar 05 2021 at 10:55, on Zulip):

makes sense, then P-high seems better :+1:

Léo Lanteri Thauvin (Mar 05 2021 at 12:29, on Zulip):

This requires LTO too

Léo Lanteri Thauvin (Mar 05 2021 at 12:29, on Zulip):

This happens only in the release builds with LTO enabled

apiraino (Mar 05 2021 at 12:37, on Zulip):

Léo Lanteri Thauvin said:

This requires LTO too

sorry, I'm missing the implication of this. Are you just stating a fact or do you imply that's a factor that makes the error worse or (on the contrary) less important?

Léo Lanteri Thauvin (Mar 05 2021 at 12:50, on Zulip):

The author said in https://github.com/rust-lang/rust/issues/82758#issuecomment-790583395:

This happens only in the release builds with LTO enabled

This is also visible in their Cargo.toml:

[profile.release]
lto = true

And, as LTO is disabled by default, the bug relying on it could be somewhat less important.

Léo Lanteri Thauvin (Mar 05 2021 at 12:51, on Zulip):

Posted https://github.com/rust-lang/rust/issues/82758#issuecomment-791398875 to ask if this does indeed require LTO

apiraino (Mar 05 2021 at 13:24, on Zulip):

ok, understood - thanks!

Yuki Okushi (Mar 05 2021 at 18:16, on Zulip):

FYI: Alex submitted a fix, https://github.com/rust-lang/rust/pull/82804

Yuki Okushi (Mar 05 2021 at 18:17, on Zulip):

given they consider backporting it, I think P-high is better (assuming it's still the T-libs-impl issue)

Yuki Okushi (Mar 05 2021 at 18:17, on Zulip):

(either way, P-high or P-medium is not that big a deal here, I feel)

Léo Lanteri Thauvin (Mar 05 2021 at 18:23, on Zulip):

Nominated the fix

apiraino (Mar 10 2021 at 14:50, on Zulip):

there's a patch on the way for this, but assigning a P-high, it's also nominated for a beta backport

Last update: Apr 10 2021 at 22:00UTC