[bmalloc] Fix OOM errors on MIPS after r261667
authorticaiolima@gmail.com <ticaiolima@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 May 2020 16:35:29 +0000 (16:35 +0000)
committerticaiolima@gmail.com <ticaiolima@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 May 2020 16:35:29 +0000 (16:35 +0000)
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:

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

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

index c26056a..ec645a1 100644 (file)
@@ -1,3 +1,14 @@
+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>
 
         Array.prototype.splice doesn't set "length" of returned object
index 908c72a..75aebe6 100644 (file)
@@ -1,4 +1,5 @@
-//@ 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")
 
 iterations = typeof(iterations) === 'undefined' ? 10000000 : iterations;
index 41d758a..9b67d71 100644 (file)
@@ -1,4 +1,3 @@
-//@ skip if ["arm", "mips"].include?($architecture)
 //@ runBigIntEnabled
 
 function assert(a) {
index ef10d26..38c2757 100644 (file)
@@ -1,4 +1,3 @@
-//@ skip if ["arm", "mips"].include?($architecture)
 //@ skip if $hostOS == "playstation"
 //@ runDefault("--gcIncrementScale=100", "--gcIncrementBytes=10", "--numberOfGCMarkers=1")
 
index 7397679..f577b0b 100644 (file)
@@ -1,3 +1,12 @@
+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>
 
         [LFC][TFC] Non-collapsing row border should not make the table wider/taller
index e4948b7..9e69c2f 100644 (file)
@@ -1,5 +1,5 @@
 // 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');
 
index 595125b..0e00a31 100644 (file)
@@ -1,3 +1,27 @@
+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>
 
         Implement a faster findBitInWord() using the hardware ctz instruction.
index 98112c0..5a73f75 100644 (file)
@@ -41,20 +41,39 @@ void ObjectTypeTable::set(UniqueLockHolder&, Chunk* chunk, ObjectType objectType
         if (bits == &sentinelBits) {
             // 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 {
             BASSERT(bits->begin());
             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));
         BASSERT(newEnd > newBegin);