Fixes to Table model class (#199)

Make the limits immutable. Use the maximum limit constant `0x1_0000_0000` when the upper limit is unspecified, and simplify comparisons accordingly. There is a class which represents limits; change `Table` to reuse it.
dylibso · Jan 26, 2024 · e1538d1 · e1538d1
1 parent 5a7d59f
commit e1538d1
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 51 deletions.
diff --git a/runtime/src/main/java/com/dylibso/chicory/runtime/HostTable.java b/runtime/src/main/java/com/dylibso/chicory/runtime/HostTable.java
@@ -3,6 +3,7 @@
 import static com.dylibso.chicory.wasm.types.Value.REF_NULL_VALUE;
 
 import com.dylibso.chicory.wasm.types.ElementType;
+import com.dylibso.chicory.wasm.types.Limits;
 import com.dylibso.chicory.wasm.types.Table;
 import java.util.Map;
 
@@ -28,14 +29,10 @@ public HostTable(String moduleName, String fieldName, Map<Integer, Integer> func
             }
         }
 
-        this.table = new Table(ElementType.FuncRef, maxFuncRef, maxFuncRef);
+        this.table = new Table(ElementType.FuncRef, new Limits(maxFuncRef, maxFuncRef));
 
         for (int i = 0; i < maxFuncRef; i++) {
-            if (funcRefs.containsKey(i)) {
-                this.table.setRef(i, funcRefs.get(i));
-            } else {
-                this.table.setRef(i, REF_NULL_VALUE);
-            }
+            this.table.setRef(i, funcRefs.getOrDefault(i, REF_NULL_VALUE));
         }
     }
 

diff --git a/runtime/src/main/java/com/dylibso/chicory/runtime/Machine.java b/runtime/src/main/java/com/dylibso/chicory/runtime/Machine.java
@@ -2146,7 +2146,7 @@ private static void TABLE_GET(MStack stack, Instance instance, long[] operands)
             table = instance.imports().table(idx).table();
         }
         var i = stack.pop().asInt();
-        if (i < 0 || (table.limitMax() != 0 && i >= table.limitMax()) || i >= table.size()) {
+        if (i < 0 || i >= table.limits().max() || i >= table.size()) {
             throw new WASMRuntimeException("out of bounds table access");
         }
         stack.push(table.ref(i));

diff --git a/wasm/src/main/java/com/dylibso/chicory/wasm/Parser.java b/wasm/src/main/java/com/dylibso/chicory/wasm/Parser.java
@@ -359,11 +359,10 @@ private static ImportSection parseImportSection(ByteBuffer buffer, long sectionI
                         var limitType = (int) readVarUInt32(buffer);
                         assert limitType == 0x00 || limitType == 0x01;
                         var min = (int) readVarUInt32(buffer);
-                        var max = -1;
-                        if (limitType > 0) {
-                            max = (int) readVarUInt32(buffer);
-                        }
-                        var limits = new Limits(min, max);
+                        var limits =
+                                limitType > 0
+                                        ? new Limits(min, readVarUInt32(buffer))
+                                        : new Limits(min);
 
                         ImportDesc tableDesc = new ImportDesc(descType, limits, tableType);
                         imports[i] = new Import(moduleName, fieldName, tableDesc);
@@ -374,11 +373,10 @@ private static ImportSection parseImportSection(ByteBuffer buffer, long sectionI
                         var limitType = (int) readVarUInt32(buffer);
                         assert limitType == 0x00 || limitType == 0x01;
                         var min = (int) readVarUInt32(buffer);
-                        var max = -1;
-                        if (limitType > 0) {
-                            max = (int) readVarUInt32(buffer);
-                        }
-                        var limits = new Limits(min, max);
+                        var limits =
+                                limitType > 0
+                                        ? new Limits(min, readVarUInt32(buffer))
+                                        : new Limits(min);
 
                         ImportDesc memDesc = new ImportDesc(descType, limits);
                         imports[i] = new Import(moduleName, fieldName, memDesc);
@@ -421,11 +419,8 @@ private static TableSection parseTableSection(ByteBuffer buffer, long sectionId)
             var limitType = readVarUInt32(buffer);
             assert limitType == 0x00 || limitType == 0x01;
             var min = readVarUInt32(buffer);
-            Long max = null;
-            if (limitType == 0x01) {
-                max = readVarUInt32(buffer);
-            }
-            tables[i] = new Table(tableType, min, max);
+            var limits = limitType > 0 ? new Limits(min, readVarUInt32(buffer)) : new Limits(min);
+            tables[i] = new Table(tableType, limits);
         }
 
         return new TableSection(sectionId, tables);

diff --git a/wasm/src/main/java/com/dylibso/chicory/wasm/types/Limits.java b/wasm/src/main/java/com/dylibso/chicory/wasm/types/Limits.java
@@ -1,19 +1,31 @@
 package com.dylibso.chicory.wasm.types;
 
 public class Limits {
-    private final int min;
-    private final int max;
+    public static final long LIMIT_MAX = 0x1_0000_0000L;
 
-    public Limits(int min, int max) {
-        this.min = min;
-        this.max = max;
+    private static final Limits UNBOUNDED = new Limits(0);
+
+    private final long min;
+    private final long max;
+
+    public Limits(long min) {
+        this(min, LIMIT_MAX);
+    }
+
+    public Limits(long min, long max) {
+        this.min = Math.min(Math.max(0, min), LIMIT_MAX);
+        this.max = Math.min(Math.max(min, max), LIMIT_MAX);
     }
 
-    public int min() {
+    public long min() {
         return min;
     }
 
-    public int max() {
+    public long max() {
         return max;
     }
+
+    public static Limits unbounded() {
+        return UNBOUNDED;
+    }
 }
diff --git a/wasm/src/main/java/com/dylibso/chicory/wasm/types/Table.java b/wasm/src/main/java/com/dylibso/chicory/wasm/types/Table.java
@@ -4,46 +4,37 @@
 
 import com.dylibso.chicory.wasm.exceptions.ChicoryException;
 import java.util.Arrays;
+import java.util.Objects;
 
 public class Table {
     private final ElementType elementType;
-    private long limitMin;
-    private long limitMax;
+    private final Limits limits;
 
     private int[] refs;
 
-    public Table(ElementType elementType, long limitMin, Long limitMax) {
-        this.elementType = elementType;
-        this.limitMin = limitMin;
-        if (limitMax != null) {
-            this.limitMax = limitMax;
-        }
-        refs = new int[(int) limitMin];
-        for (int i = 0; i < limitMin; i++) {
-            refs[i] = REF_NULL_VALUE;
-        }
+    public Table(final ElementType elementType, final Limits limits) {
+        this.elementType = Objects.requireNonNull(elementType, "elementType");
+        this.limits = Objects.requireNonNull(limits, "limits");
+        refs = new int[(int) limits.min()];
+        Arrays.fill(refs, REF_NULL_VALUE);
     }
 
     public ElementType elementType() {
         return elementType;
     }
 
-    public long limitMin() {
-        return limitMin;
-    }
-
-    public Long limitMax() {
-        return limitMax;
+    public Limits limits() {
+        return limits;
     }
 
     public int size() {
         return refs.length;
     }
 
-    public int grow(int size, Integer value) {
+    public int grow(int size, int value) {
         var oldSize = refs.length;
         var targetSize = oldSize + size;
-        if (size < 0 || (limitMax != 0 && targetSize > limitMax)) {
+        if (size < 0 || targetSize > limits().max()) {
             return -1;
         }
         var newRefs = Arrays.copyOf(refs, targetSize);
@@ -53,7 +44,7 @@ public int grow(int size, Integer value) {
     }
 
     public Value ref(int index) {
-        int res = REF_NULL_VALUE;
+        int res;
         try {
             res = this.refs[index];
         } catch (IndexOutOfBoundsException e) {
@@ -66,7 +57,7 @@ public Value ref(int index) {
         }
     }
 
-    public void setRef(int index, Integer value) {
+    public void setRef(int index, int value) {
         try {
             this.refs[index] = value;
         } catch (IndexOutOfBoundsException e) {