WebAssembly: Make the spec-tests/address.wast.js test pass
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Dec 2016 22:40:39 +0000 (22:40 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Dec 2016 22:40:39 +0000 (22:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=166429
<rdar://problem/29793220>

Reviewed by Keith Miller.

JSTests:

* wasm.yaml:

Source/JavaScriptCore:

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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@210115 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/wasm.yaml
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp

index 2480e82..93cf99e 100644 (file)
@@ -1,3 +1,13 @@
+2016-12-22  Saam Barati  <sbarati@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_miller@apple.com>
 
         WebAssembly: The validator should not allow unused stack entries at the end of a block
index 8466779..c4764b6 100644 (file)
@@ -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
index 71b7b00..0bd0c94 100644 (file)
@@ -1,3 +1,19 @@
+2016-12-22  Saam Barati  <sbarati@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_miller@apple.com>
 
         WebAssembly: The validator should not allow unused stack entries at the end of a block
index bb513f6..dad5dca 100644 (file)
@@ -465,7 +465,42 @@ auto B3IRGenerator::load(LoadOpType op, ExpressionType pointer, ExpressionType&
 {
     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 @@ auto B3IRGenerator::store(StoreOpType op, ExpressionType pointer, ExpressionType
 {
     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 { };
 }