Title: [210115] trunk
Revision
210115
Author
sbar...@apple.com
Date
2016-12-22 14:40:39 -0800 (Thu, 22 Dec 2016)

Log Message

WebAssembly: Make the spec-tests/address.wast.js test pass
https://bugs.webkit.org/show_bug.cgi?id=166429
<rdar://problem/29793220>

Reviewed by Keith Miller.

JSTests:

* wasm.yaml:

Source/_javascript_Core:

Right now, provably out of bound loads/stores (given a load/store's constant
offset) are not a validation error. However, we were failing to catch uint32_t
overflows in release builds (we did have a debug assertion). To fix this,
I now detect when uint32_t addition will overflow, and instead of emitting
a normal load/store, I emit code that throws an out of bounds memory exception.

* wasm/WasmB3IRGenerator.cpp:

Modified Paths

Diff

Modified: trunk/JSTests/ChangeLog (210114 => 210115)


--- trunk/JSTests/ChangeLog	2016-12-22 22:29:27 UTC (rev 210114)
+++ trunk/JSTests/ChangeLog	2016-12-22 22:40:39 UTC (rev 210115)
@@ -1,3 +1,13 @@
+2016-12-22  Saam Barati  <sbar...@apple.com>
+
+        WebAssembly: Make the spec-tests/address.wast.js test pass
+        https://bugs.webkit.org/show_bug.cgi?id=166429
+        <rdar://problem/29793220>
+
+        Reviewed by Keith Miller.
+
+        * wasm.yaml:
+
 2016-12-22  Keith Miller  <keith_mil...@apple.com>
 
         WebAssembly: The validator should not allow unused stack entries at the end of a block

Modified: trunk/JSTests/wasm.yaml (210114 => 210115)


--- trunk/JSTests/wasm.yaml	2016-12-22 22:29:27 UTC (rev 210114)
+++ trunk/JSTests/wasm.yaml	2016-12-22 22:40:39 UTC (rev 210115)
@@ -29,7 +29,7 @@
   cmd: runWebAssembly
 
 - path: wasm/spec-tests/address.wast.js
-  cmd: runWebAssemblySpecTest :skip
+  cmd: runWebAssemblySpecTest :normal
 
 - path: wasm/spec-tests/binary.wast.js
   cmd: runWebAssemblySpecTest :normal

Modified: trunk/Source/_javascript_Core/ChangeLog (210114 => 210115)


--- trunk/Source/_javascript_Core/ChangeLog	2016-12-22 22:29:27 UTC (rev 210114)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-12-22 22:40:39 UTC (rev 210115)
@@ -1,3 +1,19 @@
+2016-12-22  Saam Barati  <sbar...@apple.com>
+
+        WebAssembly: Make the spec-tests/address.wast.js test pass
+        https://bugs.webkit.org/show_bug.cgi?id=166429
+        <rdar://problem/29793220>
+
+        Reviewed by Keith Miller.
+
+        Right now, provably out of bound loads/stores (given a load/store's constant
+        offset) are not a validation error. However, we were failing to catch uint32_t
+        overflows in release builds (we did have a debug assertion). To fix this,
+        I now detect when uint32_t addition will overflow, and instead of emitting
+        a normal load/store, I emit code that throws an out of bounds memory exception.
+
+        * wasm/WasmB3IRGenerator.cpp:
+
 2016-12-22  Keith Miller  <keith_mil...@apple.com>
 
         WebAssembly: The validator should not allow unused stack entries at the end of a block

Modified: trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp (210114 => 210115)


--- trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp	2016-12-22 22:29:27 UTC (rev 210114)
+++ trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp	2016-12-22 22:40:39 UTC (rev 210115)
@@ -465,7 +465,42 @@
 {
     ASSERT(pointer->type() == Int32);
 
-    result = emitLoadOp(op, Origin(), emitCheckAndPreparePointer(pointer, offset, sizeOfLoadOp(op)), offset);
+    if (UNLIKELY(sumOverflows<uint32_t>(offset, sizeOfLoadOp(op)))) {
+        // FIXME: Even though this is provably out of bounds, it's not a validation error, so we have to handle it
+        // as a runtime exception. However, this may change: https://bugs.webkit.org/show_bug.cgi?id=166435
+        B3::PatchpointValue* throwException = m_currentBlock->appendNew<B3::PatchpointValue>(m_proc, B3::Void, Origin());
+        throwException->setGenerator([this] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
+            this->emitExceptionCheck(jit, ExceptionType::OutOfBoundsMemoryAccess);
+        });
+
+        switch (op) {
+        case LoadOpType::I32Load8S:
+        case LoadOpType::I32Load16S:
+        case LoadOpType::I32Load:
+        case LoadOpType::I32Load16U:
+        case LoadOpType::I32Load8U:
+            result = zeroForType(I32);
+            break;
+        case LoadOpType::I64Load8S:
+        case LoadOpType::I64Load8U:
+        case LoadOpType::I64Load16S:
+        case LoadOpType::I64Load32U:
+        case LoadOpType::I64Load32S:
+        case LoadOpType::I64Load:
+        case LoadOpType::I64Load16U:
+            result = zeroForType(I64);
+            break;
+        case LoadOpType::F32Load:
+            result = zeroForType(F32);
+            break;
+        case LoadOpType::F64Load:
+            result = zeroForType(F64);
+            break;
+        }
+
+    } else
+        result = emitLoadOp(op, Origin(), emitCheckAndPreparePointer(pointer, offset, sizeOfLoadOp(op)), offset);
+
     return { };
 }
 
@@ -527,7 +562,16 @@
 {
     ASSERT(pointer->type() == Int32);
 
-    emitStoreOp(op, Origin(), emitCheckAndPreparePointer(pointer, offset, sizeOfStoreOp(op)), value, offset);
+    if (UNLIKELY(sumOverflows<uint32_t>(offset, sizeOfStoreOp(op)))) {
+        // FIXME: Even though this is provably out of bounds, it's not a validation error, so we have to handle it
+        // as a runtime exception. However, this may change: https://bugs.webkit.org/show_bug.cgi?id=166435
+        B3::PatchpointValue* throwException = m_currentBlock->appendNew<B3::PatchpointValue>(m_proc, B3::Void, Origin());
+        throwException->setGenerator([this] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
+            this->emitExceptionCheck(jit, ExceptionType::OutOfBoundsMemoryAccess);
+        });
+    } else
+        emitStoreOp(op, Origin(), emitCheckAndPreparePointer(pointer, offset, sizeOfStoreOp(op)), value, offset);
+
     return { };
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to