Implement return_call_ref, ref.as_non_null, br_on_[non_]null instructions

[zherczeg]

Depends on #2562

Another 1000 lines addition. Depends on the previous work. Currently implements 3 opcodes, but I might add the remaining opcode (return_call_ref).

[sbc100]

Which proposal are these instruction part of?

[zherczeg]

Thank you for checking the patch.

These are the instructions: https://github.com/WebAssembly/function-references/blob/main/proposals/function-references/Overview.md#instructions-1

I am working on function references phase 5 proposal at the moment, which is requirement for the (latest) GC proposal. The work is split into two parts:

#2562 adds the concepts of (ref ...) and (ref null ...) to many parts of the engine. The big change is that references has an extra argument, and the code needs to handle these non primitive types.

This patch adds the instructions from the proposal.

This is the generic testsuite, also used by wabt: https://github.com/WebAssembly/testsuite/tree/main/proposals/function-references

My aim is to make all test pass in this directory.

[zherczeg]

What can I do with this test on s390x:

- test/spec/function-references/return_call_ref.txt
    TIMEOUT

The test is also slow on x86. It does a lot of recursion. I did not write the test, it is a spec test.

[SoniEx2]

you might be able to add ;;; SLOW: to it. hopefully that gives it enough time to run.

[zherczeg]

Great idea! Thanks it worked!

[zherczeg]

This patch is also updated. It is top of #2571 / #2562 It is a large patch, but easy to review. Still missing: tables with initializers. I plan to do it like globals, since a table in theory is a global with multiple elements. The initializers returns with the default element.

[zherczeg]

Ok the fuzzer using this patch found another issue in the code, which is not related to this patch.

Example code: (module (global $a i32 (i32.const -2)) )

The compiled code is

0000000: 0061 736d                                 ; WASM_BINARY_MAGIC
0000004: 0100 0000                                 ; WASM_BINARY_VERSION
; section "Global" (6)
0000008: 06                                        ; section code
0000009: 00                                        ; section size (guess)
000000a: 01                                        ; num globals
000000b: 7f                                        ; i32
000000c: 00                                        ; global mutability
000000d: 41                                        ; i32.const
000000e: 7e                                        ; i32 literal
000000f: 0b                                        ; end
0000009: 06                                        ; FIXUP section size

With a hexeditor I changed the 0x41 (i32.const) to 0x11 (call_indirect). Then I run objdump:

wasm-objdump: .../wabt/wabt/src/binary-reader-objdump.cc:661: void wabt::{anonymous}::BinaryReaderObjdumpDisassemble::LogOpcode(const char*, ...): Assertion `in_function_body' failed.

The reason is that init expressions are not validated by objdump. If I change 0x41 to 0x45 (i32.eqz), there is no crash, since the in_function_body is not checked by simple opcodes. But this code has an assert check for whatever reason.

What shall we do?

[zherczeg]

My suggestion: I suspect objdump does not validate the code intentionally, just dumps it. Then validation related sanity checks does not make sense, so maybe setting in_function_body to true even for init expressions is a valid solution.