Skip to content
This repository was archived by the owner on Nov 3, 2021. It is now read-only.

Bug in reference core test #114

Closed
q82419 opened this issue Sep 22, 2020 · 4 comments
Closed

Bug in reference core test #114

q82419 opened this issue Sep 22, 2020 · 4 comments

Comments

@q82419
Copy link
Contributor

q82419 commented Sep 22, 2020

If I'm correct, the test case in core/ref_func.wast should not pass.
https://github.com/WebAssembly/reference-types/blob/master/test/core/ref_func.wast#L80-L106

The following test case got an expected invalid result:

(assert_invalid
  (module (func $f (drop (ref.func $f))))
  "undeclared function reference"
)

Because the $f is not declared in elem.

I think it needs to add (elem declare func $f1 $f2) or (elem declare funcref (ref.func $f1) (ref.func $f2)) into the module in the above mentioned test case?
That is:


(module
  (func $f1)
  (func $f2)
  (func $f3)
  (func $f4)
  (func $f5)
  (func $f6)

  (table $t 1 funcref)

  (global funcref (ref.func $f1))
  (export "f" (func $f2))
  (elem declare funcref (ref.func $f1) (ref.func $f2))
  (elem (table $t) (i32.const 0) func $f3)
  (elem (table $t) (i32.const 0) funcref (ref.func $f4))
  (elem func $f5)
  (elem funcref (ref.func $f6))

  (func
    (ref.func $f1)
    (ref.func $f2)
    (ref.func $f3)
    (ref.func $f4)
    (ref.func $f5)
    (ref.func $f6)
    (return)
  )
)
@gahaas
Copy link
Contributor

gahaas commented Sep 22, 2020

The point of this test is that this is no longer needed, per the resolution of #76. The occurrences in the other declarations are sufficient.

This was the reply to the same question in #95

@q82419
Copy link
Contributor Author

q82419 commented Sep 22, 2020

Got it, thanks.
All ocurrences of ref.func in the module environment are treated as declaring a valid ref.func target for the code
section.
In the above module, the export and global section reference to $f1 and $f2, so the (elem declare func $f1 $f2) is not needed. Right?

@gahaas
Copy link
Contributor

gahaas commented Sep 22, 2020

As far as I understand, it's not needed.

@binji
Copy link
Member

binji commented Sep 22, 2020

Yes, that's correct.

@q82419 q82419 closed this as completed Sep 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants