Check type equivalence for mutable global imports, clean up global imports

Author: jirkamarsik

wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/Linker.java

@@ -327,22 +327,14 @@ private static void checkFailures(ArrayList<Throwable> failures) {
         }
     }
 
-    void resolveGlobalImport(WasmStore store, WasmInstance instance, ImportDescriptor importDescriptor, int globalIndex, int valueType, byte mutability,
-                    ImportValueSupplier imports) {
+    void resolveGlobalImport(WasmStore store, WasmInstance instance, ImportDescriptor importDescriptor, int globalIndex, int valueType, byte mutability, ImportValueSupplier imports) {
         instance.globals().setInitialized(globalIndex, false);
         final String importedGlobalName = importDescriptor.memberName();
         final String importedModuleName = importDescriptor.moduleName();
         final Runnable resolveAction = () -> {
             assert instance.module().globalImported(globalIndex) && globalIndex == importDescriptor.targetIndex() : importDescriptor;
             WasmGlobal externalGlobal = lookupImportObject(instance, importDescriptor, imports, WasmGlobal.class);
-            final int exportedValueType;
-            final SymbolTable.ClosedValueType exportedClosedValueType;
-            final byte exportedMutability;
-            if (externalGlobal != null) {
-                exportedValueType = externalGlobal.getType();
-                exportedClosedValueType = externalGlobal.getClosedValueType();
-                exportedMutability = externalGlobal.getMutability();
-            } else {
+            if (externalGlobal == null) {
                 final WasmInstance importedInstance = store.lookupModuleInstance(importedModuleName);
                 if (importedInstance == null) {
                     throw WasmException.create(Failure.UNKNOWN_IMPORT, "Module '" + importedModuleName + "', referenced in the import of global variable '" +
@@ -356,21 +348,22 @@ void resolveGlobalImport(WasmStore store, WasmInstance instance, ImportDescripto
                                     "', was not exported in the module '" + importedModuleName + "'.");
                 }
 
-                exportedValueType = importedInstance.symbolTable().globalValueType(exportedGlobalIndex);
-                exportedClosedValueType = importedInstance.symbolTable().globalClosedValueType(exportedGlobalIndex);
-                exportedMutability = importedInstance.symbolTable().globalMutability(exportedGlobalIndex);
-
                 externalGlobal = importedInstance.externalGlobal(exportedGlobalIndex);
             }
-            if (!instance.symbolTable().closedTypeOf(valueType).isSupertypeOf(exportedClosedValueType)) {
+            SymbolTable.ClosedValueType importType = instance.symbolTable().closedTypeOf(valueType);
+            SymbolTable.ClosedValueType exportType = externalGlobal.getClosedType();
+            if (mutability != externalGlobal.getMutability()) {
                 throw WasmException.create(Failure.INCOMPATIBLE_IMPORT_TYPE, "Global variable '" + importedGlobalName + "' is imported into module '" + instance.name() +
-                                "' with the type " + WasmType.toString(valueType) + ", " +
-                                "but it was exported in the module '" + importedModuleName + "' with the type " + WasmType.toString(exportedValueType) + ".");
+                                "' with the modifier " + GlobalModifier.asString(mutability) + ", " +
+                                "but it was exported in the module '" + importedModuleName + "' with the modifier " + GlobalModifier.asString(externalGlobal.getMutability()) + ".");
             }
-            if (exportedMutability != mutability) {
+            // matching for mutable globals does not work by subtyping, but requires equivalent
+            // types
+            if (!(externalGlobal.isMutable() ? importType.equals(exportType) : importType.isSupertypeOf(exportType))) {
                 throw WasmException.create(Failure.INCOMPATIBLE_IMPORT_TYPE, "Global variable '" + importedGlobalName + "' is imported into module '" + instance.name() +
-                                "' with the modifier " + GlobalModifier.asString(mutability) + ", " +
-                                "but it was exported in the module '" + importedModuleName + "' with the modifier " + GlobalModifier.asString(exportedMutability) + ".");
+                                "' with the type " + GlobalModifier.asString(mutability) + " " + WasmType.toString(valueType) + ", " +
+                                "but it was exported in the module '" + importedModuleName + "' with the type " + GlobalModifier.asString(externalGlobal.getMutability()) + " " +
+                                WasmType.toString(externalGlobal.getType()) + ".");
             }
             instance.setExternalGlobal(globalIndex, externalGlobal);
             instance.globals().setInitialized(globalIndex, true);
@@ -391,7 +384,7 @@ private static void initializeGlobal(WasmInstance instance, int globalIndex, Obj
         assert !instance.globals().isInitialized(globalIndex) : globalIndex;
         SymbolTable symbolTable = instance.symbolTable();
         if (symbolTable.globalExternal(globalIndex)) {
-            var global = new WasmGlobal(symbolTable.globalValueType(globalIndex), symbolTable.isGlobalMutable(globalIndex), instance.symbolTable(), initValue);
+            var global = new WasmGlobal(globalIndex, symbolTable, initValue);
             instance.setExternalGlobal(globalIndex, global);
         } else {
             instance.globals().store(symbolTable.globalValueType(globalIndex), symbolTable.globalAddress(globalIndex), initValue);
@@ -568,9 +561,8 @@ void resolveTagImport(WasmStore store, WasmInstance instance, ImportDescriptor i
                 }
                 importedTag = importedInstance.tag(exportedTagIndex);
             }
-            // matching for tag types does not work by subtyping, but requires equivalent types,
-            // A <= B and B <= A
-            Assert.assertTrue(type.isSupertypeOf(importedTag.type()) && importedTag.type().isSupertypeOf(type), Failure.INCOMPATIBLE_IMPORT_TYPE);
+            // matching for tag types does not work by subtyping, but requires equivalent types
+            Assert.assertTrue(type.equals(importedTag.type()), Failure.INCOMPATIBLE_IMPORT_TYPE);
             instance.setTag(tagIndex, importedTag);
         };
         resolutionDag.resolveLater(new ImportTagSym(instance.name(), importDescriptor, tagIndex), new Sym[]{new ExportTagSym(importedModuleName, importedTagName)}, resolveAction);

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

@@ -89,6 +89,17 @@ public abstract class SymbolTable {
     public static final int NO_EQUIVALENCE_CLASS = 0;
     public static final int FIRST_EQUIVALENCE_CLASS = NO_EQUIVALENCE_CLASS + 1;
 
+    /**
+     * Represents a WebAssembly value type in its closed form, with all type indices replaced with
+     * their definitions. You can query the subtyping relation on types using the predicates
+     * {@link #isSupertypeOf(ClosedValueType)} and {@link #isSubtypeOf(ClosedValueType)}, both of
+     * which are written to be PE-friendly provided the receiver type is a PE constant.
+     * <p>
+     * If you need to check whether two types are equivalent, instead of checking
+     * {@code A.isSupertypeOf(B) && A.isSubtypeOf(B)}, you can use {@code A.equals(B)}, since, in
+     * the WebAssembly type system, type equivalence corresponds to structural equality.
+     * </p>
+     */
     public abstract static sealed class ClosedValueType {
         // This is a workaround until we can use pattern matching in JDK 21+.
         public enum Kind {
@@ -1265,10 +1276,6 @@ public int globalValueType(int index) {
         return globalTypes[index];
     }
 
-    public ClosedValueType globalClosedValueType(int index) {
-        return closedTypeOf(globalTypes[index]);
-    }
-
     private byte globalFlags(int index) {
         return globalFlags[index];
     }

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

@@ -812,7 +812,7 @@ public Object globalRead(WasmGlobal global) {
                 if (!refTypes) {
                     throw new WasmJsApiException(WasmJsApiException.Kind.TypeError, "Invalid value type. Reference types are not enabled.");
                 }
-                if (SymbolTable.closedTypeOf(WasmType.EXNREF_TYPE, null).isSupertypeOf(global.getClosedValueType())) {
+                if (SymbolTable.closedTypeOf(WasmType.EXNREF_TYPE, null).isSupertypeOf(global.getClosedType())) {
                     throw new WasmJsApiException(WasmJsApiException.Kind.TypeError, WasmJsApiException.EXNREF_VALUE_ACCESS);
                 }
                 yield global.loadAsReference();
@@ -832,7 +832,7 @@ public Object globalWrite(WasmGlobal global, Object value) {
         if (!global.isMutable()) {
             throw WasmJsApiException.format(WasmJsApiException.Kind.TypeError, "Global is not mutable.");
         }
-        if (!global.getClosedValueType().matchesValue(value)) {
+        if (!global.getClosedType().matchesValue(value)) {
             throw WasmJsApiException.format(WasmJsApiException.Kind.TypeError, "Global type %s, value: %s", ValueType.fromValue(global.getType()), value);
         }
         switch (global.getType()) {
@@ -846,7 +846,7 @@ public Object globalWrite(WasmGlobal global, Object value) {
                 if (!refTypes) {
                     throw WasmJsApiException.format(WasmJsApiException.Kind.TypeError, "Invalid value type. Reference types are not enabled.");
                 }
-                if (SymbolTable.closedTypeOf(WasmType.EXNREF_TYPE, null).isSupertypeOf(global.getClosedValueType())) {
+                if (SymbolTable.closedTypeOf(WasmType.EXNREF_TYPE, null).isSupertypeOf(global.getClosedType())) {
                     throw new WasmJsApiException(WasmJsApiException.Kind.TypeError, WasmJsApiException.EXNREF_VALUE_ACCESS);
                 }
                 global.storeReference(value);

wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/globals/WasmGlobal.java

@@ -93,8 +93,9 @@ public static WasmGlobal allocRef(ValueType valueType, boolean mutable, Object v
         return result;
     }
 
-    public WasmGlobal(int type, boolean mutable, SymbolTable symbolTable, Object value) {
-        this(type, mutable, symbolTable);
+    public WasmGlobal(int globalIndex, SymbolTable symbolTable, Object value) {
+        this(symbolTable.globalValueType(globalIndex), symbolTable.isGlobalMutable(globalIndex), symbolTable);
+        assert symbolTable.globalExternal(globalIndex);
         this.globalValue = switch (type) {
             case WasmType.I32_TYPE -> (int) value;
             case WasmType.I64_TYPE -> (long) value;
@@ -109,7 +110,7 @@ public int getType() {
         return type;
     }
 
-    public SymbolTable.ClosedValueType getClosedValueType() {
+    public SymbolTable.ClosedValueType getClosedType() {
         return SymbolTable.closedTypeOf(getType(), symbolTable);
     }
 
@@ -225,14 +226,14 @@ void writeMember(String member, Object value,
             case WasmType.F32_TYPE -> storeInt(Float.floatToRawIntBits(valueLibrary.asFloat(value)));
             case WasmType.F64_TYPE -> storeLong(Double.doubleToRawLongBits(valueLibrary.asDouble(value)));
             case WasmType.V128_TYPE -> {
-                if (!getClosedValueType().matchesValue(value)) {
+                if (!getClosedType().matchesValue(value)) {
                     throw UnsupportedMessageException.create();
                 }
                 storeVector128((Vector128) value);
             }
             default -> {
                 assert WasmType.isReferenceType(type);
-                if (!getClosedValueType().matchesValue(value)) {
+                if (!getClosedType().matchesValue(value)) {
                     throw UnsupportedMessageException.create();
                 }
                 storeReference(value);