Skip to content

Commit a83135a

Browse files
eqrioneqrion
committed
Bug 1598149 - Treat data/elem.drop as shrink-to-zero, disallow zero length past end of bounds. r=lth
Spec Issue: WebAssembly/bulk-memory-operations#124 The inline path for memory.copy/fill are updated to fallback to the OOL path when the length is 0 to have proper bounds checking behavior. Differential Revision: https://phabricator.services.mozilla.com/D54599 --HG-- extra : moz-landing-system : lando
1 parent 7cefa8c commit a83135a

File tree

10 files changed

+59
-117
lines changed

10 files changed

+59
-117
lines changed

‎js/src/jit-test/tests/wasm/declared-segs.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ function test(ins) {
3232
)
3333
`),
3434
WebAssembly.RuntimeError,
35-
'use of dropped element segment');
35+
'index out of bounds');
3636
}
37-
test('(elem.drop 0)');
3837
test('(table.init 0 (i32.const 0) (i32.const 0) (i32.const 1))');
3938

4039
// Declared segments don't cause initialization of a table

‎js/src/jit-test/tests/wasm/gc/tables-fill.js

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ function testTableFill(tbl_type, val_type, obj) {
5757

5858
// Partly outside the table
5959
assertErrorMessage(() => ins.exports.fill1(8, obj[5], 3),
60-
RangeError, /table index out of bounds/);
60+
WebAssembly.RuntimeError, /index out of bounds/);
6161

6262
assertEq(ins.exports.get1(7), null);
6363
assertEq(ins.exports.get1(8), null);
@@ -106,25 +106,26 @@ function testTableFill(tbl_type, val_type, obj) {
106106

107107
// Length-one fill1 at the edge of the table fails
108108
assertErrorMessage(() => ins.exports.fill1(10, null, 1),
109-
RangeError, /table index out of bounds/);
109+
WebAssembly.RuntimeError, /index out of bounds/);
110110

111111
// Length-more-than-one fill1 at the edge of the table fails
112112
assertErrorMessage(() => ins.exports.fill1(10, null, 2),
113-
RangeError, /table index out of bounds/);
113+
WebAssembly.RuntimeError, /index out of bounds/);
114114

115115

116116
// Boundary tests on table 1: beyond the edge of the table:
117117

118-
// Length-zero fill1 beyond the edge of the table must succeed
119-
assertEq(ins.exports.fill1(11, null, 0), undefined);
118+
// Length-zero fill1 beyond the edge of the table fails
119+
assertErrorMessage(() => ins.exports.fill1(11, null, 0),
120+
WebAssembly.RuntimeError, /index out of bounds/);
120121

121122
// Length-one fill1 beyond the edge of the table fails
122123
assertErrorMessage(() => ins.exports.fill1(11, null, 1),
123-
RangeError, /table index out of bounds/);
124+
WebAssembly.RuntimeError, /index out of bounds/);
124125

125126
// Length-more-than-one fill1 beyond the edge of the table fails
126127
assertErrorMessage(() => ins.exports.fill1(11, null, 2),
127-
RangeError, /table index out of bounds/);
128+
WebAssembly.RuntimeError, /index out of bounds/);
128129

129130
// Following all the above tests on table 1, check table 0 hasn't changed.
130131
check_table0();

‎js/src/jit-test/tests/wasm/import-export.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,11 @@ var m = new Module(wasmTextToBinary(`
510510
(memory 0)
511511
(data (i32.const 0x10001) ""))
512512
`));
513-
assertEq(new Instance(m) instanceof Instance, true);
513+
if (wasmBulkMemSupported()) {
514+
assertErrorMessage(() => new Instance(m), RuntimeError, /out of bounds/);
515+
} else {
516+
assertEq(new Instance(m) instanceof Instance, true);
517+
}
514518

515519
// Errors during segment initialization do not have observable effects
516520
// and are checked against the actual memory/table length, not the declared

‎js/src/jit-test/tests/wasm/passive-segs-boundary.js

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,11 @@ mem_test("(memory.init 4 (i32.const 1234) (i32.const 1) (i32.const 1))", "",
178178
WebAssembly.CompileError, /memory.init segment index out of range/);
179179

180180
// drop with data seg ix indicating an active segment
181-
mem_test("data.drop 2", "",
182-
WebAssembly.RuntimeError, /use of dropped data segment/);
181+
mem_test("data.drop 2", "");
183182

184183
// init with data seg ix indicating an active segment
185184
mem_test("(memory.init 2 (i32.const 1234) (i32.const 1) (i32.const 1))", "",
186-
WebAssembly.RuntimeError, /use of dropped data segment/);
185+
WebAssembly.RuntimeError, /index out of bounds/);
187186

188187
// init, using a data seg ix more than once is OK
189188
mem_test_nofail(
@@ -192,13 +191,12 @@ mem_test_nofail(
192191

193192
// drop, then drop
194193
mem_test("data.drop 1",
195-
"data.drop 1",
196-
WebAssembly.RuntimeError, /use of dropped data segment/);
194+
"data.drop 1");
197195

198196
// drop, then init
199197
mem_test("data.drop 1",
200198
"(memory.init 1 (i32.const 1234) (i32.const 1) (i32.const 1))",
201-
WebAssembly.RuntimeError, /use of dropped data segment/);
199+
WebAssembly.RuntimeError, /index out of bounds/);
202200

203201
// init: seg ix is valid passive, but length to copy > len of seg
204202
mem_test("",
@@ -223,7 +221,8 @@ mem_test("",
223221
// init: seg ix is valid passive, zero len, but src offset out of bounds one
224222
// past the edge
225223
mem_test("",
226-
"(memory.init 1 (i32.const 1234) (i32.const 5) (i32.const 0))");
224+
"(memory.init 1 (i32.const 1234) (i32.const 5) (i32.const 0))",
225+
WebAssembly.RuntimeError, /index out of bounds/);
227226

228227
// init: seg ix is valid passive, zero len, but dst offset out of bounds at the
229228
// edge
@@ -233,7 +232,8 @@ mem_test("",
233232
// init: seg ix is valid passive, zero len, but dst offset out of bounds one
234233
// past the edge
235234
mem_test("",
236-
"(memory.init 1 (i32.const 0x10001) (i32.const 2) (i32.const 0))");
235+
"(memory.init 1 (i32.const 0x10001) (i32.const 2) (i32.const 0))",
236+
WebAssembly.RuntimeError, /index out of bounds/);
237237

238238
// drop: too many args
239239
mem_test("data.drop 1 (i32.const 42)", "",
@@ -305,12 +305,11 @@ tab_test("(table.init 4 (i32.const 12) (i32.const 1) (i32.const 1))", "",
305305
WebAssembly.CompileError, /table.init segment index out of range/);
306306

307307
// drop with elem seg ix indicating an active segment
308-
tab_test("elem.drop 2", "",
309-
WebAssembly.RuntimeError, /use of dropped element segment/);
308+
tab_test("elem.drop 2", "");
310309

311310
// init with elem seg ix indicating an active segment
312311
tab_test("(table.init 2 (i32.const 12) (i32.const 1) (i32.const 1))", "",
313-
WebAssembly.RuntimeError, /use of dropped element segment/);
312+
WebAssembly.RuntimeError, /index out of bounds/);
314313

315314
// init, using an elem seg ix more than once is OK
316315
tab_test_nofail(
@@ -319,13 +318,12 @@ tab_test_nofail(
319318

320319
// drop, then drop
321320
tab_test("elem.drop 1",
322-
"elem.drop 1",
323-
WebAssembly.RuntimeError, /use of dropped element segment/);
321+
"elem.drop 1");
324322

325323
// drop, then init
326324
tab_test("elem.drop 1",
327325
"(table.init 1 (i32.const 12) (i32.const 1) (i32.const 1))",
328-
WebAssembly.RuntimeError, /use of dropped element segment/);
326+
WebAssembly.RuntimeError, /index out of bounds/);
329327

330328
// init: seg ix is valid passive, but length to copy > len of seg
331329
tab_test("",
@@ -350,7 +348,8 @@ tab_test("",
350348
// init: seg ix is valid passive, zero len, but src offset out of bounds one
351349
// past the edge
352350
tab_test("",
353-
"(table.init 1 (i32.const 12) (i32.const 5) (i32.const 0))");
351+
"(table.init 1 (i32.const 12) (i32.const 5) (i32.const 0))",
352+
WebAssembly.RuntimeError, /index out of bounds/);
354353

355354
// init: seg ix is valid passive, zero len, but dst offset out of bounds
356355
tab_test("",
@@ -359,7 +358,8 @@ tab_test("",
359358
// init: seg ix is valid passive, zero len, but dst offset out of bounds one
360359
// past the edge
361360
tab_test("",
362-
"(table.init 1 (i32.const 31) (i32.const 2) (i32.const 0))");
361+
"(table.init 1 (i32.const 31) (i32.const 2) (i32.const 0))",
362+
WebAssembly.RuntimeError, /index out of bounds/);
363363

364364
// drop: too many args
365365
tab_test("elem.drop 1 (i32.const 42)", "",
@@ -433,12 +433,12 @@ tab_test("(table.copy (i32.const 30) (i32.const 15) (i32.const 0))",
433433

434434
// copy: zero length with dst offset out of bounds one past the edge
435435
tab_test("(table.copy (i32.const 31) (i32.const 15) (i32.const 0))",
436-
"");
436+
"", WebAssembly.RuntimeError, /index out of bounds/);
437437

438438
// copy: zero length with src offset out of bounds at the edge
439439
tab_test("(table.copy (i32.const 15) (i32.const 30) (i32.const 0))",
440440
"");
441441

442442
// copy: zero length with src offset out of bounds one past the edge
443443
tab_test("(table.copy (i32.const 15) (i32.const 31) (i32.const 0))",
444-
"");
444+
"", WebAssembly.RuntimeError, /index out of bounds/);

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,8 @@ function checkRange(arr, minIx, maxIxPlusOne, expectedValue)
572572
)
573573
)`
574574
);
575-
inst.exports.testfn();
575+
assertErrorMessage(() => inst.exports.testfn(),
576+
WebAssembly.RuntimeError, /index out of bounds/);
576577
}
577578

578579
// Very large range
@@ -752,7 +753,8 @@ function checkRange(arr, minIx, maxIxPlusOne, expectedValue)
752753
)
753754
)`
754755
);
755-
inst.exports.testfn();
756+
assertErrorMessage(() => inst.exports.testfn(),
757+
WebAssembly.RuntimeError, /index out of bounds/);
756758
}
757759

758760
// Zero len with src offset out-of-bounds at the edge of memory
@@ -778,7 +780,8 @@ function checkRange(arr, minIx, maxIxPlusOne, expectedValue)
778780
)
779781
)`
780782
);
781-
inst.exports.testfn();
783+
assertErrorMessage(() => inst.exports.testfn(),
784+
WebAssembly.RuntimeError, /index out of bounds/);
782785
}
783786

784787
// 100 random fills followed by 100 random copies, in a single-page buffer,

‎js/src/js.msg

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,8 +407,6 @@ MSG_DEF(JSMSG_WASM_INT_DIVIDE_BY_ZERO, 0, JSEXN_WASMRUNTIMEERROR, "integer divid
407407
MSG_DEF(JSMSG_WASM_OUT_OF_BOUNDS, 0, JSEXN_WASMRUNTIMEERROR, "index out of bounds")
408408
MSG_DEF(JSMSG_WASM_UNALIGNED_ACCESS, 0, JSEXN_WASMRUNTIMEERROR, "unaligned memory access")
409409
MSG_DEF(JSMSG_WASM_WAKE_OVERFLOW, 0, JSEXN_WASMRUNTIMEERROR, "too many woken agents")
410-
MSG_DEF(JSMSG_WASM_DROPPED_DATA_SEG, 0, JSEXN_WASMRUNTIMEERROR, "use of dropped data segment")
411-
MSG_DEF(JSMSG_WASM_DROPPED_ELEM_SEG, 0, JSEXN_WASMRUNTIMEERROR, "use of dropped element segment")
412410
MSG_DEF(JSMSG_WASM_DEREF_NULL, 0, JSEXN_WASMRUNTIMEERROR, "dereferencing null pointer")
413411
MSG_DEF(JSMSG_WASM_BAD_RANGE , 2, JSEXN_RANGEERR, "bad {0} {1}")
414412
MSG_DEF(JSMSG_WASM_BAD_GROW, 1, JSEXN_RANGEERR, "failed to grow {0}")

‎js/src/wasm/WasmBaselineCompile.cpp

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10703,7 +10703,7 @@ bool BaseCompiler::emitMemCopy() {
1070310703

1070410704
int32_t signedLength;
1070510705
if (MacroAssembler::SupportsFastUnalignedAccesses() &&
10706-
peekConstI32(&signedLength) &&
10706+
peekConstI32(&signedLength) && signedLength != 0 &&
1070710707
uint32_t(signedLength) <= MaxInlineMemoryCopyLength) {
1070810708
return emitMemCopyInline();
1070910709
}
@@ -10728,17 +10728,11 @@ bool BaseCompiler::emitMemCopyInline() {
1072810728
int32_t signedLength;
1072910729
MOZ_ALWAYS_TRUE(popConstI32(&signedLength));
1073010730
uint32_t length = signedLength;
10731+
MOZ_ASSERT(length != 0 && length <= MaxInlineMemoryCopyLength);
1073110732

1073210733
RegI32 src = popI32();
1073310734
RegI32 dest = popI32();
1073410735

10735-
// A zero length copy is a no-op and cannot trap
10736-
if (length == 0) {
10737-
freeI32(src);
10738-
freeI32(dest);
10739-
return true;
10740-
}
10741-
1074210736
// Compute the number of copies of each width we will need to do
1074310737
size_t remainder = length;
1074410738
#ifdef JS_64BIT
@@ -10980,7 +10974,7 @@ bool BaseCompiler::emitMemFill() {
1098010974
int32_t signedLength;
1098110975
int32_t signedValue;
1098210976
if (MacroAssembler::SupportsFastUnalignedAccesses() &&
10983-
peek2xI32(&signedLength, &signedValue) &&
10977+
peek2xI32(&signedLength, &signedValue) && signedLength != 0 &&
1098410978
uint32_t(signedLength) <= MaxInlineMemoryFillLength) {
1098510979
return emitMemFillInline();
1098610980
}
@@ -11003,15 +10997,10 @@ bool BaseCompiler::emitMemFillInline() {
1100310997
MOZ_ALWAYS_TRUE(popConstI32(&signedValue));
1100410998
uint32_t length = uint32_t(signedLength);
1100510999
uint32_t value = uint32_t(signedValue);
11000+
MOZ_ASSERT(length != 0 && length <= MaxInlineMemoryFillLength);
1100611001

1100711002
RegI32 dest = popI32();
1100811003

11009-
// A zero length copy is a no-op and cannot trap
11010-
if (length == 0) {
11011-
freeI32(dest);
11012-
return true;
11013-
}
11014-
1101511004
// Compute the number of copies of each width we will need to do
1101611005
size_t remainder = length;
1101711006
#ifdef JS_64BIT

‎js/src/wasm/WasmInstance.cpp

Lines changed: 13 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -463,11 +463,6 @@ inline int32_t WasmMemoryCopy(T memBase, uint32_t memLen,
463463
uint64_t srcOffsetLimit = uint64_t(srcByteOffset) + uint64_t(len);
464464

465465
if (dstOffsetLimit > memLen || srcOffsetLimit > memLen) {
466-
if (len == 0) {
467-
// Zero length copies that are out-of-bounds do not trap.
468-
return 0;
469-
}
470-
471466
JSContext* cx = TlsContext.get();
472467
JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
473468
JSMSG_WASM_OUT_OF_BOUNDS);
@@ -515,9 +510,7 @@ inline int32_t WasmMemoryCopy(T memBase, uint32_t memLen,
515510
"ensured by validation");
516511

517512
if (!instance->passiveDataSegments_[segIndex]) {
518-
JS_ReportErrorNumberASCII(TlsContext.get(), GetErrorMessage, nullptr,
519-
JSMSG_WASM_DROPPED_DATA_SEG);
520-
return -1;
513+
return 0;
521514
}
522515

523516
SharedDataSegment& segRefPtr = instance->passiveDataSegments_[segIndex];
@@ -535,11 +528,6 @@ inline int32_t WasmMemoryFill(T memBase, uint32_t memLen, uint32_t byteOffset,
535528
uint64_t offsetLimit = uint64_t(byteOffset) + uint64_t(len);
536529

537530
if (offsetLimit > memLen) {
538-
if (len == 0) {
539-
// Zero length fills that are out-of-bounds do not trap.
540-
return 0;
541-
}
542-
543531
JSContext* cx = TlsContext.get();
544532
JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
545533
JSMSG_WASM_OUT_OF_BOUNDS);
@@ -586,15 +574,13 @@ inline int32_t WasmMemoryFill(T memBase, uint32_t memLen, uint32_t byteOffset,
586574
MOZ_RELEASE_ASSERT(size_t(segIndex) < instance->passiveDataSegments_.length(),
587575
"ensured by validation");
588576

589-
// Zero length inits that are out-of-bounds do not trap, even if the segment
590-
// has been dropped.
591-
if (len == 0) {
592-
return 0;
593-
}
594-
595577
if (!instance->passiveDataSegments_[segIndex]) {
578+
if (len == 0 && srcOffset == 0) {
579+
return 0;
580+
}
581+
596582
JS_ReportErrorNumberASCII(TlsContext.get(), GetErrorMessage, nullptr,
597-
JSMSG_WASM_DROPPED_DATA_SEG);
583+
JSMSG_WASM_OUT_OF_BOUNDS);
598584
return -1;
599585
}
600586

@@ -652,11 +638,6 @@ inline int32_t WasmMemoryFill(T memBase, uint32_t memLen, uint32_t byteOffset,
652638
uint64_t srcOffsetLimit = uint64_t(srcOffset) + len;
653639

654640
if (dstOffsetLimit > dstTableLen || srcOffsetLimit > srcTableLen) {
655-
// Zero length copies that are out-of-bounds do not trap.
656-
if (len == 0) {
657-
return 0;
658-
}
659-
660641
JS_ReportErrorNumberASCII(TlsContext.get(), GetErrorMessage, nullptr,
661642
JSMSG_WASM_OUT_OF_BOUNDS);
662643
return -1;
@@ -696,9 +677,7 @@ inline int32_t WasmMemoryFill(T memBase, uint32_t memLen, uint32_t byteOffset,
696677
"ensured by validation");
697678

698679
if (!instance->passiveElemSegments_[segIndex]) {
699-
JS_ReportErrorNumberASCII(TlsContext.get(), GetErrorMessage, nullptr,
700-
JSMSG_WASM_DROPPED_ELEM_SEG);
701-
return -1;
680+
return 0;
702681
}
703682

704683
SharedElemSegment& segRefPtr = instance->passiveElemSegments_[segIndex];
@@ -776,15 +755,13 @@ bool Instance::initElems(uint32_t tableIndex, const ElemSegment& seg,
776755
MOZ_RELEASE_ASSERT(size_t(segIndex) < instance->passiveElemSegments_.length(),
777756
"ensured by validation");
778757

779-
// Zero length inits that are out-of-bounds do not trap, even if the segment
780-
// has been dropped.
781-
if (len == 0) {
782-
return 0;
783-
}
784-
785758
if (!instance->passiveElemSegments_[segIndex]) {
759+
if (len == 0 && srcOffset == 0) {
760+
return 0;
761+
}
762+
786763
JS_ReportErrorNumberASCII(TlsContext.get(), GetErrorMessage, nullptr,
787-
JSMSG_WASM_DROPPED_ELEM_SEG);
764+
JSMSG_WASM_OUT_OF_BOUNDS);
788765
return -1;
789766
}
790767

@@ -830,13 +807,8 @@ bool Instance::initElems(uint32_t tableIndex, const ElemSegment& seg,
830807
uint64_t offsetLimit = uint64_t(start) + uint64_t(len);
831808

832809
if (offsetLimit > table.length()) {
833-
// Zero length fills that are out-of-bounds do not trap.
834-
if (len == 0) {
835-
return 0;
836-
}
837-
838810
JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
839-
JSMSG_WASM_TABLE_OUT_OF_BOUNDS);
811+
JSMSG_WASM_OUT_OF_BOUNDS);
840812
return -1;
841813
}
842814

0 commit comments

Comments
 (0)