rustc: Fix wasm64 metadata object files by alexcrichton · Pull Request #121464 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation10 Commits1 Checks0 Files changed
Conversation
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
[ Show hidden characters]({{ revealButtonHref }})
It looks like LLD will detect object files being either 32 or 64-bit depending on any memory present. LLD will additionally reject 32-bit objects during a 64-bit link. Previously metadata objects did not have any memories in them which led LLD to conclude they were 32-bit objects which broke 64-bit targets for wasm.
This commit fixes this by ensuring that for 64-bit targets there's a memory object present to get LLD to detect it's a 64-bit target. Additionally this commit moves away from a hand-crafted wasm encoder to the wasm-encoder crate on crates.io as the complexity grows for the generated object file.
Closes #121460
r? @wesleywiser
rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Area: The testsuite used to check the correctness of rustc
Status: Awaiting review from the assignee but also interested parties.
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.
The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging.
I'll also note that if the compiler would prefer to not depend on wasm-encoder I'm happy to do something more hand-coded instead. I figured at this point though with a new kind of section it was complicated enough to cross the threshold of "probably worth it to have a dependency".
It looks like LLD will detect object files being either 32 or 64-bit depending on any memory present. LLD will additionally reject 32-bit objects during a 64-bit link. Previously metadata objects did not have any memories in them which led LLD to conclude they were 32-bit objects which broke 64-bit targets for wasm.
This commit fixes this by ensuring that for 64-bit targets there's a
memory object present to get LLD to detect it's a 64-bit target.
Additionally this commit moves away from a hand-crafted wasm encoder to
the wasm-encoder crate on crates.io as the complexity grows for the
generated object file.
Closes rust-lang#121460
Comment on lines +651 to +654
| imports.import( |
|---|
| "env", |
| "__linear_memory", |
| wasm_encoder::MemoryType { minimum: 0, maximum: None, memory64: true, shared: false }, |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, does the memory persist into the final wasm object or can it be dropped by the linker since it's unused?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, but this is only here to assist with detection of what kind of object it is, I believe LLD has separate logic for omitting the memory if it's not needed (although practically all rust programs will need a linear memory)
📌 Commit 646e8e7 has been approved by wesleywiser
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
bors added a commit to rust-lang-ci/rust that referenced this pull request
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#121464 - alexcrichton:fix-wasm64, r=wesleywiser
rustc: Fix wasm64 metadata object files
It looks like LLD will detect object files being either 32 or 64-bit depending on any memory present. LLD will additionally reject 32-bit objects during a 64-bit link. Previously metadata objects did not have any memories in them which led LLD to conclude they were 32-bit objects which broke 64-bit targets for wasm.
This commit fixes this by ensuring that for 64-bit targets there's a memory object present to get LLD to detect it's a 64-bit target. Additionally this commit moves away from a hand-crafted wasm encoder to the wasm-encoder crate on crates.io as the complexity grows for the generated object file.
Closes rust-lang#121460
celinval pushed a commit to model-checking/kani that referenced this pull request
Relevant upstream changes:
rust-lang/rust#120675: An intrinsic Symbol is
now wrapped in a IntrinsicDef struct, so the relevant part of the code
needed to be updated.
rust-lang/rust#121464: The second argument of
the create_wrapper_file function changed from a vector to a string.
rust-lang/rust#121662: NullOp::DebugAssertions
was renamed to NullOp::UbCheck and it now has data (currently unused
by Kani)
rust-lang/rust#121728: Introduces F16 and
F128, so needed to add stubs for them
rust-lang/rust#121969: parse_sess was renamed
to psess, so updated the relevant code.
rust-lang/rust#122059: The
is_val_statically_known intrinsic is now used in some core::fmt
code, so had to handle it in (codegen'ed to false).
rust-lang/rust#122233: This added a new
retag_box_to_raw intrinsic. This is an operation that is primarily
relevant for stacked borrows. For Kani, we just return the pointer.
Resolves #3057
Labels
Area: The testsuite used to check the correctness of rustc
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Relevant to the compiler team, which will review and decide on the PR/issue.