Bug 1524530 - Fix encoding/decoding of passive element segments. r=jseward

Passive segments carry a type code (currently always AnyFunc but in
the future also AnyRef and other ref types) and each element is not a
raw function reference, but an initializer expression that must be a
RefFunc opcode with a function index, followed by End.

This use of RefFunc creates a circularity in the proposal space, since
reftypes has not definitively assigned an opcode to that operation.
Also, we want to ship bulk copy without having to ship reftypes or
handle RefFunc generally.  So RefFunc is defined provisionally here
outside the opcode space and is only recognized in this one context;
that will change once we have function support in our reftypes
implementation.

--HG--
extra : rebase_source : b63b343c37e697e92dcbee21c4137aace8492e93
extra : histedit_source : b1d0879255b7116f27a26151398d31e5283a78c3%2Cf8067891a3483a0f2c5e429c83888bead4091b3a

js/src/jit-test/lib/wasm-binary.js

@@ -99,6 +99,7 @@ const I64DivUCode      = 0x80;
 const I64RemSCode      = 0x81;
 const I64RemUCode      = 0x82;
 const RefNull          = 0xd0;
+const PlaceholderRefFunc = 0xd2;
 
 const FirstInvalidOpcode = 0xc5;
 const LastInvalidOpcode = 0xfb;

js/src/jit-test/tests/wasm/passive-segs-nonboundary.js

@@ -161,7 +161,6 @@ tab_test("(table.init 1 (i32.const 7) (i32.const 0) (i32.const 4)) \n" +
          "(table.copy (i32.const 19) (i32.const 20) (i32.const 5))",
          [e,e,3,1,4, 1,e,2,7,1, 8,e,7,e,7, 5,2,7,e,9, e,7,e,8,8, e,e,e,e,e]);
 
-
 // And now a simplified version of the above, for memory.{init,drop,copy}.
 
 function gen_mem_mod_t(insn)
@@ -298,6 +297,44 @@ checkNoDataCount([I32ConstCode, 0,
 checkNoDataCount([MiscPrefix, DataDropCode, 0],
                  /data.drop requires a DataCount section/);
 
+// Verification that we can handle encoding errors for passive element segments
+// properly.
+
+function checkPassiveElemSegment(mangle, err) {
+    let bin = moduleWithSections(
+        [v2vSigSection, declSection([0]), // One function
+         tableSection(1),                 // One table
+         { name: elemId,                  // One passive segment
+           body: (function () {
+               let body = [];
+               body.push(1);           // 1 element segment
+               body.push(1);           // Flag: Passive
+               body.push(AnyFuncCode + (mangle == "type" ? 1 : 0)); // always anyfunc
+               body.push(1);           // Element count
+               body.push(PlaceholderRefFunc + (mangle == "ref.func" ? 1 : 0)); // always ref.func
+               body.push(0);           // func index
+               body.push(EndCode + (mangle == "end" ? 1 : 0));
+               return body;
+           })() },
+         bodySection(                   // Empty function
+             [funcBody(
+                 {locals:[],
+                  body:[]})])
+        ]);
+    if (err) {
+        assertErrorMessage(() => new WebAssembly.Module(bin),
+                           WebAssembly.CompileError,
+                           err);
+    } else {
+        new WebAssembly.Module(bin);
+    }
+}
+
+checkPassiveElemSegment("");
+checkPassiveElemSegment("type", /passive segments can only contain function references/);
+checkPassiveElemSegment("ref.func", /failed to read ref.func operation/);
+checkPassiveElemSegment("end", /failed to read end of ref.func expression/);
+
 //---------------------------------------------------------------------//
 //---------------------------------------------------------------------//
 // Some further tests for memory.copy and memory.fill.  First, validation

js/src/wasm/WasmAST.h

@@ -1250,6 +1250,7 @@ class AstElemSegment : public AstNode {
 
   AstRef targetTable() const { return targetTable_; }
   AstRef& targetTableRef() { return targetTable_; }
+  bool isPassive() const { return offsetIfActive_ == nullptr; }
   AstExpr* offsetIfActive() const { return offsetIfActive_; }
   AstRefVector& elems() { return elems_; }
   const AstRefVector& elems() const { return elems_; }

js/src/wasm/WasmConstants.h

@@ -357,6 +357,11 @@ enum class Op {
   Limit = 0x100
 };
 
+// TODO: RefFunc can't be incorporated into the opcode table until we're willing
+// to handle it generally and we've renumbered RefEq, but we need it to express
+// passive element segments.
+constexpr uint16_t PlaceholderRefFunc = 0xd2;
+
 inline bool IsPrefixByte(uint8_t b) { return b >= uint8_t(Op::FirstPrefix); }
 
 // Opcodes in the "miscellaneous" opcode space.

js/src/wasm/WasmTextToBinary.cpp

@@ -7172,14 +7172,28 @@ static bool EncodeElemSegment(Encoder& e, AstElemSegment& segment) {
     return false;
   }
 
+  if (segment.isPassive()) {
+    if (!e.writeFixedU8(uint8_t(TypeCode::AnyFunc))) {
+      return false;
+    }
+  }
+
   if (!e.writeVarU32(segment.elems().length())) {
     return false;
   }
 
   for (const AstRef& elem : segment.elems()) {
+    // Passive segments have an initializer expression, for now restricted to a
+    // function index.
+    if (segment.isPassive() && !e.writeFixedU8(PlaceholderRefFunc)) {
+      return false;
+    }
     if (!e.writeVarU32(elem.index())) {
       return false;
     }
+    if (segment.isPassive() && !e.writeFixedU8(uint8_t(Op::End))) {
+      return false;
+    }
   }
 
   return true;

js/src/wasm/WasmValidate.cpp

@@ -2324,13 +2324,26 @@ static bool DecodeElemSection(Decoder& d, ModuleEnvironment* env) {
 
     seg->tableIndex = tableIndex;
 
-    if (initializerKind == InitializerKind::Active ||
-        initializerKind == InitializerKind::ActiveWithIndex) {
-      InitExpr offset;
-      if (!DecodeInitializerExpression(d, env, ValType::I32, &offset)) {
-        return false;
+    switch (initializerKind) {
+      case InitializerKind::Active:
+      case InitializerKind::ActiveWithIndex: {
+        InitExpr offset;
+        if (!DecodeInitializerExpression(d, env, ValType::I32, &offset)) {
+          return false;
+        }
+        seg->offsetIfActive.emplace(offset);
+        break;
+      }
+      case InitializerKind::Passive: {
+        uint8_t form;
+        if (!d.readFixedU8(&form)) {
+          return d.fail("expected type form");
+        }
+        if (form != uint8_t(TypeCode::AnyFunc)) {
+          return d.fail("passive segments can only contain function references");
+        }
+        break;
       }
-      seg->offsetIfActive.emplace(offset);
     }
 
     uint32_t numElems;
@@ -2355,7 +2368,18 @@ static bool DecodeElemSection(Decoder& d, ModuleEnvironment* env) {
                          env->tables[tableIndex].importedOrExported;
 #endif
 
+    // For passive segments we should use DecodeInitializerExpression() but we
+    // don't really want to generalize that function yet, so instead read the
+    // required Ref.Func and End here.
+
     for (uint32_t i = 0; i < numElems; i++) {
+      if (initializerKind == InitializerKind::Passive) {
+        OpBytes op;
+        if (!d.readOp(&op) || op.b0 != PlaceholderRefFunc) {
+          return d.fail("failed to read ref.func operation");
+        }
+      }
+
       uint32_t funcIndex;
       if (!d.readVarU32(&funcIndex)) {
         return d.fail("failed to read element function index");
@@ -2372,6 +2396,13 @@ static bool DecodeElemSection(Decoder& d, ModuleEnvironment* env) {
       }
 #endif
 
+      if (initializerKind == InitializerKind::Passive) {
+        OpBytes end;
+        if (!d.readOp(&end) || end.b0 != uint16_t(Op::End)) {
+          return d.fail("failed to read end of ref.func expression");
+        }
+      }
+
       seg->elemFuncIndices.infallibleAppend(funcIndex);
     }