Title: [238326] trunk
Revision
238326
Author
fpi...@apple.com
Date
2018-11-16 16:42:44 -0800 (Fri, 16 Nov 2018)

Log Message

All users of ArrayBuffer should agree on the same max size
https://bugs.webkit.org/show_bug.cgi?id=191771

Reviewed by Mark Lam.

JSTests:

* stress/big-wasm-memory-grow-no-max.js: Added.
(foo):
(catch):
* stress/big-wasm-memory-grow.js: Added.
(foo):
(catch):
* stress/big-wasm-memory.js: Added.
(foo):
(catch):

Source/_javascript_Core:

Array buffers cannot be larger than 0x7fffffff, because otherwise loading typedArray.length in the DFG/FTL would produce
a uint32 or would require a signedness check, neither of which sounds reasonable. It's better to just bound their max size
instead.

* runtime/ArrayBuffer.cpp:
(JSC::ArrayBufferContents::ArrayBufferContents):
(JSC::ArrayBufferContents::tryAllocate):
(JSC::ArrayBufferContents::transferTo):
(JSC::ArrayBufferContents::copyTo):
(JSC::ArrayBufferContents::shareWith):
* runtime/ArrayBuffer.h:
* wasm/WasmMemory.cpp:
(JSC::Wasm::Memory::tryCreate):
(JSC::Wasm::Memory::grow):
* wasm/WasmPageCount.h:

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (238325 => 238326)


--- trunk/JSTests/ChangeLog	2018-11-17 00:31:29 UTC (rev 238325)
+++ trunk/JSTests/ChangeLog	2018-11-17 00:42:44 UTC (rev 238326)
@@ -1,5 +1,22 @@
 2018-11-16  Filip Pizlo  <fpi...@apple.com>
 
+        All users of ArrayBuffer should agree on the same max size
+        https://bugs.webkit.org/show_bug.cgi?id=191771
+
+        Reviewed by Mark Lam.
+
+        * stress/big-wasm-memory-grow-no-max.js: Added.
+        (foo):
+        (catch):
+        * stress/big-wasm-memory-grow.js: Added.
+        (foo):
+        (catch):
+        * stress/big-wasm-memory.js: Added.
+        (foo):
+        (catch):
+
+2018-11-16  Filip Pizlo  <fpi...@apple.com>
+
         Unreviewed, make some more tests not crash my computer by only running on instance of it. These tests do not need to
         run for each JSC config since they're regression tests for runtime bugs.
 

Added: trunk/JSTests/stress/big-wasm-memory-grow-no-max.js (0 => 238326)


--- trunk/JSTests/stress/big-wasm-memory-grow-no-max.js	                        (rev 0)
+++ trunk/JSTests/stress/big-wasm-memory-grow-no-max.js	2018-11-17 00:42:44 UTC (rev 238326)
@@ -0,0 +1,33 @@
+let bigArray = new Array(0x7000000);
+bigArray[0] = 1.1;
+bigArray[1] = 1.2;
+
+function foo(array) {
+    var index = array.length;
+    if (index >= bigArray.length || (index - 0x1ffdc01) < 0)
+        return;
+    return bigArray[index - 0x1ffdc01];
+}
+
+noInline(foo);
+
+var okArray = new Uint8Array(0x1ffdc02);
+
+for (var i = 0; i < 10000; ++i)
+    foo(okArray);
+
+var ok = false;
+try {
+    var memory = new WebAssembly.Memory({ initial: 0x1000 });
+    memory.grow(0x7000);
+    var result = foo(new Uint8Array(memory.buffer));
+    if (result !== void 0)
+        throw "Error: bad result at end: " + result;
+    ok = true;
+} catch (e) {
+    if (e.toString() != "RangeError: WebAssembly.Memory.grow expects the grown size to be a valid page count")
+        throw e;
+}
+
+if (ok)
+    throw "Error: did not throw error";

Added: trunk/JSTests/stress/big-wasm-memory-grow.js (0 => 238326)


--- trunk/JSTests/stress/big-wasm-memory-grow.js	                        (rev 0)
+++ trunk/JSTests/stress/big-wasm-memory-grow.js	2018-11-17 00:42:44 UTC (rev 238326)
@@ -0,0 +1,33 @@
+let bigArray = new Array(0x7000000);
+bigArray[0] = 1.1;
+bigArray[1] = 1.2;
+
+function foo(array) {
+    var index = array.length;
+    if (index >= bigArray.length || (index - 0x1ffdc01) < 0)
+        return;
+    return bigArray[index - 0x1ffdc01];
+}
+
+noInline(foo);
+
+var okArray = new Uint8Array(0x1ffdc02);
+
+for (var i = 0; i < 10000; ++i)
+    foo(okArray);
+
+var ok = false;
+try {
+    var memory = new WebAssembly.Memory({ initial: 0x1000, maximum: 0x8000 });
+    memory.grow(0x7000);
+    var result = foo(new Uint8Array(memory.buffer));
+    if (result !== void 0)
+        throw "Error: bad result at end: " + result;
+    ok = true;
+} catch (e) {
+    if (e.toString() != "RangeError: WebAssembly.Memory.grow expects the grown size to be a valid page count")
+        throw e;
+}
+
+if (ok)
+    throw "Error: did not throw error";

Added: trunk/JSTests/stress/big-wasm-memory.js (0 => 238326)


--- trunk/JSTests/stress/big-wasm-memory.js	                        (rev 0)
+++ trunk/JSTests/stress/big-wasm-memory.js	2018-11-17 00:42:44 UTC (rev 238326)
@@ -0,0 +1,31 @@
+let bigArray = new Array(0x7000000);
+bigArray[0] = 1.1;
+bigArray[1] = 1.2;
+
+function foo(array) {
+    var index = array.length;
+    if (index >= bigArray.length || (index - 0x1ffdc01) < 0)
+        return;
+    return bigArray[index - 0x1ffdc01];
+}
+
+noInline(foo);
+
+var okArray = new Uint8Array(0x1ffdc02);
+
+for (var i = 0; i < 10000; ++i)
+    foo(okArray);
+
+var ok = false;
+try {
+    var result = foo(new Uint8Array(new WebAssembly.Memory({ initial: 0x8000, maximum: 0x8000 }).buffer));
+    if (result !== void 0)
+        throw "Error: bad result at end: " + result;
+    ok = true;
+} catch (e) {
+    if (e.toString() != "RangeError: WebAssembly.Memory 'initial' page count is too large")
+        throw e;
+}
+
+if (ok)
+    throw "Error: did not throw error";

Modified: trunk/Source/_javascript_Core/ChangeLog (238325 => 238326)


--- trunk/Source/_javascript_Core/ChangeLog	2018-11-17 00:31:29 UTC (rev 238325)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-11-17 00:42:44 UTC (rev 238326)
@@ -1,3 +1,26 @@
+2018-11-16  Filip Pizlo  <fpi...@apple.com>
+
+        All users of ArrayBuffer should agree on the same max size
+        https://bugs.webkit.org/show_bug.cgi?id=191771
+
+        Reviewed by Mark Lam.
+
+        Array buffers cannot be larger than 0x7fffffff, because otherwise loading typedArray.length in the DFG/FTL would produce
+        a uint32 or would require a signedness check, neither of which sounds reasonable. It's better to just bound their max size
+        instead.
+
+        * runtime/ArrayBuffer.cpp:
+        (JSC::ArrayBufferContents::ArrayBufferContents):
+        (JSC::ArrayBufferContents::tryAllocate):
+        (JSC::ArrayBufferContents::transferTo):
+        (JSC::ArrayBufferContents::copyTo):
+        (JSC::ArrayBufferContents::shareWith):
+        * runtime/ArrayBuffer.h:
+        * wasm/WasmMemory.cpp:
+        (JSC::Wasm::Memory::tryCreate):
+        (JSC::Wasm::Memory::grow):
+        * wasm/WasmPageCount.h:
+
 2018-11-16  Saam Barati  <sbar...@apple.com>
 
         KnownCellUse should also have SpecCellCheck as its type filter

Modified: trunk/Source/_javascript_Core/runtime/ArrayBuffer.cpp (238325 => 238326)


--- trunk/Source/_javascript_Core/runtime/ArrayBuffer.cpp	2018-11-17 00:31:29 UTC (rev 238325)
+++ trunk/Source/_javascript_Core/runtime/ArrayBuffer.cpp	2018-11-17 00:42:44 UTC (rev 238326)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -59,6 +59,7 @@
     : m_data(data)
     , m_sizeInBytes(sizeInBytes)
 {
+    RELEASE_ASSERT(m_sizeInBytes <= MAX_ARRAY_BUFFER_SIZE);
     m_destructor = WTFMove(destructor);
 }
 
@@ -97,8 +98,7 @@
     // Do not allow 31-bit overflow of the total size.
     if (numElements) {
         unsigned totalSize = numElements * elementByteSize;
-        if (totalSize / numElements != elementByteSize
-            || totalSize > static_cast<unsigned>(std::numeric_limits<int32_t>::max())) {
+        if (totalSize / numElements != elementByteSize || totalSize > MAX_ARRAY_BUFFER_SIZE) {
             reset();
             return;
         }
@@ -116,6 +116,7 @@
         memset(m_data.get(), 0, size);
 
     m_sizeInBytes = numElements * elementByteSize;
+    RELEASE_ASSERT(m_sizeInBytes <= MAX_ARRAY_BUFFER_SIZE);
     m_destructor = [] (void* p) { Gigacage::free(Gigacage::Primitive, p); };
 }
 
@@ -130,6 +131,7 @@
     other.clear();
     other.m_data = m_data;
     other.m_sizeInBytes = m_sizeInBytes;
+    RELEASE_ASSERT(other.m_sizeInBytes <= MAX_ARRAY_BUFFER_SIZE);
     other.m_destructor = WTFMove(m_destructor);
     other.m_shared = m_shared;
     reset();
@@ -143,6 +145,7 @@
         return;
     memcpy(other.m_data.get(), m_data.get(), m_sizeInBytes);
     other.m_sizeInBytes = m_sizeInBytes;
+    RELEASE_ASSERT(other.m_sizeInBytes <= MAX_ARRAY_BUFFER_SIZE);
 }
 
 void ArrayBufferContents::shareWith(ArrayBufferContents& other)
@@ -153,6 +156,7 @@
     other.m_shared = m_shared;
     other.m_data = m_data;
     other.m_sizeInBytes = m_sizeInBytes;
+    RELEASE_ASSERT(other.m_sizeInBytes <= MAX_ARRAY_BUFFER_SIZE);
 }
 
 Ref<ArrayBuffer> ArrayBuffer::create(unsigned numElements, unsigned elementByteSize)

Modified: trunk/Source/_javascript_Core/runtime/ArrayBuffer.h (238325 => 238326)


--- trunk/Source/_javascript_Core/runtime/ArrayBuffer.h	2018-11-17 00:31:29 UTC (rev 238325)
+++ trunk/Source/_javascript_Core/runtime/ArrayBuffer.h	2018-11-17 00:42:44 UTC (rev 238326)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -36,6 +36,8 @@
 
 namespace JSC {
 
+#define MAX_ARRAY_BUFFER_SIZE 0x7fffffffu
+
 class VM;
 class ArrayBuffer;
 class ArrayBufferView;

Modified: trunk/Source/_javascript_Core/wasm/WasmMemory.cpp (238325 => 238326)


--- trunk/Source/_javascript_Core/wasm/WasmMemory.cpp	2018-11-17 00:31:29 UTC (rev 238325)
+++ trunk/Source/_javascript_Core/wasm/WasmMemory.cpp	2018-11-17 00:42:44 UTC (rev 238326)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -282,6 +282,8 @@
     const size_t initialBytes = initial.bytes();
     const size_t maximumBytes = maximum ? maximum.bytes() : 0;
 
+    RELEASE_ASSERT(initialBytes <= MAX_ARRAY_BUFFER_SIZE);
+
     if (maximum && !maximumBytes) {
         // User specified a zero maximum, initial size must also be zero.
         RELEASE_ASSERT(!initialBytes);
@@ -372,7 +374,10 @@
         return makeUnexpected(GrowFailReason::InvalidDelta);
     
     const Wasm::PageCount newPageCount = oldPageCount + delta;
-    if (!newPageCount)
+    // FIXME: Creating a wasm memory that is bigger than the ArrayBuffer limit but smaller than the spec limit should throw
+    // OOME not RangeError
+    // https://bugs.webkit.org/show_bug.cgi?id=191776
+    if (!newPageCount || !newPageCount.isValid() || newPageCount.bytes() >= MAX_ARRAY_BUFFER_SIZE)
         return makeUnexpected(GrowFailReason::InvalidGrowSize);
 
     auto success = [&] () {
@@ -395,6 +400,7 @@
         return makeUnexpected(GrowFailReason::WouldExceedMaximum);
 
     size_t desiredSize = newPageCount.bytes();
+    RELEASE_ASSERT(desiredSize <= MAX_ARRAY_BUFFER_SIZE);
     RELEASE_ASSERT(desiredSize > m_size);
     size_t extraBytes = desiredSize - m_size;
     RELEASE_ASSERT(extraBytes);

Modified: trunk/Source/_javascript_Core/wasm/js/WebAssemblyMemoryConstructor.cpp (238325 => 238326)


--- trunk/Source/_javascript_Core/wasm/js/WebAssemblyMemoryConstructor.cpp	2018-11-17 00:31:29 UTC (rev 238325)
+++ trunk/Source/_javascript_Core/wasm/js/WebAssemblyMemoryConstructor.cpp	2018-11-17 00:42:44 UTC (rev 238326)
@@ -70,7 +70,10 @@
         RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
         uint32_t size = toNonWrappingUint32(exec, minSizeValue);
         RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
-        if (!Wasm::PageCount::isValid(size))
+        // FIXME: Creating a wasm memory that is bigger than the ArrayBuffer limit but smaller than the spec limit should throw
+        // OOME not RangeError
+        // https://bugs.webkit.org/show_bug.cgi?id=191776
+        if (!Wasm::PageCount::isValid(size) || Wasm::PageCount(size).bytes() >= MAX_ARRAY_BUFFER_SIZE)
             return JSValue::encode(throwException(exec, throwScope, createRangeError(exec, "WebAssembly.Memory 'initial' page count is too large"_s)));
         initialPageCount = Wasm::PageCount(size);
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to