Make Validator work with TryTable and blocks with return types

Author: patrick96

web-image/src/com.oracle.svm.hosted.webimage/src/com/oracle/svm/hosted/webimage/wasm/ast/Instruction.java

@@ -609,6 +609,11 @@ public static Const forWord(WordBase word) {
             // TODO GR-42105 Use forInt
             return forLong(word.rawValue());
         }
+
+        @Override
+        protected String toInnerString() {
+            return literal.type + ", " + literal.asText();
+        }
     }
 
     /**

web-image/src/com.oracle.svm.hosted.webimage/src/com/oracle/svm/hosted/webimage/wasm/ast/visitors/WasmValidator.java

@@ -78,10 +78,24 @@
 public class WasmValidator extends WasmVisitor {
     static class CtrlFrame {
         final WasmId.Label label;
+        final WasmValType returnType;
+        /**
+         * Whether the end of this frame (block) is unreachable. There might still be reachable code
+         * in the frame, but if this is true, execution can never reach the end of the block.
+         */
         boolean unreachable = false;
+        /**
+         * Whether any instructions target this block.
+         */
+        boolean targeted = false;
 
-        CtrlFrame(WasmId.Label label) {
+        CtrlFrame(WasmId.Label label, WasmValType returnType) {
             this.label = label;
+            this.returnType = returnType;
+        }
+
+        public WasmValType getReturnType() {
+            return returnType;
         }
     }
 
@@ -343,21 +357,30 @@ private void errorIf(boolean condition, String msg) {
         }
     }
 
-    private static boolean assertIdsEqual(WasmId first, WasmId second) {
+    private static boolean idsEqual(WasmId first, WasmId second) {
         return Objects.equals(first, second);
     }
 
     private CtrlFrame topFrame() {
         return Objects.requireNonNull(ctrls.peek());
     }
 
+    /**
+     * Whether the current frame is marked as unreachable.
+     *
+     * @see #markUnreachable()
+     */
+    private boolean isUnreachable() {
+        return topFrame().unreachable;
+    }
+
     private void pushVal(WasmValType t) {
+        errorIf(isUnreachable(), "Tried to push " + t + " when unreachable");
         vals.push(t);
     }
 
     private WasmValType popVal() {
-        CtrlFrame top = topFrame();
-        if (vals.isEmpty() && top.unreachable) {
+        if (isUnreachable()) {
             return null;
         }
 
@@ -383,6 +406,10 @@ private RuntimeException typeMismatch(WasmValType[] expected, Deque<WasmValType>
      * @param expected The top of the stack must match this (the last element is at the very top).
      */
     private void popVals(WasmValType... expected) {
+        if (isUnreachable()) {
+            return;
+        }
+
         int numTypes = expected.length;
         if (vals.size() < numTypes) {
             throw typeMismatch(expected);
@@ -416,12 +443,20 @@ private Global getAndAssertGlobalExists(WasmId.Global global) {
     }
 
     private void pushCtrl(WasmId.Label label) {
+        pushCtrl(label, null);
+    }
+
+    private void pushCtrl(WasmId.Label label, WasmValType resultType) {
         assertStackEmpty();
         if (label != null) {
             // No name conflicts.
             assertIdUniqueName(label, ctrls.stream().map(frame -> frame.label).filter(Objects::nonNull).collect(Collectors.toList()));
         }
-        ctrls.push(new CtrlFrame(label));
+        ctrls.push(new CtrlFrame(label, resultType));
+    }
+
+    private void pushBlockCtrl(Instruction.WasmBlock block) {
+        pushCtrl(block.getLabel(), block.getResult());
     }
 
     private void popCtrl(WasmId.Label expectedLabel) {
@@ -430,6 +465,15 @@ private void popCtrl(WasmId.Label expectedLabel) {
         assertStackEmpty();
         errorIf(frame.label != expectedLabel, "Expected control frame " + expectedLabel + " but got " + frame.label);
         ctrls.pop();
+        /*
+         * If the end of the frame was unreachable and no instruction targeted it, it means that it
+         * is impossible to reach the instruction right after it and we have to mark the outer block
+         * as unreachable as well.
+         */
+        if (!frame.targeted && frame.unreachable && !ctrls.isEmpty()) {
+            markUnreachable();
+        }
+
     }
 
     private void popBlockCtrl(Instruction.WasmBlock block) {
@@ -442,7 +486,16 @@ private void popBlockCtrl(Instruction.WasmBlock block) {
         }
     }
 
-    private void unreachable() {
+    /**
+     * Marks the top frame as unreachable.
+     * <p>
+     * An unreachable frame satisfies all requirements on the operand stack (i.e.
+     * {@link #popVals(WasmValType...) never errors.}).
+     * <p>
+     * Call this method after visiting any instruction at which execution stops and never returns
+     * (e.g. {@code br}).
+     */
+    private void markUnreachable() {
         CtrlFrame top = topFrame();
         vals.clear();
         top.unreachable = true;
@@ -461,8 +514,15 @@ private void applyTypeUse(TypeUse typeUse) {
         typeUse.results.forEach(this::pushVal);
     }
 
-    private void assertLabelExists(WasmId.Label label) {
-        errorIf(ctrls.stream().noneMatch(frame -> assertIdsEqual(label, frame.label)), "Label " + label + " does not exist.");
+    private CtrlFrame markLabelTargeted(WasmId.Label label) {
+        CtrlFrame frame = ctrls.stream().filter(f -> idsEqual(label, f.label)).findFirst().orElseThrow(() -> error("Label " + label + " does not exist."));
+        frame.targeted = true;
+        return frame;
+    }
+
+    private void markLabelTargetedWithReturnType(WasmId.Label label, WasmValType returnType) {
+        CtrlFrame frame = markLabelTargeted(label);
+        errorIf(!Objects.equals(frame.getReturnType(), returnType), "Label " + label + " has return type " + frame.getReturnType() + " but " + returnType + " was expected");
     }
 
     /**
@@ -576,14 +636,14 @@ public void visitInstruction(Instruction inst) {
 
     @Override
     public void visitBlock(Instruction.Block block) {
-        pushCtrl(block.getLabel());
+        pushBlockCtrl(block);
         super.visitBlock(block);
         popBlockCtrl(block);
     }
 
     @Override
     public void visitLoop(Instruction.Loop loop) {
-        pushCtrl(loop.getLabel());
+        pushBlockCtrl(loop);
         super.visitLoop(loop);
         popBlockCtrl(loop);
     }
@@ -592,19 +652,63 @@ public void visitLoop(Instruction.Loop loop) {
     public void visitIf(Instruction.If ifBlock) {
         visitInstruction(ifBlock.condition);
         popVals(i32);
-        pushCtrl(ifBlock.getLabel());
+        pushBlockCtrl(ifBlock);
+
+        /*
+         * Propagating the information about unreachability upward here requires some more work. We
+         * only want to mark the end of the if-block as unreachable if both the end of the then- and
+         * else-branches are unreachable.
+         */
+        boolean thenBlockUnreachable = false;
+        boolean elseBlockUnreachable = false;
+
+        // Control frame around both branches to intercept the unreachable state
+        pushCtrl(null);
+
+        // Control frame around the then-branch
+        pushCtrl(null);
         visitInstructions(ifBlock.thenInstructions);
-        popBlockCtrl(ifBlock);
+        if (topFrame().unreachable) {
+            thenBlockUnreachable = true;
+            topFrame().unreachable = false;
+        }
+        popCtrl(null);
+
         if (ifBlock.hasElse()) {
-            pushCtrl(ifBlock.getLabel());
+            pushCtrl(null);
             visitInstructions(ifBlock.elseInstructions);
-            popBlockCtrl(ifBlock);
+            if (topFrame().unreachable) {
+                elseBlockUnreachable = true;
+                topFrame().unreachable = false;
+            }
+            popCtrl(null);
+        }
+        if (thenBlockUnreachable && elseBlockUnreachable) {
+            // This will mark the parent block as unreachable once we pop this control frame.
+            markUnreachable();
         }
+        popCtrl(null);
+        popBlockCtrl(ifBlock);
+    }
+
+    @Override
+    public void visitTryTable(Instruction.TryTable tryBlock) {
+        tryBlock.catchBlocks.forEach(this::assertCatchValid);
+        pushBlockCtrl(tryBlock);
+        visitInstructions(tryBlock.instructions);
+        popBlockCtrl(tryBlock);
+    }
+
+    private void assertCatchValid(Instruction.TryTable.Catch catchClause) {
+        errorIf(!ctxt.hasTag(catchClause.tag), "No matching tag for catch clause: " + catchClause);
+        List<WasmValType> catchParams = catchClause.tag.typeUse.params;
+        errorIf(catchParams.size() != 1, "Can only support catch clause tags with a single param, got" + catchParams.size());
+        markLabelTargetedWithReturnType(catchClause.label, catchParams.getFirst());
     }
 
     @Override
     public void visitTry(Instruction.Try tryBlock) {
-        pushCtrl(tryBlock.getLabel());
+        pushBlockCtrl(tryBlock);
         visitInstructions(tryBlock.instructions);
 
         for (Instruction.Try.Catch catchBlock : tryBlock.catchBlocks) {
@@ -620,7 +724,7 @@ public void visitTry(Instruction.Try tryBlock) {
 
     @Override
     public void visitUnreachable(Instruction.Unreachable unreachable) {
-        unreachable();
+        markUnreachable();
         super.visitUnreachable(unreachable);
     }
 
@@ -642,10 +746,10 @@ public void visitBreak(Instruction.Break inst) {
 
         WasmId.Label targetLabel = inst.getTarget();
 
-        assertLabelExists(targetLabel);
+        markLabelTargeted(targetLabel);
 
         if (inst.condition == null) {
-            unreachable();
+            markUnreachable();
         } else {
             popVals(i32);
         }
@@ -658,13 +762,13 @@ public void visitBreakTable(Instruction.BreakTable inst) {
         popVals(i32);
 
         WasmId.Label defaultLabel = inst.getDefaultTarget();
-        assertLabelExists(defaultLabel);
+        markLabelTargeted(defaultLabel);
 
         for (int i = 0; i < inst.numTargets(); i++) {
-            assertLabelExists(inst.getTarget(i));
+            markLabelTargeted(inst.getTarget(i));
         }
 
-        unreachable();
+        markUnreachable();
     }
 
     @Override
@@ -753,6 +857,7 @@ public void visitThrow(Instruction.Throw inst) {
 
         errorIf(!ctxt.hasTag(inst.tag), "No matching tag for throw: " + inst);
         applyTypeUse(inst.tag.typeUse);
+        markUnreachable();
     }
 
     @Override

web-image/src/com.oracle.svm.hosted.webimage/src/com/oracle/svm/hosted/webimage/wasm/codegen/WebImageWasmCodeGen.java

@@ -184,10 +184,6 @@ protected void emitCode() {
         module.constructActiveDataSegments();
         ((WebImageWasmHeapBreakdownProvider) HeapBreakdownProvider.singleton()).setActualTotalHeapSize((int) getFullImageHeapSize());
 
-        if (WebImageOptions.DebugOptions.VerificationPhases.getValue(options)) {
-            validateModule();
-        }
-
         emitJSCode();
 
         try (Writer writer = Files.newBufferedWriter(watFile)) {
@@ -196,6 +192,10 @@ protected void emitCode() {
             throw new RuntimeException(e);
         }
 
+        if (WebImageOptions.DebugOptions.VerificationPhases.getValue(options)) {
+            validateModule();
+        }
+
         assembleWasmFile(watFile, wasmFile);
     }