-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Fix wasm_import_module attribute cross-crate
#148363
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
Conversation
This comment has been minimized.
This comment has been minimized.
5c2ab46 to
dc48982
Compare
This comment has been minimized.
This comment has been minimized.
This commit fixes an accidental regression from 144678 where wasm targets would now accidentally use the wrong import module map for a symbol causing a symbol to skip mangling. This can result in compilation failures when symbols are used in cross-crate situations. Closes 148347
dc48982 to
93fef45
Compare
|
I built this and verified that it now works as expected for both the smaller examples shared as well as for some larger projects that had previously encountered a large number of linker errors. |
|
It looks like we found some more issues as a result of this bug, namely a runtime issue so this isn't just a build failure. Given that I'd like to propose this get backported to beta at least, so I'm going to attempt to beta nominate this. Haven't done this in awhile... @rustbot beta-nominate compiler This is not something I'd ask for a stable backport of, but if a stable point release is already going out I wouldn't mind trying to champion trying to get this in there. I'll sort that out if/when this is accepted for a beta backport. |
|
Error: The feature Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip. |
|
I've probably abused my priviledges and manually added the label here, please teach me the error of my ways if that is uncouth. |
(This is perfectly fine, the triagebot invocation is just |
|
@rustbot label +stable-nominated Some discussion at #t-compiler/backports > #148363: beta-nominated @ 💬 and it sounds like it's ok to tag this as stable-nominated but also to be clear that this alone, in my opinion, doesn't justify a point release on its own. If one's already going out I think this would be reasonable to include, however. |
|
@bors r+ |
…=jackh726 Fix `wasm_import_module` attribute cross-crate This commit fixes an accidental regression from rust-lang#144678 where wasm targets would now accidentally use the wrong import module map for a symbol causing a symbol to skip mangling. This can result in compilation failures when symbols are used in cross-crate situations. Closes rust-lang#148347
|
@bors p=1 Fixes high-priority issue, beta and stable backport nominated PR |
Rollup of 7 pull requests Successful merges: - #147141 (Suggest making binding `mut` on `&mut` reborrow) - #147945 (Port `cfg!()` macro to the new attribute parsing system ) - #147951 (Add check for `+=` typo in let chains) - #148004 (fix: Only special case single line item attribute suggestions) - #148264 (reflect that type and const parameter can be intermixed) - #148363 (Fix `wasm_import_module` attribute cross-crate) - #148447 (Tweak E0401) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #148363 - alexcrichton:fix-wasm-link-name, r=jackh726 Fix `wasm_import_module` attribute cross-crate This commit fixes an accidental regression from #144678 where wasm targets would now accidentally use the wrong import module map for a symbol causing a symbol to skip mangling. This can result in compilation failures when symbols are used in cross-crate situations. Closes #148347
|
This makes it near impossible to recognize wasi symbols in Miri to implement shims... we now see the mangled name there, unlike on every other target. I thought I could implement basic support for wasi myself, that was apparently a misjudgment on my side. I think I'm going to remove the little bit of wasi support we have in Miri as this target is clearly too strange to support it without someone who knows it in detail being actively involved. |
|
Miri can look up the DefId of the function in wasm_import_map to get the name by which the wasm module would import it and from which module it would import it. |
|
Yeah we could add more target-specific hacks... but that falls into the "target is too strange" category for me. I thought it'd be easy to get a bit of wasi support, but I won't commit to maintaining a target that needs special treatment for even just the most basic functionality. So, if someone shows up who wants to maintain wasi in Miri, we can make that work, but I see this as a signal that I shouldn't be trying to do this myself. ;) |
|
Error: Label beta-accepted can only be set by Rust team members Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip. |
[stable] Prepare Rust 1.91.1 This PR prepares the artifacts for Rust 1.91.1, targeting next Monday with the following backports: * #148322 * #148363 cc `@rust-lang/release`
[stable] Prepare Rust 1.91.1 This PR prepares the artifacts for Rust 1.91.1, targeting next Monday with the following backports: * #148322 * #148363 cc `@rust-lang/release`
[stable] Prepare Rust 1.91.1 This PR prepares the artifacts for Rust 1.91.1, targeting next Monday with the following backports: * #148322 * #148363 cc `@rust-lang/release`
This commit fixes an accidental regression from #144678 where wasm targets would now accidentally use the wrong import module map for a symbol causing a symbol to skip mangling. This can result in compilation failures when symbols are used in cross-crate situations.
Closes #148347