Changeset 262100 in webkit

Timestamp:

May 23, 2020, 9:35:29 AM (5 years ago)

Author:

Caio Lima

Message:

[bmalloc] Fix OOM errors on MIPS after r261667
​https://bugs.webkit.org/show_bug.cgi?id=212016

Reviewed by Yusuke Suzuki.

JSTests:

• stress/array-buffer-view-watchpoint-can-be-fired-in-really-add-in-dfg.js:
• stress/big-int-mod-memory-stress.js:
• stress/incremental-marking-should-not-dead-lock-in-new-property-transition.js:

Source/bmalloc:

The way we were calculating newBegin and newEnd on
ObjectTypeTable::set when index is out of bounds didn't consider
cases where bits->begin() - bits->count() or index - ObjectTypeTable::Bits::bitCountPerWord * 4
can underflow and bits->end() + bits->count() can overflow.
Given that, the value used is going to be index or index + 1.
Since we extend the size of bitvector everytime we have an OOB, this can cause a pathological case
that memory will keep extending quite often until systems reachs OOM.
It is reproducible on ARMv7 and MIPS architectures on
stress/array-buffer-view-watchpoint-can-be-fired-in-really-add-in-dfg.js,
stress/big-int-mod-memory-stress.js and some other tests.
This patch is including a verification if those operations are going
to overflow/underflow, and properly set newBegin to 0 and newEnd
to UINT_MAX when we observe such behavior.

• bmalloc/ObjectTypeTable.cpp:

(bmalloc::ObjectTypeTable::set):

LayoutTests:

• js/script-tests/stack-overflow-regexp.js:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/array-buffer-view-watchpoint-can-be-fired-in-really-add-in-dfg.js (modified) (1 diff)
• JSTests/stress/big-int-mod-memory-stress.js (modified) (1 diff)
• JSTests/stress/incremental-marking-should-not-dead-lock-in-new-property-transition.js (modified) (1 diff)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/js/script-tests/stack-overflow-regexp.js (modified) (1 diff)
• Source/bmalloc/ChangeLog (modified) (1 diff)
• Source/bmalloc/bmalloc/ObjectTypeTable.cpp (modified) (2 diffs)

trunk/JSTests/ChangeLog ¶

@@ +1 @@
+2020-05-23  Caio Lima  <ticaiolima@gmail.com>
+
+        [bmalloc] Fix OOM errors on MIPS after r261667
+        https://bugs.webkit.org/show_bug.cgi?id=212016
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/array-buffer-view-watchpoint-can-be-fired-in-really-add-in-dfg.js:
+        * stress/big-int-mod-memory-stress.js:
+        * stress/incremental-marking-should-not-dead-lock-in-new-property-transition.js:
+
 2020-05-22  Alexey Shvayka  <shvaikalesh@gmail.com>
 

trunk/JSTests/stress/array-buffer-view-watchpoint-can-be-fired-in-really-add-in-dfg.js ¶

@@ -1 @@
-//@ skip if ["arm", "mips", "powerpc", "powerpc64", "s390"].include?($architecture) and $hostOS == "linux"
+//@ skip if ["arm", "powerpc", "powerpc64", "s390"].include?($architecture) and $hostOS == "linux"
+//@ requireOptions("-e", "let iterations=40000") if ["mips"].include?($architecture)
 //@ runDefault("--jitPolicyScale=0")
 

trunk/JSTests/stress/big-int-mod-memory-stress.js ¶

@@ -1 @@
-//@ skip if ["arm", "mips"].include?($architecture)
 //@ runBigIntEnabled
 

trunk/JSTests/stress/incremental-marking-should-not-dead-lock-in-new-property-transition.js ¶

@@ -1 @@
-//@ skip if ["arm", "mips"].include?($architecture)
 //@ skip if $hostOS == "playstation"
 //@ runDefault("--gcIncrementScale=100", "--gcIncrementBytes=10", "--numberOfGCMarkers=1")

trunk/LayoutTests/ChangeLog ¶

@@ +1 @@
+2020-05-23  Caio Lima  <ticaiolima@gmail.com>
+
+        [bmalloc] Fix OOM errors on MIPS after r261667
+        https://bugs.webkit.org/show_bug.cgi?id=212016
+
+        Reviewed by Yusuke Suzuki.
+
+        * js/script-tests/stack-overflow-regexp.js:
+
 2020-05-23  Zalan Bujtas  <zalan@apple.com>
 

trunk/LayoutTests/js/script-tests/stack-overflow-regexp.js ¶

@@ -1 +1 @@
 // https://bugs.webkit.org/show_bug.cgi?id=190755
-//@ skip if ["arm", "mips"].include?($architecture) and $hostOS == "linux"
+//@ skip if $architecture == "arm" and $hostOS == "linux"
 //  &&&&
 description('Test that we do not overflow the stack while handling regular expressions');

trunk/Source/bmalloc/ChangeLog ¶

@@ +1 @@
+2020-05-23  Caio Lima  <ticaiolima@gmail.com>
+
+        [bmalloc] Fix OOM errors on MIPS after r261667
+        https://bugs.webkit.org/show_bug.cgi?id=212016
+
+        Reviewed by Yusuke Suzuki.
+
+        The way we were calculating `newBegin` and `newEnd` on
+        `ObjectTypeTable::set` when index is out of bounds didn't consider
+        cases where `bits->begin() - bits->count()` or `index - ObjectTypeTable::Bits::bitCountPerWord * 4`
+        can underflow and `bits->end() + bits->count()` can overflow.
+        Given that, the value used is going to be `index` or `index + 1`.
+        Since we extend the size of bitvector everytime we have an OOB, this can cause a pathological case
+        that memory will keep extending quite often until systems reachs OOM.
+        It is reproducible on ARMv7 and MIPS architectures on
+        `stress/array-buffer-view-watchpoint-can-be-fired-in-really-add-in-dfg.js`,
+        `stress/big-int-mod-memory-stress.js` and some other tests.
+        This patch is including a verification if those operations are going
+        to overflow/underflow, and properly set `newBegin` to 0 and `newEnd`
+        to UINT_MAX when we observe such behavior.
+
+        * bmalloc/ObjectTypeTable.cpp:
+        (bmalloc::ObjectTypeTable::set):
+
 2020-05-18  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/bmalloc/bmalloc/ObjectTypeTable.cpp ¶

@@ -42 +42 @@
             // This is initial allocation of ObjectTypeTable. In this case, it could be possible that for the first registration,
             // some VAs are already allocated for a different purpose, and later they will be reused for bmalloc. In that case,
-            // soon, we will see a smaller index request than this initial one. We subtract a 128MB offset to the initial newBegin
-            // to cover such patterns without extending table too quickly.
-            newBegin = std::min<unsigned>(index, index - ObjectTypeTable::Bits::bitCountPerWord * 4);
+            // soon, we will see a smaller index request than this initial one. We try to subtract a 128MB offset to the initial
+            // newBegin to cover such patterns without extending table too quickly, and if we can't subtract 128MB, we will set
+            // newBegin to 0.  
+            constexpr unsigned offsetForInitialAllocation = ObjectTypeTable::Bits::bitCountPerWord * 4;
+            if (index < offsetForInitialAllocation)
+                newBegin = 0;
+            else
+                newBegin = index - offsetForInitialAllocation;
             newEnd = index + 1;
         } else if (index < bits->begin()) {
             BASSERT(bits->begin());
             BASSERT(bits->end());
-            newBegin = std::min<unsigned>(index, bits->begin() - bits->count());
+            // We need to verify if "bits->begin() - bits->count()" doesn't underflow,
+            // otherwise we will set "newBegin" as "index" and it creates a pathological
+            // case that will keep increasing BitVector everytime we access
+            // "index < bits->begin()".
+            if (bits->begin() < bits->count())
+                newBegin = 0;
+            else
+                newBegin = std::min<unsigned>(index, bits->begin() - bits->count());
             newEnd = bits->end();
         } else {
@@ -55 +67 @@
             BASSERT(bits->end());
             newBegin = bits->begin();
-            newEnd = std::max<unsigned>(index + 1, bits->end() + bits->count());
+            // We need to verify if "bits->end() + bits->count()" doesn't overflow,
+            // otherwise we will set "newEnd" as "index + 1" and it creates a
+            // pathological case that will keep increasing BitVector everytime we access
+            // "index > bits->end()".
+            if (std::numeric_limits<unsigned>::max() - bits->count() < bits->end())
+                newEnd = std::numeric_limits<unsigned>::max();
+            else
+                newEnd = std::max<unsigned>(index + 1, bits->end() + bits->count());
         }
         newBegin = static_cast<unsigned>(roundDownToMultipleOf<size_t>(ObjectTypeTable::Bits::bitCountPerWord, newBegin));