Title: [249221] trunk/Source/_javascript_Core
Revision
249221
Author
mark....@apple.com
Date
2019-08-28 14:24:47 -0700 (Wed, 28 Aug 2019)

Log Message

Wasm's AirIRGenerator::addLocal() and B3IRGenerator::addLocal() are doing unnecessary overflow checks.
https://bugs.webkit.org/show_bug.cgi?id=201006
<rdar://problem/52053991>

Reviewed by Yusuke Suzuki.

We already ensured that it is not possible to overflow in Wasm::FunctionParser's
parse().  It is unnecessary and misleading to do those overflow checks in
AirIRGenerator and B3IRGenerator.  The only check that is necessary is that
m_locals.tryReserveCapacity() is successful, otherwise, we have an out of memory
situation.

This patch changes these unnecessary checks to assertions instead.

* wasm/WasmAirIRGenerator.cpp:
(JSC::Wasm::AirIRGenerator::addLocal):
* wasm/WasmB3IRGenerator.cpp:
(JSC::Wasm::B3IRGenerator::addLocal):
* wasm/WasmValidate.cpp:
(JSC::Wasm::Validate::addLocal):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (249220 => 249221)


--- trunk/Source/_javascript_Core/ChangeLog	2019-08-28 21:23:01 UTC (rev 249220)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-08-28 21:24:47 UTC (rev 249221)
@@ -1,3 +1,26 @@
+2019-08-28  Mark Lam  <mark....@apple.com>
+
+        Wasm's AirIRGenerator::addLocal() and B3IRGenerator::addLocal() are doing unnecessary overflow checks.
+        https://bugs.webkit.org/show_bug.cgi?id=201006
+        <rdar://problem/52053991>
+
+        Reviewed by Yusuke Suzuki.
+
+        We already ensured that it is not possible to overflow in Wasm::FunctionParser's
+        parse().  It is unnecessary and misleading to do those overflow checks in
+        AirIRGenerator and B3IRGenerator.  The only check that is necessary is that
+        m_locals.tryReserveCapacity() is successful, otherwise, we have an out of memory
+        situation.
+
+        This patch changes these unnecessary checks to assertions instead.
+
+        * wasm/WasmAirIRGenerator.cpp:
+        (JSC::Wasm::AirIRGenerator::addLocal):
+        * wasm/WasmB3IRGenerator.cpp:
+        (JSC::Wasm::B3IRGenerator::addLocal):
+        * wasm/WasmValidate.cpp:
+        (JSC::Wasm::Validate::addLocal):
+
 2019-08-28  Keith Rollin  <krol...@apple.com>
 
         Remove support for macOS < 10.13 (part 2)

Modified: trunk/Source/_javascript_Core/wasm/WasmAirIRGenerator.cpp (249220 => 249221)


--- trunk/Source/_javascript_Core/wasm/WasmAirIRGenerator.cpp	2019-08-28 21:23:01 UTC (rev 249220)
+++ trunk/Source/_javascript_Core/wasm/WasmAirIRGenerator.cpp	2019-08-28 21:24:47 UTC (rev 249221)
@@ -914,10 +914,10 @@
 
 auto AirIRGenerator::addLocal(Type type, uint32_t count) -> PartialResult
 {
-    Checked<uint32_t, RecordOverflow> totalBytesChecked = count;
-    totalBytesChecked += m_locals.size();
-    uint32_t totalBytes;
-    WASM_COMPILE_FAIL_IF((totalBytesChecked.safeGet(totalBytes) == CheckedState::DidOverflow) || !m_locals.tryReserveCapacity(totalBytes), "can't allocate memory for ", totalBytes, " locals");
+    size_t newSize = m_locals.size() + count;
+    ASSERT(!(CheckedUint32(count) + m_locals.size()).hasOverflowed());
+    ASSERT(newSize <= maxFunctionLocals);
+    WASM_COMPILE_FAIL_IF(!m_locals.tryReserveCapacity(newSize), "can't allocate memory for ", newSize, " locals");
 
     for (uint32_t i = 0; i < count; ++i) {
         auto local = tmpForType(type);

Modified: trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp (249220 => 249221)


--- trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp	2019-08-28 21:23:01 UTC (rev 249220)
+++ trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp	2019-08-28 21:24:47 UTC (rev 249221)
@@ -658,10 +658,10 @@
 
 auto B3IRGenerator::addLocal(Type type, uint32_t count) -> PartialResult
 {
-    Checked<uint32_t, RecordOverflow> totalBytesChecked = count;
-    totalBytesChecked += m_locals.size();
-    uint32_t totalBytes;
-    WASM_COMPILE_FAIL_IF((totalBytesChecked.safeGet(totalBytes) == CheckedState::DidOverflow) || !m_locals.tryReserveCapacity(totalBytes), "can't allocate memory for ", totalBytes, " locals");
+    size_t newSize = m_locals.size() + count;
+    ASSERT(!(CheckedUint32(count) + m_locals.size()).hasOverflowed());
+    ASSERT(newSize <= maxFunctionLocals);
+    WASM_COMPILE_FAIL_IF(!m_locals.tryReserveCapacity(newSize), "can't allocate memory for ", newSize, " locals");
 
     for (uint32_t i = 0; i < count; ++i) {
         Variable* local = m_proc.addVariable(toB3Type(type));

Modified: trunk/Source/_javascript_Core/wasm/WasmValidate.cpp (249220 => 249221)


--- trunk/Source/_javascript_Core/wasm/WasmValidate.cpp	2019-08-28 21:23:01 UTC (rev 249220)
+++ trunk/Source/_javascript_Core/wasm/WasmValidate.cpp	2019-08-28 21:24:47 UTC (rev 249221)
@@ -249,8 +249,10 @@
 
 auto Validate::addLocal(Type type, uint32_t count) -> Result
 {
-    size_t size = m_locals.size() + count;
-    WASM_VALIDATOR_FAIL_IF(!m_locals.tryReserveCapacity(size), "can't allocate memory for ", size, " locals");
+    size_t newSize = m_locals.size() + count;
+    ASSERT(!(CheckedUint32(count) + m_locals.size()).hasOverflowed());
+    ASSERT(newSize <= maxFunctionLocals);
+    WASM_VALIDATOR_FAIL_IF(!m_locals.tryReserveCapacity(newSize), "can't allocate memory for ", newSize, " locals");
 
     for (uint32_t i = 0; i < count; ++i)
         m_locals.uncheckedAppend(type);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to