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

zherczeg · 2025-03-20T08:31:53Z

Followup to #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 · 2025-03-20T17:54:29Z

Which proposal are these instruction part of?

zherczeg · 2025-03-21T05:58:42Z

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 · 2025-03-21T07:40:10Z

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 · 2025-03-21T11:45:32Z

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

zherczeg · 2025-03-21T14:01:18Z

Great idea! Thanks it worked!

zherczeg · 2025-04-05T06:29:08Z

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 · 2025-04-25T06:46:58Z

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 · 2025-04-25T07:14:32Z

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.

Fixes the fuzzer bug in #2567 I noticed all objdump logs are guarded by `if (!in_function_body)`. The code crashes without this check.

…ions

zherczeg · 2025-11-08T04:06:49Z

src/shared-validator.cc

+  FuncType func_type;
+  result |= typechecker_.OnReturnCallRef(function_type_var.to_type());
+  result |= CheckFuncTypeIndex(function_type_var, &func_type);
+  result |= typechecker_.OnReturnCall(func_type.params, func_type.results);


This code is based on the original implementation in 9aac0a2, although OnReturnCallIndirect uses its own function which report errors as "call_indirect" rather than "call". Maybe adding a string "descriptor" argument would be better than duplicating the whole function. Call indirect has the code duplication as well, perhaps we could simplify the whole thing with the string argument.

Seems like a good idea, could certainly be in a followup too

sbc100 · 2025-11-08T21:00:09Z

src/apply-names.cc

+Result NameApplier::OnBrOnNullExpr(BrOnNullExpr* expr) {
+  std::string_view label = FindLabelByVar(&expr->var);
+  UseNameForVar(label, &expr->var);
+  return Result::Ok;


Unrelated to this PR but this seems to be a repeated pattern here.. i wonder if we could refactor/tempetize/macro somehow?

True. I will do it.

sbc100 · 2025-11-08T21:06:50Z

src/shared-validator.cc

+  FuncType func_type;
+  result |= typechecker_.OnReturnCallRef(function_type_var.to_type());
+  result |= CheckFuncTypeIndex(function_type_var, &func_type);
+  result |= typechecker_.OnReturnCall(func_type.params, func_type.results);


Seems like a good idea, could certainly be in a followup too

This is just a small set of changes which should follow #2567.

zherczeg changed the title ~~Implement ref.as_non_null, br_non_null, br_on_non_null instructions~~ Implement return_call_ref, ref.as_non_null, br_on_[non_]null instructions Mar 21, 2025

zherczeg force-pushed the more_instructions branch 2 times, most recently from 92898bd to bce48df Compare March 21, 2025 07:10

zherczeg marked this pull request as ready for review March 21, 2025 07:21

zherczeg mentioned this pull request Mar 21, 2025

Add support for (ref ...) / (ref null ...) constructs #2562

Merged

zherczeg force-pushed the more_instructions branch from bce48df to 758858b Compare March 21, 2025 13:24

zherczeg force-pushed the more_instructions branch from 758858b to ef7290e Compare April 5, 2025 06:02

zherczeg force-pushed the more_instructions branch 2 times, most recently from 1184506 to 10500a7 Compare April 25, 2025 06:07

zherczeg mentioned this pull request Apr 25, 2025

Add missing in_function_body check to objdump #2591

Merged

sbc100 pushed a commit that referenced this pull request Apr 25, 2025

Add missing in_function_body check to objdump (#2591)

6572796

Fixes the fuzzer bug in #2567 I noticed all objdump logs are guarded by `if (!in_function_body)`. The code crashes without this check.

zherczeg force-pushed the more_instructions branch 2 times, most recently from ec09f77 to 69054bf Compare April 28, 2025 04:57

zherczeg force-pushed the more_instructions branch 3 times, most recently from 13fc7b7 to d03887c Compare May 8, 2025 10:29

zherczeg mentioned this pull request May 8, 2025

Support table initializer expressions; completes function references support #2593

Merged

zherczeg force-pushed the more_instructions branch 3 times, most recently from 2c092e7 to 126985f Compare May 14, 2025 07:54

zherczeg force-pushed the more_instructions branch 2 times, most recently from ab7cd3c to c652aee Compare May 22, 2025 03:14

zherczeg force-pushed the more_instructions branch from c652aee to 5855654 Compare May 22, 2025 05:01

zherczeg force-pushed the more_instructions branch 2 times, most recently from 5017a02 to 9ab4bd1 Compare June 3, 2025 10:43

zherczeg force-pushed the more_instructions branch 4 times, most recently from 8bb32da to 1e29a04 Compare June 22, 2025 18:14

zherczeg force-pushed the more_instructions branch 2 times, most recently from 80f22fa to 073a9d8 Compare June 26, 2025 14:35

Clashsoft mentioned this pull request Aug 14, 2025

Function References: Implement return_call_ref, ref.as_non_null, br_on_[non_]null instructions ds-ukassel/wabt#2

Merged

Implement return_call_ref, ref.as_non_null, br_on_[non_]null instruct…

528f5a0

…ions

zherczeg force-pushed the more_instructions branch from 073a9d8 to 528f5a0 Compare November 8, 2025 03:42

zherczeg commented Nov 8, 2025

View reviewed changes

sbc100 approved these changes Nov 8, 2025

View reviewed changes

zherczeg mentioned this pull request Nov 9, 2025

Code improvements for call ref and br_on_[non_]null #2663

Merged

sbc100 merged commit ed94ed7 into WebAssembly:main Nov 9, 2025
17 checks passed

zherczeg deleted the more_instructions branch November 9, 2025 06:51

sbc100 pushed a commit that referenced this pull request Nov 16, 2025

Code improvements for call ref and br_on_[non_]null (#2663)

d8184ba

This is just a small set of changes which should follow #2567.

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

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

Uh oh!

Conversation

zherczeg commented Mar 20, 2025 • edited by sbc100 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbc100 commented Mar 20, 2025

Uh oh!

zherczeg commented Mar 21, 2025

Uh oh!

zherczeg commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SoniEx2 commented Mar 21, 2025

Uh oh!

zherczeg commented Mar 21, 2025

Uh oh!

zherczeg commented Apr 5, 2025

Uh oh!

zherczeg commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zherczeg commented Apr 25, 2025

Uh oh!

zherczeg Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbc100 Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

sbc100 Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

zherczeg Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

sbc100 Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zherczeg commented Mar 20, 2025 •

edited by sbc100

Loading

zherczeg commented Mar 21, 2025 •

edited

Loading

zherczeg commented Apr 25, 2025 •

edited

Loading

zherczeg Nov 8, 2025 •

edited

Loading