Bug 1313024: Raise segments errors at wasm instanciation, not compilation; r=luke
authorBenjamin Bouvier <benj@benj.me>
Thu, 27 Oct 2016 20:08:20 +0200
changeset 346648 94efce672651a0dadb9fff2325103f4e22d89362
parent 346647 9a5184bfa3877779da110696be3e68ce43b05227
child 346649 dc216bb3f2c7f149d4d5ca8e1dfee6aac9c5cd4d
push id10298
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:33:03 +0000
treeherdermozilla-aurora@7e29173b1641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1313024
milestone52.0a1
Bug 1313024: Raise segments errors at wasm instanciation, not compilation; r=luke MozReview-Commit-ID: bpA9RNMtOY
js/src/asmjs/WasmBinaryFormat.cpp
js/src/asmjs/WasmCompile.cpp
js/src/asmjs/WasmModule.cpp
js/src/jit-test/tests/wasm/basic.js
js/src/jit-test/tests/wasm/binary.js
js/src/jit-test/tests/wasm/import-export.js
js/src/jit-test/tests/wasm/spec/memory.wast.js
js/src/jit-test/tests/wasm/tables.js
--- a/js/src/asmjs/WasmBinaryFormat.cpp
+++ b/js/src/asmjs/WasmBinaryFormat.cpp
@@ -244,22 +244,16 @@ wasm::DecodeDataSection(Decoder& d, bool
 
         DataSegment seg;
         if (!DecodeInitializerExpression(d, globals, ValType::I32, &seg.offset))
             return false;
 
         if (!d.readVarU32(&seg.length))
             return d.fail("expected segment size");
 
-        if (seg.offset.isVal()) {
-            uint32_t off = seg.offset.val().i32();
-            if (off > minMemoryByteLength || minMemoryByteLength - off < seg.length)
-                return d.fail("data segment does not fit");
-        }
-
         seg.bytecodeOffset = d.currentOffset();
 
         if (!d.readBytes(seg.length))
             return d.fail("data segment shorter than declared");
 
         if (!segments->append(seg))
             return false;
     }
--- a/js/src/asmjs/WasmCompile.cpp
+++ b/js/src/asmjs/WasmCompile.cpp
@@ -1041,23 +1041,16 @@ DecodeElemSection(Decoder& d, Uint32Vect
         InitExpr offset;
         if (!DecodeInitializerExpression(d, mg.globals(), ValType::I32, &offset))
             return false;
 
         uint32_t numElems;
         if (!d.readVarU32(&numElems))
             return d.fail("expected segment size");
 
-        uint32_t tableLength = mg.tables()[tableIndex].limits.initial;
-        if (offset.isVal()) {
-            uint32_t off = offset.val().i32();
-            if (off > tableLength || tableLength - off < numElems)
-                return d.fail("element segment does not fit");
-        }
-
         Uint32Vector elemFuncIndices;
         if (!elemFuncIndices.resize(numElems))
             return false;
 
         for (uint32_t i = 0; i < numElems; i++) {
             if (!d.readVarU32(&elemFuncIndices[i]))
                 return d.fail("failed to read element function index");
             if (elemFuncIndices[i] >= mg.numFuncs())
--- a/js/src/asmjs/WasmModule.cpp
+++ b/js/src/asmjs/WasmModule.cpp
@@ -582,20 +582,24 @@ Module::initSegments(JSContext* cx,
 {
     Instance& instance = instanceObj->instance();
     const SharedTableVector& tables = instance.tables();
 
     // Perform all error checks up front so that this function does not perform
     // partial initialization if an error is reported.
 
     for (const ElemSegment& seg : elemSegments_) {
+        uint32_t numElems = seg.elemCodeRangeIndices.length();
+        if (!numElems)
+            continue;
+
         uint32_t tableLength = tables[seg.tableIndex]->length();
         uint32_t offset = EvaluateInitExpr(globalImports, seg.offset);
 
-        if (offset > tableLength || tableLength - offset < seg.elemCodeRangeIndices.length()) {
+        if (offset > tableLength || tableLength - offset < numElems) {
             JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_WASM_BAD_FIT,
                                       "elem", "table");
             return false;
         }
 
         for (uint32_t elemFuncIndex : seg.elemFuncIndices) {
             if (elemFuncIndex < funcImports.length()) {
                 HandleFunction f = funcImports[elemFuncIndex];
@@ -605,16 +609,19 @@ Module::initSegments(JSContext* cx,
                     return false;
                 }
             }
         }
     }
 
     if (memoryObj) {
         for (const DataSegment& seg : dataSegments_) {
+            if (!seg.length)
+                continue;
+
             uint32_t memoryLength = memoryObj->buffer().byteLength();
             uint32_t offset = EvaluateInitExpr(globalImports, seg.offset);
 
             if (offset > memoryLength || memoryLength - offset < seg.length) {
                 JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_WASM_BAD_FIT,
                                           "data", "memory");
                 return false;
             }
--- a/js/src/jit-test/tests/wasm/basic.js
+++ b/js/src/jit-test/tests/wasm/basic.js
@@ -167,19 +167,16 @@ var buf = wasmEvalText('(module (memory 
 assertEq(new Uint8Array(buf)[0], 'a'.charCodeAt(0));
 assertEq(new Uint8Array(buf)[1], 0);
 assertEq(new Uint8Array(buf)[2], 'b'.charCodeAt(0));
 
 var buf = wasmEvalText('(module (memory 1) (data (i32.const 65535) "c") (export "memory" memory))').exports.memory.buffer;
 assertEq(new Uint8Array(buf)[0], 0);
 assertEq(new Uint8Array(buf)[65535], 'c'.charCodeAt(0));
 
-wasmFailValidateText('(module (memory 1) (data (i32.const 65536) "a") (export "memory" memory))', /data segment does not fit/);
-wasmFailValidateText('(module (memory 1) (data (i32.const 65535) "ab") (export "memory" memory))', /data segment does not fit/);
-
 // ----------------------------------------------------------------------------
 // locals
 
 assertEq(wasmEvalText('(module (func (param i32) (result i32) (get_local 0)) (export "" 0))').exports[""](), 0);
 assertEq(wasmEvalText('(module (func (param i32) (result i32) (get_local 0)) (export "" 0))').exports[""](42), 42);
 assertEq(wasmEvalText('(module (func (param i32) (param i32) (result i32) (get_local 0)) (export "" 0))').exports[""](42, 43), 42);
 assertEq(wasmEvalText('(module (func (param i32) (param i32) (result i32) (get_local 1)) (export "" 0))').exports[""](42, 43), 43);
 
--- a/js/src/jit-test/tests/wasm/binary.js
+++ b/js/src/jit-test/tests/wasm/binary.js
@@ -280,21 +280,16 @@ wasmEval(moduleWithSections([
 
 assertErrorMessage(() => wasmEval(moduleWithSections([ {name: dataId, body: [], } ])), CompileError, /data section requires a memory section/);
 
 wasmEval(moduleWithSections([tableSection(0)]));
 wasmEval(moduleWithSections([elemSection([])]));
 wasmEval(moduleWithSections([tableSection(0), elemSection([])]));
 wasmEval(moduleWithSections([tableSection(1), elemSection([{offset:1, elems:[]}])]));
 assertErrorMessage(() => wasmEval(moduleWithSections([tableSection(1), elemSection([{offset:0, elems:[0]}])])), CompileError, /table element out of range/);
-assertErrorMessage(() => wasmEval(moduleWithSections([tableSection(0), elemSection([{offset:0, elems:[0]}])])), CompileError, /element segment does not fit/);
-assertErrorMessage(() => wasmEval(moduleWithSections([tableSection(1), elemSection([{offset:1, elems:[0]}])])), CompileError, /element segment does not fit/);
-assertErrorMessage(() => wasmEval(moduleWithSections([tableSection(1), elemSection([{offset:0, elems:[0,0]}])])), CompileError, /element segment does not fit/);
-assertErrorMessage(() => wasmEval(moduleWithSections([tableSection(0), elemSection([{offset:1, elems:[]}])])), CompileError, /element segment does not fit/);
-assertErrorMessage(() => wasmEval(moduleWithSections([tableSection(0), elemSection([{offset:-1, elems:[]}])])), CompileError, /element segment does not fit/);
 wasmEval(moduleWithSections([sigSection([v2vSig]), declSection([0]), tableSection(1), elemSection([{offset:0, elems:[0]}]), bodySection([v2vBody])]));
 wasmEval(moduleWithSections([sigSection([v2vSig]), declSection([0]), tableSection(2), elemSection([{offset:0, elems:[0,0]}]), bodySection([v2vBody])]));
 assertErrorMessage(() => wasmEval(moduleWithSections([sigSection([v2vSig]), declSection([0]), tableSection(2), elemSection([{offset:0, elems:[0,1]}]), bodySection([v2vBody])])), CompileError, /table element out of range/);
 wasmEval(moduleWithSections([sigSection([v2vSig]), declSection([0,0,0]), tableSection(4), elemSection([{offset:0, elems:[0,1,0,2]}]), bodySection([v2vBody, v2vBody, v2vBody])]));
 wasmEval(moduleWithSections([sigSection([v2vSig,i2vSig]), declSection([0,0,1]), tableSection(3), elemSection([{offset:0,elems:[0,1,2]}]), bodySection([v2vBody, v2vBody, v2vBody])]));
 
 function invalidTableSection0() {
     var body = [];
--- a/js/src/jit-test/tests/wasm/import-export.js
+++ b/js/src/jit-test/tests/wasm/import-export.js
@@ -370,16 +370,30 @@ var m = new Module(wasmTextToBinary(`
         (memory 1)
         (data (get_global 0) "\\0a\\0b"))
 `));
 assertEq(new Instance(m, {glob:{a:0}}) instanceof Instance, true);
 assertEq(new Instance(m, {glob:{a:(64*1024 - 2)}}) instanceof Instance, true);
 assertErrorMessage(() => new Instance(m, {glob:{a:(64*1024 - 1)}}), RangeError, /data segment does not fit/);
 assertErrorMessage(() => new Instance(m, {glob:{a:64*1024}}), RangeError, /data segment does not fit/);
 
+var m = new Module(wasmTextToBinary(`
+    (module
+        (memory 1)
+        (data (i32.const 0x10001) "\\0a\\0b"))
+`));
+assertErrorMessage(() => new Instance(m), RangeError, /data segment does not fit/);
+
+var m = new Module(wasmTextToBinary(`
+    (module
+        (memory 0)
+        (data (i32.const 0x10001) ""))
+`));
+assertEq(new Instance(m) instanceof Instance, true);
+
 // Errors during segment initialization do not have observable effects
 // and are checked against the actual memory/table length, not the declared
 // initial length.
 
 var m = new Module(wasmTextToBinary(`
     (module
         (import "a" "mem" (memory 1))
         (import "a" "tbl" (table 1 anyfunc))
--- a/js/src/jit-test/tests/wasm/spec/memory.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/memory.wast.js
@@ -1,6 +1,4 @@
 // |jit-test| test-also-wasm-baseline
 // TODO initializer expression can reference global module-defined variables?
-// TODO data segment fitting must be done at instantiation time, not compile time
-// TODO module with 0-sized memory can accept empty segments at any offsets?
 quit();
 var importedArgs = ['memory.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/tables.js
+++ b/js/src/jit-test/tests/wasm/tables.js
@@ -9,18 +9,19 @@ const RuntimeError = WebAssembly.Runtime
 
 var callee = i => `(func $f${i} (result i32) (i32.const ${i}))`;
 
 wasmFailValidateText(`(module (elem (i32.const 0) $f0) ${callee(0)})`, /table index out of range/);
 wasmFailValidateText(`(module (table 10 anyfunc) (elem (i32.const 0) 0))`, /table element out of range/);
 wasmFailValidateText(`(module (table 10 anyfunc) (func) (elem (i32.const 0) 0 1))`, /table element out of range/);
 wasmFailValidateText(`(module (table 10 anyfunc) (func) (elem (f32.const 0) 0) ${callee(0)})`, /type mismatch/);
 
-wasmFailValidateText(`(module (table 10 anyfunc) (elem (i32.const 10) $f0) ${callee(0)})`, /element segment does not fit/);
-wasmFailValidateText(`(module (table 10 anyfunc) (elem (i32.const 8) $f0 $f0 $f0) ${callee(0)})`, /element segment does not fit/);
+assertErrorMessage(() => wasmEvalText(`(module (table 10 anyfunc) (elem (i32.const 10) $f0) ${callee(0)})`), RangeError, /elem segment does not fit/);
+assertErrorMessage(() => wasmEvalText(`(module (table 10 anyfunc) (elem (i32.const 8) $f0 $f0 $f0) ${callee(0)})`), RangeError, /elem segment does not fit/);
+wasmEvalText(`(module (table 0 anyfunc) (func) (elem (i32.const 0x10001)))`);
 
 assertErrorMessage(() => wasmEvalText(`(module (table 10 anyfunc) (import "globals" "a" (global i32)) (elem (get_global 0) $f0) ${callee(0)})`, {globals:{a:10}}), RangeError, /elem segment does not fit/);
 assertErrorMessage(() => wasmEvalText(`(module (table 10 anyfunc) (import "globals" "a" (global i32)) (elem (get_global 0) $f0 $f0 $f0) ${callee(0)})`, {globals:{a:8}}), RangeError, /elem segment does not fit/);
 
 assertEq(new Module(wasmTextToBinary(`(module (table 10 anyfunc) (elem (i32.const 1) $f0 $f0) (elem (i32.const 0) $f0) ${callee(0)})`)) instanceof Module, true);
 assertEq(new Module(wasmTextToBinary(`(module (table 10 anyfunc) (elem (i32.const 1) $f0 $f0) (elem (i32.const 2) $f0) ${callee(0)})`)) instanceof Module, true);
 wasmEvalText(`(module (table 10 anyfunc) (import "globals" "a" (global i32)) (elem (i32.const 1) $f0 $f0) (elem (get_global 0) $f0) ${callee(0)})`, {globals:{a:0}});
 wasmEvalText(`(module (table 10 anyfunc) (import "globals" "a" (global i32)) (elem (get_global 0) $f0 $f0) (elem (i32.const 2) $f0) ${callee(0)})`, {globals:{a:1}});