spec icon indicating copy to clipboard operation
spec copied to clipboard

Question about linking.wast

Open yamt opened this issue 3 years ago • 7 comments

linking.wast has the following test. https://github.com/WebAssembly/spec/blob/f1229e10c774359355963d93694b48f225a008e1/test/core/linking.wast#L264-L275

does it mean that, while the instantiation attempt failed, the runtime should actually keep the partially-instantiated instance live so that the function $f can be executed via the funcref?

honestly speaking, i feel it's too much to require every engines to implement, especially for typical non-web engines.

also, i feel it's even dangerous to run partially-instantiated instances like this.

how do you think?

maybe this is related to this question: https://github.com/WebAssembly/spec/issues/1513

yamt avatar Sep 07 '22 11:09 yamt

The idea is that by the point active segments "run", the instance has notionally already been created - think of a failing active segment as like trapping in the middle of the start function, which also may require keeping the instance alive (e.g. if a function was already installed into an imported table).

Note that non-Web engines are still free to refuse to continue to execute any (collections of) modules that trap during instantiation (including in the start function), and they can take advantage of this to optimise their handling of active segments. The spec'd behaviour is only observable if the implementation allows traps to be recovered from and handled during runtime (which is the case for Web engines because of JS).

conrad-watt avatar Sep 07 '22 11:09 conrad-watt

The idea is that by the point active segments "run", the instance has notionally already been created - think of a failing active segment as like trapping in the middle of the start function, which also may require keeping the instance alive (e.g. if a function was already installed into an imported table).

if there are two failure modes, does it make sense for an engine to have an api to distinguish them? (i'm trying to make my engine pass all tests here)

does assert_trap always assert the failure mode where the instance has notionally been created?

Note that non-Web engines are still free to refuse to continue to execute any (collections of) modules that trap during instantiation (including in the start function), and they can take advantage of this to optimise their handling of active segments.

but in that case they can't run linking.wast, can they?

The spec'd behaviour is only observable if the implementation allows traps to be recovered from and handled during runtime (which is the case for Web engines because of JS).

if the test case applies to only a subset of implementations, it's better for the spec test not to have it, IMO. at least it should be marked so i guess.

yamt avatar Sep 07 '22 13:09 yamt

@yamt:

if there are two failure modes, does it make sense for an engine to have an api to distinguish them?

Probably. I guess it depends on what the API is trying to achieve.

does assert_trap always assert the failure mode where the instance has notionally been created?

Yes, it asserts an actual trap after execution has started. Any failure prior to execution is covered by assert_unlinkable.

Note that non-Web engines are still free to refuse to continue to execute any (collections of) modules that trap during instantiation (including in the start function), and they can take advantage of this to optimise their handling of active segments.

but in that case they can't run linking.wast, can they?

That depends on what you mean by "run". It will run, but abort with a trap. This is basically an instance of the larger design question where an engine does not enable recovering from a trap.

if the test case applies to only a subset of implementations, it's better for the spec test not to have it, IMO. at least it should be marked so i guess.

The test should apply to all engines, but some may not distinguish between assert_unlinkable and assert_trap during instantiation, which is a choice they are free to make (many engines do not distinguish between assert_malformed and assert_invalid, for example).

rossberg avatar Sep 12 '22 12:09 rossberg

@yamt:

if there are two failure modes, does it make sense for an engine to have an api to distinguish them?

Probably. I guess it depends on what the API is trying to achieve.

does assert_trap always assert the failure mode where the instance has notionally been created?

Yes, it asserts an actual trap after execution has started. Any failure prior to execution is covered by assert_unlinkable.

ok. i tweaked the api of my engine to make this case testable. https://github.com/yamt/toywasm/commit/7bacd5037ab2e644538b20c9c6cecf35af4ee3e4

Note that non-Web engines are still free to refuse to continue to execute any (collections of) modules that trap during instantiation (including in the start function), and they can take advantage of this to optimise their handling of active segments.

but in that case they can't run linking.wast, can they?

That depends on what you mean by "run". It will run, but abort with a trap. This is basically an instance of the larger design question where an engine does not enable recovering from a trap.

i meant that this test case seems relying on a particular way to recover from a trap during instantiation. that is, do not free the instance. i guess typical implementations free up the instance when instantiation attempt failed.

if the test case applies to only a subset of implementations, it's better for the spec test not to have it, IMO. at least it should be marked so i guess.

The test should apply to all engines, but some may not distinguish between assert_unlinkable and assert_trap during instantiation, which is a choice they are free to make (many engines do not distinguish between assert_malformed and assert_invalid, for example).

if an engine doesn't distinguish assert_unlinkable and assert_trap, how can it know if it can run the following invoke $Mt "call" (i32.const 7)? some kind of g/c which keeps the function and its corresponding module instance for the funcref is expected here?

yamt avatar Sep 13 '22 10:09 yamt

@yamt:

i meant that this test case seems relying on a particular way to recover from a trap during instantiation. that is, do not free the instance. i guess typical implementations free up the instance when instantiation attempt failed.

An implementation that can recover from traps cannot simply free this instance (at least not all of it). In general, engines that allow dynamic instantiation need some form of memory management for instances.

To put it differently, this test is equivalent to

   (module 
     (table $t (import "Mt" "tab") 10 funcref) 
     (func $f (result i32) (i32.const 0)) 
     (elem $e1 func $f) 
     (elem $e2 func $f $f $f $f $f)
     (func (export "init")
       (table.init $t $e1 (i32.const 7) (i32.const 0) (i32.const 1))
       (table.init $t $e2 (i32.const 8) (i32.const 0) (i32.const 5))
     )
   )
  (assert_trap (invoke "init") "out of bounds table access") 
  (assert_return (invoke $Mt "call" (i32.const 7)) (i32.const 0)) 

Any engine that supports dynamic instantiation and can handle this one should also be able to handle the other.

But you are right that an engine that can not recover from traps cannot really run this test because it is unobservable to it. But it still is relevant for engines where the behaviour is observable, so the test shouldn't be removed. Rather, the former engine should ignore the test. I believe there are various other tests in the test suite that such an engine cannot handle, for example, the directly preceding one.

rossberg avatar Sep 15 '22 15:09 rossberg

@yamt:

i meant that this test case seems relying on a particular way to recover from a trap during instantiation. that is, do not free the instance. i guess typical implementations free up the instance when instantiation attempt failed.

An implementation that can recover from traps cannot simply free this instance (at least not all of it). In general, engines that allow dynamic instantiation need some form of memory management for instances.

by memory management, do you mean g/c or equivalent, which would prevent the instance from being freed because it's still referenced by the funcref left in the table?

To put it differently, this test is equivalent to

   (module 
     (table $t (import "Mt" "tab") 10 funcref) 
     (func $f (result i32) (i32.const 0)) 
     (elem $e1 func $f) 
     (elem $e2 func $f $f $f $f $f)
     (func (export "init")
       (table.init $t $e1 (i32.const 7) (i32.const 0) (i32.const 1))
       (table.init $t $e2 (i32.const 8) (i32.const 0) (i32.const 5))
     )
   )
  (assert_trap (invoke "init") "out of bounds table access") 
  (assert_return (invoke $Mt "call" (i32.const 7)) (i32.const 0)) 

Any engine that supports dynamic instantiation and can handle this one should also be able to handle the other.

if g/c is assumed, i agree.

But you are right that an engine that can not recover from traps cannot really run this test because it is unobservable to it. But it still is relevant for engines where the behaviour is observable, so the test shouldn't be removed. Rather, the former engine should ignore the test. I believe there are various other tests in the test suite that such an engine cannot handle, for example, the directly preceding one.

do you mean this one? https://github.com/WebAssembly/spec/blob/f1229e10c774359355963d93694b48f225a008e1/test/core/linking.wast#L252-L262 this one seems non-problem as there is no side effects of the failed attempt of the instantiation.

but i agree there are other tests in the suite which such engines can't handle.

yamt avatar Sep 17 '22 11:09 yamt

An implementation that can recover from traps cannot simply free this instance (at least not all of it). In general, engines that allow dynamic instantiation need some form of memory management for instances.

by memory management, do you mean g/c or equivalent, which would prevent the instance from being freed because it's still referenced by the funcref left in the table?

Yes.

Any engine that supports dynamic instantiation and can handle this one should also be able to handle the other.

if g/c is assumed, i agree.

It's not an assumption, the semantics demands that it works, so keeping the instance alive long enough becomes a practical requirement. GC or RC are possible solutions, never collecting instances a lamer one.

this one seems non-problem as there is no side effects of the failed attempt of the instantiation.

That there is no side effect does not matter, it still requires executing something successfully after an instantiation has failed. An implementation that aborts on failed instantiations won't be able to run it.

rossberg avatar Sep 20 '22 06:09 rossberg

An implementation that can recover from traps cannot simply free this instance (at least not all of it). In general, engines that allow dynamic instantiation need some form of memory management for instances.

by memory management, do you mean g/c or equivalent, which would prevent the instance from being freed because it's still referenced by the funcref left in the table?

Yes.

Any engine that supports dynamic instantiation and can handle this one should also be able to handle the other.

if g/c is assumed, i agree.

It's not an assumption, the semantics demands that it works, so keeping the instance alive long enough becomes a practical requirement. GC or RC are possible solutions, never collecting instances a lamer one.

ok.

this one seems non-problem as there is no side effects of the failed attempt of the instantiation.

That there is no side effect does not matter, it still requires executing something successfully after an instantiation has failed. An implementation that aborts on failed instantiations won't be able to run it.

if an implementation aborts the whole engine, sure. but i guess it's more common to only abort the particular instance which failed to instantiate.

yamt avatar Sep 26 '22 07:09 yamt

thank you for explanation. i think i understand the intention and the situation. please feel free to close this. (i'm not sure if it's appropriate to close it by myself.)

yamt avatar Sep 26 '22 07:09 yamt