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

Use trampolines for all libcalls in engine-universal and engine-dylib #2748

Merged
merged 1 commit into from Jan 20, 2022

Conversation

Amanieu
Copy link
Contributor

@Amanieu Amanieu commented Jan 11, 2022

In both of these engines, the compiled code may be loaded in memory far
from the Wasmer runtime which means that libcalls may not be reachable
through the normal relocation types. Instead a trampoline is needed to
allow reaching any address in the 64-bit address space.

In the case of engine-dylib, this is even worse since the symbols are
not exported by the executable without some special linker flags. The
solution here is to manually patch in the addresses at load time into
a data table of function pointers.

Fixes #1722

@Amanieu Amanieu requested a review from ptitSeb Jan 11, 2022
@Amanieu Amanieu requested a review from syrusakbary as a code owner Jan 11, 2022
@Amanieu
Copy link
Contributor Author

@Amanieu Amanieu commented Jan 11, 2022

This is still incomplete: LLVM is bypassing the trampoline generation since it generates the object file directly.

@syrusakbary
Copy link
Member

@syrusakbary syrusakbary commented Jan 14, 2022

I think @ptitSeb will be the best one to review this

@Amanieu
Copy link
Contributor Author

@Amanieu Amanieu commented Jan 14, 2022

Blocked on gimli-rs/object#422.

@Amanieu Amanieu force-pushed the libcall_trampolines branch 2 times, most recently from 1be3c73 to 6e40e90 Compare Jan 19, 2022
@Amanieu
Copy link
Contributor Author

@Amanieu Amanieu commented Jan 19, 2022

Ready for review

@syrusakbary
Copy link
Member

@syrusakbary syrusakbary commented Jan 19, 2022

This LGTM, @ptitSeb can you take a look to approve it or give feedback?

// We currently allocate all code segments independently, so nothing
// is colocated.
colocated: false,
colocated: true,
Copy link
Member

@syrusakbary syrusakbary Jan 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be missing some context on what colocated means. Why is this necessary?

Copy link
Contributor Author

@Amanieu Amanieu Jan 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary to avoid text relocations. It means that all libcalls are reachable through normal call instructions (+/- 2G on x86, +/- 128M on aarch64) so it doesn't need to load the target from memory and do an indirect call. This is true since we generate trampolines that are near the compiled code.

The text relocations are due to the way Cranelift generates indirect calls: it places the target as a pointer in the .text section (since it puts everything in .text) and marks it with a relocation so the linker fills in the libcall address at load time.

In both of these engines, the compiled code may be loaded in memory far
from the Wasmer runtime which means that libcalls may not be reachable
through the normal relocation types. Instead a trampoline is needed to
allow reaching any address in the 64-bit address space.

In the case of engine-dylib, this is even worse since the symbols are
not exported by the executable without some special linker flags. The
solution here is to manually patch in the addresses at load time into
a data table of function pointers.
@Amanieu
Copy link
Contributor Author

@Amanieu Amanieu commented Jan 20, 2022

bors r+

@bors
Copy link
Contributor

@bors bors bot commented Jan 20, 2022

@bors bors bot merged commit 8472703 into master Jan 20, 2022
11 checks passed
@bors bors bot deleted the libcall_trampolines branch Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants