bytecodealliance / wasm-micro-runtime

WebAssembly Micro Runtime (WAMR)
Apache License 2.0
4.83k stars 617 forks source link

import/export issue #1353

Open yamt opened 2 years ago

yamt commented 2 years ago

i wrote a test case to experiment memory import. (see below)

my intention is to share a single "mem" instance. i believe it's the spec-wise correct behavior. it works as i expected for other engines. (i tested with the reference interpreter, wabt, wasmer, wasmtime, toywasm)

unfortunately, it didn't work for wamr. it seems wamr creates two instances for "mem". is it an intended divergence from the spec? i can understand both behaviors can have valid use cases.

while i can "fix" sub_module_instantiate to use shared instances for dependencies, i feel it actually needs a bigger surgery to fix properly. IMO, import/export processing should be done at instance level, not module level. how do you think?

(module (memory (export "m1") 1))

(register "mem")

(module
(memory (import "mem" "m1") 0)
(func (export "inc") (param $i i32) (i32.store (local.get $i) (i32.add (i32.const 1) (i32.load (local.get $i)))))
)

(register "sub")

(module
(memory (import "mem" "m1") 0)
(func (export "inc") (import "sub" "inc") (param $i i32))
(func (export "get") (param $i i32) (result i32) (i32.load (local.get $i)))
)

(assert_return (invoke "get" (i32.const 0)) (i32.const 0))
(assert_return (invoke "inc" (i32.const 0)))
(assert_return (invoke "get" (i32.const 0)) (i32.const 1))
(assert_return (invoke "inc" (i32.const 0)))
(assert_return (invoke "get" (i32.const 0)) (i32.const 2))
lum1n0us commented 2 years ago

YES, it is a design choice about whehter one sub-module should have multiple instances if it is imported multiple times.

In above case, module sum and module main(I am going to call the third module "main") create their own module mem instance and create two linear memories. So both functions inc() and get() actually operate differnt linear memories.

image

inc() writes the No.2 linear memory via sub.inc() while get() reads the No.1 linear memory.

The most reason of this choice is we want keep a private memory for every module. I guess there are some stories how an unsafe 3rd library successl peek user data inlight us.

WAMR is using a loading-time-link mechanism. It is similar with the process of ld searchs and loads all shared libraries used by a program. It is an old fashioned way and looks reasonable if there are only .wasm files until host-made runtime objects involved. wasm-c-api provides another way to link at instantiation wasm_instance_new(). Lucky, all those divergence will be merged in the "component model feature".

yamt commented 2 years ago

WAMR is using a loading-time-link mechanism. It is similar with the process of ld searchs and loads all shared libraries used by a program. It is an old fashioned way and looks reasonable if there are only .wasm files until host-made runtime objects involved. wasm-c-api provides another way to link at instantiation wasm_instance_new().

i haven't noticed wamr has a separate link logic for wasm-c-api. thank you. memory kind is not implemented there though.

Lucky, all those divergence will be merged in the "component model feature".

is there anyone working on it for wamr?

lum1n0us commented 2 years ago

is there anyone working on it for warm?

not for now.

yamt commented 2 years ago

WAMR is using a loading-time-link mechanism. It is similar with the process of ld searchs and loads all shared libraries used by a program. It is an old fashioned way and looks reasonable if there are only .wasm files until host-made runtime objects involved. wasm-c-api provides another way to link at instantiation wasm_instance_new().

i haven't noticed wamr has a separate link logic for wasm-c-api. thank you. memory kind is not implemented there though.

i looked at the wasm-c-api implementation a bit.

lum1n0us commented 2 years ago

Yes. WAMR tends to only link with functions now. And we may reconsider the design during implementing the Component Module proposal.

https://github.com/bytecodealliance/wasm-micro-runtime/issues/1168

yamt commented 2 years ago

The most reason of this choice is we want keep a private memory for every module. I guess there are some stories how an unsafe 3rd library successl peek user data inlight us.

in that case, it's better to make the request fail explicitly, rather than behaving in an incompatible way silently.

yamt commented 1 year ago

i want to use instantiate-time linking for my application. do you think it makes sense to introduce a WAMR_BUILD_xxx option for this?