Fix resolution of indirect calls.

rkennke · Aug 5, 2020 · c0075a9 · c0075a9
1 parent 540f6c6
commit c0075a9
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 36 deletions.
diff --git a/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/BinaryParser.java b/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/BinaryParser.java
@@ -999,30 +999,20 @@ private void readElementSection() {
                 functionIndices[index] = functionIndex;
             }
             module.addLinkAction((context, instance) -> {
-                final WasmTable table = instance.table();
-                if (table == null || currentOffsetGlobalIndex == -1) {
-                    // Note: we do not check if the earlier element segments were executed,
-                    // and we do not try to execute the element segments in order,
-                    // as we do with data sections and the memory.
-                    // Instead, if any table element is written more than once, we report an error.
-                    // Thus, the order in which the element sections are loaded is not important
-                    // (also, I did not notice the toolchains overriding the same element slots,
-                    // or anything in the spec about that).
-                    WasmFunction[] elements = new WasmFunction[segmentLength];
-                    for (int index = 0; index != segmentLength; ++index) {
-                        final int functionIndex = functionIndices[index];
-                        final WasmFunction function = symbolTable.function(functionIndex);
-                        elements[index] = function;
-                    }
-                    context.linker().resolveElemSegment(context, instance, currentElemSegmentId, currentOffsetAddress, currentOffsetGlobalIndex, segmentLength, elements);
-                } else {
-                    table.ensureSizeAtLeast(currentOffsetAddress + segmentLength);
-                    for (int index = 0; index != segmentLength; ++index) {
-                        final int functionIndex = functionIndices[index];
-                        final WasmFunction function = symbolTable.function(functionIndex);
-                        table.initialize(currentOffsetAddress + index, function);
-                    }
+                // Note: we do not check if the earlier element segments were executed,
+                // and we do not try to execute the element segments in order,
+                // as we do with data sections and the memory.
+                // Instead, if any table element is written more than once, we report an error.
+                // Thus, the order in which the element sections are loaded is not important
+                // (also, I did not notice the toolchains overriding the same element slots,
+                // or anything in the spec about that).
+                WasmFunction[] elements = new WasmFunction[segmentLength];
+                for (int index = 0; index != segmentLength; ++index) {
+                    final int functionIndex = functionIndices[index];
+                    final WasmFunction function = symbolTable.function(functionIndex);
+                    elements[index] = function;
                 }
+                context.linker().resolveElemSegment(context, instance, currentElemSegmentId, currentOffsetAddress, currentOffsetGlobalIndex, segmentLength, elements);
             });
         }
     }

diff --git a/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/Linker.java b/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/Linker.java
@@ -71,7 +71,6 @@
 import org.graalvm.wasm.nodes.WasmBlockNode;
 
 import com.oracle.truffle.api.CompilerDirectives;
-import com.oracle.truffle.api.interop.UnknownIdentifierException;
 
 public class Linker {
     private enum LinkState {
@@ -252,11 +251,7 @@ void resolveFunctionImport(WasmContext context, WasmInstance instance, WasmFunct
                                                 "', does not exist.");
             }
             WasmFunction importedFunction;
-            try {
-                importedFunction = (WasmFunction) importedInstance.readMember(function.importedFunctionName());
-            } catch (UnknownIdentifierException e) {
-                importedFunction = null;
-            }
+            importedFunction = importedInstance.module().exportedFunctions().get(function.importedFunctionName());
             if (importedFunction == null) {
                 throw new WasmLinkerException("The imported function '" + function.importedFunctionName() + "', referenced in the module '" + instance.name() +
                                 "', does not exist in the imported module '" + function.importedModuleName() + "'.");
@@ -417,7 +412,8 @@ void resolveElemSegment(WasmContext context, WasmInstance instance, int elemSegm
             table.ensureSizeAtLeast(baseAddress + segmentLength);
             for (int index = 0; index != segmentLength; ++index) {
                 final WasmFunction function = functions[index];
-                table.initialize(baseAddress + index, function);
+                final CallTarget target = instance.target(function.index());
+                table.initialize(baseAddress + index, new WasmFunctionInstance(function, target));
             }
         };
         final ArrayList<Sym> dependencies = new ArrayList<>();
@@ -427,6 +423,11 @@ void resolveElemSegment(WasmContext context, WasmInstance instance, int elemSegm
         if (offsetGlobalIndex != -1) {
             dependencies.add(new InitializeGlobalSym(instance.name(), offsetGlobalIndex));
         }
+        for (WasmFunction function : functions) {
+            if (function.importDescriptor() != null) {
+                dependencies.add(new ImportFunctionSym(instance.name(), function.importDescriptor()));
+            }
+        }
         resolutionDag.resolveLater(new ElemSym(instance.name(), elemSegmentId), dependencies.toArray(new Sym[dependencies.size()]), resolveAction);
     }
 

diff --git a/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/WasmFunctionInstance.java b/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/WasmFunctionInstance.java
@@ -51,7 +51,6 @@
 
 @ExportLibrary(InteropLibrary.class)
 public class WasmFunctionInstance implements TruffleObject {
-
     private final WasmFunction function;
     private final CallTarget target;
 
@@ -72,6 +71,14 @@ public String name() {
         return function.name();
     }
 
+    public WasmFunction function() {
+        return function;
+    }
+
+    public CallTarget target() {
+        return target;
+    }
+
     @ExportMessage
     boolean isExecutable() {
         return true;

diff --git a/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/WasmTable.java b/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/WasmTable.java
@@ -91,7 +91,7 @@ public void set(int index, Object function) {
         elements[index] = function;
     }
 
-    public void initialize(int i, WasmFunction function) {
+    public void initialize(int i, WasmFunctionInstance function) {
         if (elements[i] != null) {
             throw new WasmValidationException("Table " + tableIndex + " already has an element at index " + i + ".");
         }

diff --git a/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/nodes/WasmBlockNode.java b/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/nodes/WasmBlockNode.java
@@ -234,8 +234,10 @@
 import org.graalvm.wasm.WasmCodeEntry;
 import org.graalvm.wasm.WasmContext;
 import org.graalvm.wasm.WasmFunction;
+import org.graalvm.wasm.WasmFunctionInstance;
 import org.graalvm.wasm.WasmInstance;
 import org.graalvm.wasm.WasmLanguage;
+import org.graalvm.wasm.WasmTable;
 import org.graalvm.wasm.exception.WasmExecutionException;
 import org.graalvm.wasm.exception.WasmTrap;
 import org.graalvm.wasm.memory.WasmMemory;
@@ -607,14 +609,16 @@ public int execute(WasmContext context, VirtualFrame frame) {
                     // Extract the function object.
                     stackPointer--;
                     final SymbolTable symtab = instance().symbolTable();
-                    final Object[] elements = instance().table().elements();
+                    final WasmTable table = instance().table();
+                    final Object[] elements = table.elements();
                     final int elementIndex = popInt(frame, stackPointer);
                     if (elementIndex < 0 || elementIndex >= elements.length) {
                         throw new WasmTrap(this, "Element index '" + elementIndex + "' out of table bounds.");
                     }
                     // Currently, table elements may only be functions.
                     // We can add a check here when this changes in the future.
-                    final WasmFunction function = (WasmFunction) elements[elementIndex];
+                    final WasmFunctionInstance functionInstance = (WasmFunctionInstance) elements[elementIndex];
+                    final WasmFunction function = functionInstance.function();
                     if (function == null) {
                         throw new WasmTrap(this, "Table element at index " + elementIndex + " is uninitialized.");
                     }
@@ -650,8 +654,8 @@ public int execute(WasmContext context, VirtualFrame frame) {
                     stackPointer -= args.length;
 
                     trace("indirect call to function %s (%d args)", function, args.length);
-                    CallTarget target = instance().target(function.index());
-                    Object result = callNode.execute(target, args);
+                    final CallTarget target = functionInstance.target();
+                    final Object result = callNode.execute(target, args);
                     trace("return from indirect_call to function %s : %s", function, result);
                     // At the moment, WebAssembly functions may return up to one value.
                     // As per the WebAssembly specification, this restriction may be lifted in the