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.

Sign up for GitHub

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

wasm-interp: Add multi-module support #2009

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

wasm-interp: Add multi-module support #2009

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
d53a1e9
module-loading works, but errors!
tDwtp Sep 28, 2022
b9da00b
removed unnecessary lines and guarded exports
tDwtp Sep 28, 2022
ff68a0b
forgot to clear the maps
tDwtp Sep 28, 2022
f1d7371
no hints needed anymore
tDwtp Sep 28, 2022
86eb626
removed more notes expanded documentation properly clear the maps
tDwtp Sep 28, 2022
daab8ed
linting and better doc, test expanded
tDwtp Sep 29, 2022
d77a735
wasi working again
tDwtp Oct 1, 2022
b2a63ba
updated help and test file
tDwtp Oct 1, 2022
307e91c
resolve conflicts
tDwtp Oct 14, 2022
9be9d33
Merge branch 'main' into module-loading
tDwtp Oct 14, 2022
d4e4833
satisfy linter
tDwtp Oct 14, 2022
fbe602b
Merge branch 'module-loading' of https://github.com/tDwtp/wabt into m…
tDwtp Oct 14, 2022
baa0e5c
satisfy linter
tDwtp Oct 14, 2022
523aaa4
correction due to comments on #2009 and the linter
tDwtp Oct 14, 2022
18b09f1
type and spelling correction
tDwtp Oct 14, 2022
ba49f14
removed the flag from argumentl ist handling
tDwtp Oct 14, 2022
50db7e0
place InitWasi correctly inside macro-block
tDwtp Oct 14, 2022
ca6aa99
again fixed non-wasi build
tDwtp Oct 14, 2022
7f0489a
reverted something i removed for testing
tDwtp Oct 14, 2022
1ed7b45
dot removed from old parsing checks
tDwtp Oct 14, 2022
f7160d2
linter update
tDwtp Oct 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 64 additions & 61 deletions src/interp/interp-wasi.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

#include "uvwasi.h"

#include <cstdio>
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

was for testing and finding a memory-leak culprit. is removed. Thanks for the notice ._.

#include <cinttypes>
#include <unordered_map>

Expand Down Expand Up @@ -652,85 +653,90 @@ std::unordered_map<Instance*, WasiInstance*> wasiInstances;
namespace wabt {
namespace interp {

Result WasiBindImports(const Module::Ptr& module,
RefVec& imports,
Stream* stream,
Stream* trace_stream) {
Store* store = module.store();
for (auto&& import : module->desc().imports) {
if (import.type.type->kind != ExternKind::Func) {
stream->Writef("wasi error: invalid import type: %s\n",
import.type.name.c_str());
return Result::Error;
Result RegisterWasiInstance(const Instance::Ptr& instance,
uvwasi_s* uvwasi,
Stream* err_stream,
Stream* trace_stream) {
Store* store = instance.store();
auto module = store->UnsafeGet<Module>(instance->module());
auto&& module_desc = module->desc();

Memory::Ptr memory;
for (auto&& export_ : module_desc.exports) {
if (export_.type.name == "memory") {
if (export_.type.type->kind != ExternalKind::Memory) {
err_stream->Writef("wasi error: memory export has incorrect type\n");
return Result::Error;
}
memory = store->UnsafeGet<Memory>(instance->memories()[export_.index]);
break;
}
}

if (import.type.module != "wasi_snapshot_preview1" &&
import.type.module != "wasi_unstable") {
stream->Writef("wasi error: unknown module import: `%s`\n",
import.type.module.c_str());
return Result::Error;
}
if (!memory) {
err_stream->Writef("wasi error: memory export not found\n");
return Result::Error;
}

WasiInstance* wasi = new WasiInstance(instance, uvwasi, std::move(memory).get(), trace_stream);
wasiInstances[instance.get()] = std::move(wasi);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think std::move is meaningful on raw pointer.

Maybe just avoid the local and do wasiInstances[instance.get()] = new WasiInstance...


auto func_type = *cast<FuncType>(import.type.type.get());
auto import_name = StringPrintf("%s.%s", import.type.module.c_str(),
import.type.name.c_str());
HostFunc::Ptr host_func;
return Result::Ok;
}

// TODO(sbc): Validate signatures of imports.
#define WASI_FUNC(NAME) \
if (import.type.name == #NAME) { \
host_func = HostFunc::New(*store, func_type, NAME); \
goto found; \
void UnregisterWasiInstance(const Instance::Ptr& instance) {
WasiInstance* wasi = wasiInstances[instance.get()];
if (wasi) {
Copy link
Member

Choose a reason for hiding this comment

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

How about assert(wasi); and skip the if check?

Copy link
Contributor Author

@tDwtp tDwtp Oct 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It might already be deleted. The main file gets a delete before this happens, which might not be cleared, if _start was not found or used. See WasiRunStart. Its just for later, when I might load and clear without any order.

wasiInstances.erase(instance.get());
delete wasi;
}
#include "wasi_api.def"
#undef WASI_FUNC
}

stream->Writef("unknown wasi API import: `%s`\n", import.type.name.c_str());
return Result::Error;
found:
imports.push_back(host_func.ref());
Ref WasiGetImport(const Module::Ptr& module, const ImportDesc& import) {
auto func_type = *cast<FuncType>(import.type.type.get());
HostFunc::Ptr host_func;
Store* store = module.store();

// TODO(sbc): Validate signatures of imports.
#define WASI_FUNC(NAME) \
if (import.type.name == #NAME) { \
return HostFunc::New(*store, func_type, NAME).ref(); \
}
#include "wasi_api.def"
#undef WASI_FUNC

return Result::Ok;
return Ref::Null;
}

Result WasiRunStart(const Instance::Ptr& instance,
uvwasi_s* uvwasi,
Stream* err_stream,
Stream* trace_stream) {
static Result FindWasiEntryPoint(const Instance::Ptr& instance,
Stream* err_stream,
Func::Ptr* entry_func) {
Store* store = instance.store();
auto module = store->UnsafeGet<Module>(instance->module());
auto&& module_desc = module->desc();

Func::Ptr start;
Memory::Ptr memory;
for (auto&& export_ : module_desc.exports) {
if (export_.type.name == "memory") {
if (export_.type.type->kind != ExternalKind::Memory) {
err_stream->Writef("wasi error: memory export has incorrect type\n");
return Result::Error;
}
memory = store->UnsafeGet<Memory>(instance->memories()[export_.index]);
}
if (export_.type.name == "_start") {
if (export_.type.type->kind != ExternalKind::Func) {
err_stream->Writef("wasi error: _start export is not a function\n");
return Result::Error;
}
start = store->UnsafeGet<Func>(instance->funcs()[export_.index]);
*entry_func = store->UnsafeGet<Func>(instance->funcs()[export_.index]);
return Result::Ok;
}
if (start && memory) {
break;
}
}

if (!start) {
err_stream->Writef("wasi error: _start export not found\n");
return Result::Error;
}
return Result::Error;
}

if (!memory) {
err_stream->Writef("wasi error: memory export not found\n");
Result WasiRunStart(const Instance::Ptr& instance,
uvwasi_s* uvwasi,
Stream* err_stream,
Stream* trace_stream) {
Func::Ptr start;
Result found = FindWasiEntryPoint(instance, err_stream, &start);
if (found == Result::Error) {
err_stream->Writef("wasi error: "
"_start export not a function or not found\n");
return Result::Error;
}

Expand All @@ -739,11 +745,8 @@ Result WasiRunStart(const Instance::Ptr& instance,
return Result::Error;
}

// Register memory
WasiInstance wasi(instance, uvwasi, memory.get(), trace_stream);
wasiInstances[instance.get()] = &wasi;

// Call start ([] -> [])
Store* store = instance.store();
Values params;
Values results;
Trap::Ptr trap;
Expand All @@ -753,7 +756,7 @@ Result WasiRunStart(const Instance::Ptr& instance,
}

// Unregister memory
wasiInstances.erase(instance.get());
UnregisterWasiInstance(instance);
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be renamed I think.

Copy link
Member

Choose a reason for hiding this comment

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

Should this line be removed now that the registration and unregistration now all seems to be taken care in wasm-interp.cc?

Copy link
Contributor Author

@tDwtp tDwtp Oct 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Nope, You might use it elsewhere (in spec-test for example). I did not want to monolith wasm-interp.cc.

EDIT: also see: WasiRunStart

return res;
}

Expand Down
12 changes: 8 additions & 4 deletions src/interp/interp-wasi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@ struct uvwasi_s;
namespace wabt {
namespace interp {

Result WasiBindImports(const Module::Ptr& module,
RefVec& imports,
Stream* err_stream,
Stream* trace_stream);
Result RegisterWasiInstance(const Instance::Ptr& instance,
uvwasi_s* uvwasi,
Stream* stream,
Stream* trace_stream);

void UnregisterWasiInstance(const Instance::Ptr& instance);
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 make all these function symbols start with Wasi?


Ref WasiGetImport(const Module::Ptr& module, const ImportDesc& import);

Result WasiRunStart(const Instance::Ptr& instance,
uvwasi_s* uvwasi,
Expand Down
Loading