add all wasi hostcalls used by Go SDK#533
Conversation
Signed-off-by: zty98751 <zty98751@alibaba-inc.com>
Signed-off-by: Matt Leon <mattleon@google.com>
leonm1
requested review from
PiotrSikora,
martijneken and
mpwarres
as code owners
| @@ -379,28 +379,65 @@ ContextBase *WasmBase::getRootContext(const std::shared_ptr<PluginBase> &plugin, | |||
| void WasmBase::startVm(ContextBase *root_context) { | |||
| // wasi_snapshot_preview1.clock_time_get | |||
| wasm_vm_->setRestrictedCallback( | |||
There was a problem hiding this comment.
Is there anything ABI we are actually preventing from being called during startVm?
If not, remove this?
If yes, invert the list if possible (TODO?), and for now let's document which surfaces are out of bounds.
There was a problem hiding this comment.
You cannot invert the list, since it's primarily used when allocating memory (where we're very strict).
As for the changes in this PR:
// None of the following wasi functions _should_ be called during
// initialization, but global variable initialization happens during
// `_initialize`, which means what happens during this phase is fully up
// to the wasm module author's code and their compiler implementation.
// For Go, module initializers run during this phase as well.
//
// Allow all of these (for the most part, they just return ENOTSUP) to
// maximize compatibility and prevent issues upon updating compiler
// versions.
mpwarres
left a comment
mpwarres
left a comment
There was a problem hiding this comment.
LGTM mod minor comment and question. Thanks!
| // len); | ||
| Word wasi_unstable_fd_allocate(Word fd, uint64_t offset, uint64_t len) { | ||
| // fd_allocate is used to ensure that space is allocated for a file. | ||
| // Since we don't have a real file system in proxy-wasm, we can just return success without doing |
There was a problem hiding this comment.
Suggest using the language present in other hostcalls that only allow stdout and stderr FDs, i.e.:
// Since we don't have a real file system in proxy-wasm, we can just return success for
// stdout/stderr and an error for other file descriptors.
| return 21; // __WASI_EFAULT - If there is a failure setting memory | ||
| } | ||
|
|
||
| return 0; // __WASI_ESUCCESS |
There was a problem hiding this comment.
I forget, why do we want to return success from this hostcall given that it is a no-op?
5 participants
Rebases and updates #433. Original PR discussion follows:
In #427, a portion of the wasi hostcalls was added, but not all. Now, all the wasi hostcalls have been included, and their stability has been verified in our multiple go 1.24 compiled wasm plugins