Skip to content

Commit d29db53

Browse files
woesswoess
committed
Address review feedback and fix bugs in exception handling.
Fix reading more than one exception handler. Fix exn_read: should return exception fields, not the tag. Add exn_tag. Fix tag count limit. Fix THROW bytecode length. Simplify ParserState.exit(), moving out TryTableFrame handling.
1 parent 64af7fb commit d29db53

File tree

16 files changed

+95
-67
lines changed

16 files changed

+95
-67
lines changed

‎wasm/src/org.graalvm.wasm.test/src/org/graalvm/wasm/test/suites/bytecode/MultiInstantiationSuite.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,16 @@ public void testImportsAndExports() throws IOException, InterruptedException {
232232
final Object e = WebAssembly.instanceExport(i, "e");
233233
final Object exception = WebAssembly.instanceExport(i, "exception");
234234
final Object eInstance = lib.execute(exception);
235-
final Object tagRead = wasm.readMember("exn_read");
236-
final Object eInstanceTag = lib.execute(tagRead, eInstance);
235+
236+
final Object exnTag = wasm.readMember("exn_tag");
237+
final Object eInstanceTag = lib.execute(exnTag, eInstance);
237238
Assert.assertSame("Exception tag does not match", e, eInstanceTag);
238239

240+
final Object exnRead = wasm.readMember("exn_read");
241+
final Object eInstanceFields = lib.execute(exnRead, eInstance);
242+
Assert.assertTrue("Exception fields is not an array", lib.hasArrayElements(eInstanceFields));
243+
Assert.assertEquals("Exception fields array size", 0, lib.getArraySize(eInstanceFields));
244+
239245
final Object test = WebAssembly.instanceExport(i, "test");
240246
final int result = lib.asInt(lib.execute(test));
241247
Assert.assertEquals("Invalid test value", 64, result);

‎wasm/src/org.graalvm.wasm.test/src/test/exceptions/exceptions_with_params.wat‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
block $h (result i32)
4848
try_table (result i32) (catch $e0 $h)
4949
block $h1 (result i32)
50-
try_table (result i32) (catch $e1 $h)
50+
try_table (result i32) (catch $e1 $h1)
5151
i32.const 4
5252
throw $e0
5353
i32.const 42

‎wasm/src/org.graalvm.wasm.test/src/test/exceptions/throw_ref.wat‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
(type $t0 (func))
4343

4444
(tag $e0 (type $t0))
45-
(tag $e1 (type $t0))
4645

4746
(func $throw_e0 (param i32)
4847
local.get 0

‎wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/BinaryParser.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
import org.graalvm.wasm.api.Vector128Shape;
7979
import org.graalvm.wasm.collection.ByteArrayList;
8080
import org.graalvm.wasm.constants.Bytecode;
81+
import org.graalvm.wasm.constants.BytecodeBitEncoding;
8182
import org.graalvm.wasm.constants.ExceptionHandlerType;
8283
import org.graalvm.wasm.constants.ExportIdentifier;
8384
import org.graalvm.wasm.constants.GlobalModifier;
@@ -1103,9 +1104,10 @@ private CodeEntry readFunction(int functionIndex, byte[] locals, byte[] resultTy
11031104
final int exceptionTableOffset;
11041105
if (state.needsExceptionTable()) {
11051106
exceptionTableOffset = bytecode.location();
1107+
assert exceptionTableOffset != BytecodeBitEncoding.INVALID_EXCEPTION_TABLE_OFFSET;
11061108
state.generateExceptionTable();
11071109
} else {
1108-
exceptionTableOffset = 0;
1110+
exceptionTableOffset = BytecodeBitEncoding.INVALID_EXCEPTION_TABLE_OFFSET;
11091111
}
11101112

11111113
if (offsetToLineIndexMap == null) {
@@ -2504,7 +2506,7 @@ private ExceptionHandler[] readExceptionHandlers(ParserState state) {
25042506
final int length = readLength();
25052507
final ExceptionHandler[] handlers = new ExceptionHandler[length];
25062508

2507-
for (int i = 0; i < length; i += 2) {
2509+
for (int i = 0; i < length; i++) {
25082510
final int opcode = read1() & 0xFF;
25092511
switch (opcode) {
25102512
case ExceptionHandlerType.CATCH -> {

‎wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/ModuleLimits.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,8 @@ public void checkMemoryInstanceSize(long size, boolean indexType64) {
201201
}
202202
}
203203

204-
public void checkTagCount(int size) {
205-
assertUnsignedIntLessOrEqual(size, tagCountLimit, Failure.TAG_COUNT_LIMIT_EXCEEDED);
204+
public void checkTagCount(int count) {
205+
assertUnsignedIntLessOrEqual(count, tagCountLimit, Failure.TAG_COUNT_LIMIT_EXCEEDED);
206206
}
207207

208208
public int tableInstanceSizeLimit() {

‎wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/SymbolTable.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1199,7 +1199,7 @@ public void exportTag(int tagIndex, String name) {
11991199
checkNotParsed();
12001200
exportSymbol(name);
12011201
if (!checkExistingTagIndex(tagIndex)) {
1202-
throw WasmException.create(Failure.UNSPECIFIED_INVALID, "No tag with the specified index has been declared or imported, so it cannot be exported.");
1202+
throw WasmException.create(Failure.UNKNOWN_TAG, "No tag with the specified index has been declared or imported, so it cannot be exported.");
12031203
}
12041204
exportedTags.put(name, tagIndex);
12051205
module().addLinkAction((context, store, instance, imports) -> {

‎wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/api/FuncType.java‎

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -66,29 +66,22 @@ public static FuncType fromString(String s) {
6666
if (leftPar == -1 || rightPar == -1) {
6767
throw new WasmJsApiException(WasmJsApiException.Kind.TypeError, "Invalid function type format");
6868
}
69-
final String paramTypesString = s.substring(leftPar + 1, rightPar);
70-
final ValueType[] params;
71-
if (paramTypesString.isEmpty()) {
72-
params = EMTPY;
73-
} else {
74-
final String[] paramTypes = s.substring(leftPar + 1, rightPar).split("\\s");
75-
params = new ValueType[paramTypes.length];
76-
for (int i = 0; i < paramTypes.length; i++) {
77-
params[i] = ValueType.valueOf(paramTypes[i]);
78-
}
79-
}
80-
final String resultTypesString = s.substring(rightPar + 1);
81-
final ValueType[] results;
82-
if (resultTypesString.isEmpty()) {
83-
results = EMTPY;
69+
final ValueType[] params = parseTypeString(s, leftPar + 1, rightPar);
70+
final ValueType[] results = parseTypeString(s, rightPar + 1, s.length());
71+
return new FuncType(params, results);
72+
}
73+
74+
private static ValueType[] parseTypeString(String typesString, int start, int end) {
75+
if (start >= end) {
76+
return EMTPY;
8477
} else {
85-
final String[] resultTypes = s.substring(rightPar + 1).split("\\s");
86-
results = new ValueType[resultTypes.length];
87-
for (int i = 0; i < resultTypes.length; i++) {
88-
results[i] = ValueType.valueOf(resultTypes[i]);
78+
String[] typeNames = typesString.substring(start, end).split(" ");
79+
ValueType[] types = new ValueType[typeNames.length];
80+
for (int i = 0; i < typeNames.length; i++) {
81+
types[i] = ValueType.valueOf(typeNames[i]);
8982
}
83+
return types;
9084
}
91-
return new FuncType(params, results);
9285
}
9386

9487
public static FuncType fromFunctionType(SymbolTable.FunctionType functionType) {

‎wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/api/JsConstants.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ private JsConstants() {
6464
private static final int LOCAL_COUNT_LIMIT = 50000;
6565
private static final int TABLE_SIZE_LIMIT = 10000000;
6666
private static final int MEMORY_SIZE_LIMIT = 65536;
67-
private static final int TAG_COUNT_LIMIT = 10000000;
67+
private static final int TAG_COUNT_LIMIT = 1_000_000;
6868

6969
public static final ModuleLimits JS_LIMITS = new ModuleLimits(
7070
MODULE_SIZE_LIMIT,

‎wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/api/WebAssembly.java‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ public WebAssembly(WasmContext currentContext) {
119119
addMember("tag_type", new Executable(WebAssembly::tagType));
120120

121121
addMember("exn_alloc", new Executable(this::exnAlloc));
122+
addMember("exn_tag", new Executable(WebAssembly::exnTag));
122123
addMember("exn_read", new Executable(WebAssembly::exnRead));
123124

124125
addMember("module_imports", new Executable(WebAssembly::moduleImports));
@@ -445,7 +446,7 @@ public WasmTable tableAlloc(int initial, int maximum, TableKind elemKind, Object
445446
throw new WasmJsApiException(WasmJsApiException.Kind.TypeError, "Element type must be a reftype");
446447
}
447448
if (!refTypes && (elemKind == TableKind.externref || elemKind == TableKind.exnref)) {
448-
throw new WasmJsApiException(WasmJsApiException.Kind.TypeError, "Element type must be anyfunc. Enable reference types to support externref");
449+
throw new WasmJsApiException(WasmJsApiException.Kind.TypeError, "Element type must be anyfunc. Enable wasm.BulkMemoryAndRefTypes to support other reference types");
449450
}
450451
final int maxAllowedSize = minUnsigned(maximum, JS_LIMITS.tableInstanceSizeLimit());
451452
return new WasmTable(initial, maximum, maxAllowedSize, elemKind.byteValue(), initialValue);
@@ -1004,14 +1005,24 @@ public Object exnAlloc(Object[] args) {
10041005
return new WasmRuntimeException(null, tag, fields);
10051006
}
10061007

1007-
public static Object exnRead(Object[] args) {
1008+
public static Object exnTag(Object[] args) {
10081009
checkArgumentCount(args, 1);
10091010
if (!(args[0] instanceof WasmRuntimeException exn)) {
1010-
throw new WasmJsApiException(WasmJsApiException.Kind.TypeError, "First argument must be a wasm tag");
1011+
throw new WasmJsApiException(WasmJsApiException.Kind.TypeError, "First argument must be a wasm exception");
10111012
}
10121013
return exn.tag();
10131014
}
10141015

1016+
public static Object exnRead(Object[] args) {
1017+
checkArgumentCount(args, 1);
1018+
if (!(args[0] instanceof WasmRuntimeException exn)) {
1019+
throw new WasmJsApiException(WasmJsApiException.Kind.TypeError, "First argument must be a wasm exception");
1020+
}
1021+
// Should return exn.fields.
1022+
// WasmRuntimeException already exposes its fields as array elements.
1023+
return exn;
1024+
}
1025+
10151026
private static Object instanceExport(Object[] args) {
10161027
checkArgumentCount(args, 2);
10171028
if (!(args[0] instanceof WasmInstance instance)) {

‎wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/constants/BytecodeBitEncoding.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,4 +180,8 @@ public class BytecodeBitEncoding {
180180

181181
public static final int CODE_ENTRY_LOCALS_FLAG = 0b0000_0010;
182182
public static final int CODE_ENTRY_RESULT_FLAG = 0b0000_0001;
183+
184+
// Exception handlers
185+
186+
public static final int INVALID_EXCEPTION_TABLE_OFFSET = -1;
183187
}

0 commit comments

Comments
 (0)