std: Fix a bug on the wasm32-wasi target opening files#82804
Merged
bors merged 1 commit intorust-lang:masterfrom Mar 14, 2021
Merged
std: Fix a bug on the wasm32-wasi target opening files#82804bors merged 1 commit intorust-lang:masterfrom
bors merged 1 commit intorust-lang:masterfrom
Conversation
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.
Contributor
|
r? @m-ou-se (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Member
Author
|
Also, if possible, I'd like to nominate this for beta inclusion since this is a relatively serious bug for the wasi target |
LeSeulArtichaut
added
the
beta-nominated
apiraino
added
T-libs
Contributor
|
Reviewed in T-compiler meeting. @bors r+ |
Collaborator
|
📌 Commit d6b06b8 has been approved by |
bors
added
S-waiting-on-bors
Contributor
|
beta backport discussed and delegated to T-libs-impl call on this. Approved for nightly. |
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Mar 11, 2021
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.
Collaborator
|
⌛ Testing commit d6b06b8 with merge 9e7307ab7817bc4fe561e6d0132326b035530137... |
Collaborator
|
💥 Test timed out |
bors
added
S-waiting-on-review
Collaborator
Member
|
@bors retry |
bors
added
S-waiting-on-bors
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Mar 13, 2021
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.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 14, 2021
Rollup of 10 pull requests Successful merges: - rust-lang#81465 (Add documentation about formatting `Duration` values) - rust-lang#82121 (Implement Extend and FromIterator for OsString) - rust-lang#82617 (Document `everybody_loops`) - rust-lang#82789 (Get with field index from pattern slice instead of directly indexing) - rust-lang#82798 (Rename `rustdoc` to `rustdoc::all`) - rust-lang#82804 (std: Fix a bug on the wasm32-wasi target opening files) - rust-lang#82943 (Demonstrate best practice for feeding stdin of a child processes) - rust-lang#83066 (Add `reverse` search alias for Iterator::rev()) - rust-lang#83070 (Update cargo) - rust-lang#83081 (Fix panic message of `assert_failed_inner`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Contributor
|
Discussed in this week's T-compiler triage meeting. Beta backport approved. |
pnkfelix
added
the
beta-accepted
Mark-Simulacrum
removed
the
beta-nominated
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 22, 2021
…imulacrum [stable] 1.51.0 release Also includes backports of the release notes, as well as: * SplitInclusive is public API rust-lang#83372 * std: Fix a bug on the wasm32-wasi target opening files rust-lang#82804 * Fix io::copy specialization using copy_file_range when writer was opened with O_APPEND rust-lang#82417 r? `@Mark-Simulacrum`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit fixes an issue pointed out in #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_relpathassumed that the relative portion of the pathreturned was always contained within thee input
bufwe passed in. Thisisn'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_relpathas the source of truth. Thebufused forlocal 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 nowthis is probably the easiest patch to fix the original issue.