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

The definition of validation of br_table is inadequate #1708

Closed
r1ru opened this issue Nov 21, 2023 · 3 comments · Fixed by #1709
Closed

The definition of validation of br_table is inadequate #1708

r1ru opened this issue Nov 21, 2023 · 3 comments · Fixed by #1709

Comments

@r1ru
Copy link
Contributor

r1ru commented Nov 21, 2023

The spec defines the validation of br_table as follows and I think this definition is inadequate.

validate_br_table

When I first saw this definition, I took it to mean "all labels have the same type". However, I realised my mistake when I saw the following test in unreached-valid.wast.

https://github.com/WebAssembly/spec/blob/main/test/core/unreached-valid.wast#L49

unreached-valid

In my first interpretation, this module is invalid because [f32] and [f64] are not the same.
In order for this module to be valid, t* must be something like [f32 f64].

The problem is that it is unclear what is intended by "corresponding" in the spec.

@rossberg
Copy link
Member

rossberg commented Nov 21, 2023

The problem is that it is unclear what is intended by "corresponding" in the spec.

That is the type in the j-th position, i.e., 𝑡_𝑗 and 𝑡'_𝑖𝑗 are in the same position in their respective vectors. That wording is used in other places of the spec as well. I suppose it is unclear here due to the missing precondition that 𝑡* and C.labels[𝑙_𝑖] have the same length. Please see #1709 for a fix.

@conrad-watt
Copy link
Contributor

Just to unpack this a little more, instead of "all labels have the same type" it would be more precise to say "all labels are supertypes of the current stack type" - the reason that the linked test validates is because in unreachable code, there's a conceptual "bottom" type which is a subtype of both f32 and f64, so effectively the current stack is typed as [⊥], and we have [⊥] <: [f32] and [⊥] <: [f64].

In particular the following wouldn't validate:

(module
  (func (export "meet?")
    (block (result f64)
      (block (result f32)
        (f32.const 0) (f64.const 0)
        (br_table 0 1 1 (i32.const 1))
      )
      (drop)
      (f64.const 0)
    )
    (drop)
  )
)

@r1ru
Copy link
Contributor Author

r1ru commented Nov 21, 2023

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants