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.

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

Recursive continuation type crashes the interpreter #17

Closed
dhil opened this issue Jan 16, 2024 · 5 comments
Closed

Recursive continuation type crashes the interpreter #17

dhil opened this issue Jan 16, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@dhil
Copy link
Member

dhil commented Jan 16, 2024

Using a recursive continuation type triggers an assertion failure in the interpreter. Minimal repo:

(module
  (type $ct (cont $ct))
  (func (param $k (ref $ct))
    (cont.bind $ct $ct (local.get $k))
    (drop)
  )
)

Output:

./wasm: uncaught exception File "valid/valid.ml", line 107, characters 16-22: Assertion failed
Raised at Wasm__Valid.func_type_of_heap_type in file "valid/valid.ml", line 107, characters 16-28
Called from Wasm__Valid.func_type_of_cont_type in file "valid/valid.ml" (inlined), line 112, characters 2-32
Called from Wasm__Valid.check_instr in file "valid/valid.ml", line 630, characters 27-59
Called from Wasm__Valid.check_seq in file "valid/valid.ml", line 1025, characters 26-43
Called from Wasm__Valid.check_block in file "valid/valid.ml", line 1030, characters 15-41
Called from Stdlib__List.iter in file "list.ml", line 112, characters 12-15
Called from Wasm__Valid.check_module in file "valid/valid.ml", line 1214, characters 2-42
Called from Wasm__Run.run_command in file "script/run.ml", line 518, characters 6-26
Called from Stdlib__List.iter in file "list.ml", line 112, characters 12-15
Called from Wasm__Run.run_script in file "script/run.ml" (inlined), line 588, characters 2-30
Called from Wasm__Run.run_quote_script in file "script/run.ml", line 593, characters 7-24
Re-raised at Wasm__Run.run_quote_script in file "script/run.ml", line 593, characters 58-67
Called from Wasm__Run.input_from in file "script/run.ml", line 104, characters 4-14
Called from Wasm__Run.input_sexpr_file in file "script/run.ml", line 144, characters 18-39
Re-raised at Wasm__Run.input_sexpr_file in file "script/run.ml", line 147, characters 27-36
Called from Wasm__Run.run_meta in file "script/run.ml", line 565, characters 16-50
Called from Stdlib__List.iter in file "list.ml", line 112, characters 12-15
Called from Wasm__Run.input_from in file "script/run.ml", line 104, characters 4-14
Called from Dune__exe__Wasm.(fun) in file "wasm.ml", line 45, characters 33-53
Called from Stdlib__List.iter in file "list.ml", line 112, characters 12-15
Called from Dune__exe__Wasm in file "wasm.ml", line 45, characters 4-72
@dhil dhil added the bug Something isn't working label Jan 16, 2024
@dhil
Copy link
Member Author

dhil commented Jan 25, 2024

@rossberg, I would expect this program to fail due to $ct not being a well-formed type. A well-formed continuation type is always parameterised by a valid function type. But looking at the reference interpreter, we never seem to check whether types are well-formed.

I noticed it is possible to define a function type in terms of itself, e.g. the following program type checks and runs:

(module
  (type $ft (func (param (ref $ft))))
  (func $f (param $f (ref $ft))
    (call_ref $ft
      (local.get $f)
      (local.get $f)))
  (elem declare func $f)
  (func (export "main")
    (call $f (ref.func $f)))
)
(assert_exhaustion (invoke "main") "call stack exhausted")

This construction leads me to think that singly typedefs are implicitly recursive. Is this construction intended?

@rossberg
Copy link

A type without rec is simply a short-hand for a singleton recursion group, so the latter is well-typed.

As for the OP, I'm surprised the type definition passes validation. AFAICS, we do check type definitions, and we do check that cont is given a function type.

@rossberg
Copy link

I was looking at the version in the effect-handlers repo, the one here does indeed seem to miss that check:

| ContT ft -> check_heap_type c ft at

@dhil
Copy link
Member Author

dhil commented Jan 25, 2024

aha, so this has been lost in a merge sometime ago. I will reinstall the check. Thanks!

dhil added a commit to dhil/webassembly-spec that referenced this issue Jan 25, 2024
This patch (re)installs the check for continuation type
well-formedness.
@dhil dhil closed this as completed in a9ec313 Jan 25, 2024
dhil pushed a commit that referenced this issue Jul 31, 2024
* [interpreter] Handle custom sections and annotations

Co-authored-by: Yuri Iozzelli <[email protected]>

* Fix merge conflict

* Fix lexer priorities

* Fix wast.ml

* Oops

* Update wast.ml

---------

Co-authored-by: Andreas Rossberg <[email protected]>
dhil pushed a commit that referenced this issue Jul 31, 2024
* [interpreter] Handle custom sections and annotations

Co-authored-by: Yuri Iozzelli <[email protected]>

* Fix merge conflict

* Fix lexer priorities

* Fix wast.ml

* Oops

* Update wast.ml

---------

Co-authored-by: Andreas Rossberg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants