Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Conversation

@pepyakin
Copy link
Contributor

This is a quick and dirty implementation of the wasmtime counterpart of #4550. This is aimed to unblock polkadot progress while we are waiting #4686.

The idea is that we employ parity-wasm to extract all the imports and do the resolving ourselves, then we collect all the missing function imports and create modules and stubs for them.

@pepyakin pepyakin added the A0-please_review Pull request needs code review. label Jan 24, 2020
@pepyakin pepyakin requested a review from bkchr January 24, 2020 15:05
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Looks good so far, just some small improvements :)

func_ty
}
_ => {
// The executor doesn't provide any non function imports!
Copy link
Member

Choose a reason for hiding this comment

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

When we switch to import memory, will this not "break" here? Why not ignore non function imports?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it will bail here! We can remove this in rely it to fail at the instantiation time.

};
let signature = convert_parity_wasm_signature(func_ty);

if import_entry.module() == "env" {
Copy link
Member

Choose a reason for hiding this comment

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

Can we have function exports that don't have the module env?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we can! If I remember correctly, in Rust you'd use #[link(wasm_import_module = "non_env")] for that.

MissingFunctionStubs::new()
};

let env_missing_functions = missing_functions_stubs.stubs.remove("env").unwrap_or_else(|| Vec::new());
Copy link
Member

Choose a reason for hiding this comment

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

Can we not do this in the loop below and just check:

if module == env {
    host_functions
} else {
    &[]
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main point of doing that is not in host functions but rather in creating a new instance and giving it a name for non-env modules.

@bkchr bkchr merged commit d56db2b into master Jan 25, 2020
@bkchr bkchr deleted the ser-dirty-wasmtime-allow-imports branch January 25, 2020 22:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants