Skip to content

Commit

Permalink
[EH][GC] Send a non-nullable exnref from TryTable (#7013)
Browse files Browse the repository at this point in the history
When EH+GC are enabled then wasm has non-nullable types, and the
sent exnref should be non-nullable. In BinaryenIR we use the non-
nullable type all the time, which we also do for function references
and other things; we lower it if GC is not enabled to a nullable type
for the binary format (see `WasmBinaryWriter::writeType`, to which
comments were added in this PR). That is, this PR makes us handle
exnref the same as those other types.

A new test verifies that behavior. Various existing tests are updated
because ReFinalize will now use the more refined type, so this is
an optimization. It is also a bugfix as in #6987 we started to emit
the refined form in the fuzzer, and this PR makes us handle it
properly in validation and ReFinalization.
  • Loading branch information
kripken authored Oct 17, 2024
1 parent fd86ead commit 6566329
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 76 deletions.
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,16 @@ There are a few differences between Binaryen IR and the WebAssembly language:
much about this when writing Binaryen passes. For more details see the
`requiresNonNullableLocalFixups()` hook in `pass.h` and the
`LocalStructuralDominance` class.
* Binaryen IR uses the most refined types possible for references,
specifically:
* The IR type of a `ref.func` is always a specific function type, and not
plain `funcref`. It is also non-nullable.
* Non-nullable types are also used for the type that `try_table` sends
on branches (if we branch, a null is never sent), that is, it sends
(ref exn) and not (ref null exn).
In both cases if GC is not enabled then we emit the less-refined type in the
binary. When reading a binary, the more refined types will be applied as we
build the IR.
* `br_if` output types are more refined in Binaryen IR: they have the type of
the value, when a value flows in. In the wasm spec the type is that of the
branch target, which may be less refined. Using the more refined type here
Expand Down
8 changes: 5 additions & 3 deletions src/wasm/wasm-binary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1544,9 +1544,9 @@ void WasmBinaryWriter::writeInlineBuffer(const char* data, size_t size) {

void WasmBinaryWriter::writeType(Type type) {
if (type.isRef()) {
// The only reference types allowed without GC are funcref and externref. We
// internally use more refined versions of those types, but we cannot emit
// those more refined types.
// The only reference types allowed without GC are funcref, externref, and
// exnref. We internally use more refined versions of those types, but we
// cannot emit those without GC.
if (!wasm->features.hasGC()) {
auto ht = type.getHeapType();
if (ht.isMaybeShared(HeapType::string)) {
Expand All @@ -1555,6 +1555,8 @@ void WasmBinaryWriter::writeType(Type type) {
// string, the stringref feature must be enabled.
type = Type(HeapTypes::string.getBasic(ht.getShared()), Nullable);
} else {
// Only the top type (func, extern, exn) is available, and only the
// nullable version.
type = Type(type.getHeapType().getTop(), Nullable);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/wasm/wasm-validator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2658,7 +2658,7 @@ void FunctionValidator::visitTryTable(TryTable* curr) {
"the number of catch tags and sent types do not match");

const char* invalidSentTypeMsg = "invalid catch sent type information";
Type exnref = Type(HeapType::exn, Nullable);
Type exnref = Type(HeapType::exn, NonNullable);
for (Index i = 0; i < curr->catchTags.size(); i++) {
auto sentType = curr->sentTypes[i];
size_t tagTypeSize;
Expand Down
6 changes: 5 additions & 1 deletion src/wasm/wasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,11 @@ static void populateTryTableSentTypes(TryTable* curr, Module* wasm) {
return;
}
curr->sentTypes.clear();
Type exnref = Type(HeapType::exn, Nullable);
// We always use the refined non-nullable type in our IR, which is what the
// wasm spec defines when GC is enabled (=== non-nullable types are allowed).
// If GC is not enabled then we emit a nullable type in the binary format in
// WasmBinaryWriter::writeType.
Type exnref = Type(HeapType::exn, NonNullable);
for (Index i = 0; i < curr->catchTags.size(); i++) {
auto tagName = curr->catchTags[i];
std::vector<Type> sentType;
Expand Down
27 changes: 27 additions & 0 deletions test/lit/basic/exception-handling-no-gc.wast
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.

;; Test that we do not emit an invalid (ref exn) when exceptions are enabled
;; but not GC. GC is required for us to be non-nullable.

;; RUN: wasm-opt %s --enable-reference-types --enable-exception-handling --disable-gc --roundtrip -S -o - | filecheck %s

(module
;; CHECK: (func $test (result exnref)
;; CHECK-NEXT: (block $label$1 (result exnref)
;; CHECK-NEXT: (try_table (catch_all_ref $label$1)
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $test (result exnref)
;; It is valid to write (ref exn) in Binaryen IR, and internally that is how
;; we represent things, but when we emit the binary we emit a nullable type,
;; so after the roundtrip we are less refined.
(block $label (result (ref exn))
(try_table (catch_all_ref $label)
(unreachable)
)
)
)
)

2 changes: 1 addition & 1 deletion test/lit/passes/merge-blocks-eh.wast
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@

;; CHECK: (func $drop-block-try_catch_multi_partial (type $0)
;; CHECK-NEXT: (tuple.drop 2
;; CHECK-NEXT: (block $outer (type $1) (result i32 exnref)
;; CHECK-NEXT: (block $outer (type $2) (result i32 (ref exn))
;; CHECK-NEXT: (block $inner
;; CHECK-NEXT: (try_table (catch_ref $i32 $outer) (catch_all $inner)
;; CHECK-NEXT: (call $import)
Expand Down
Loading

0 comments on commit 6566329

Please sign in to comment.