- Revision
- 262100
- Author
- ticaiol...@gmail.com
- Date
- 2020-05-23 09:35:29 -0700 (Sat, 23 May 2020)
Log 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:
Modified Paths
Diff
Modified: trunk/JSTests/ChangeLog (262099 => 262100)
--- trunk/JSTests/ChangeLog 2020-05-23 16:21:11 UTC (rev 262099)
+++ trunk/JSTests/ChangeLog 2020-05-23 16:35:29 UTC (rev 262100)
@@ -1,3 +1,14 @@
+2020-05-23 Caio Lima <ticaiol...@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 <shvaikal...@gmail.com>
Array.prototype.splice doesn't set "length" of returned object
Modified: trunk/JSTests/stress/array-buffer-view-watchpoint-can-be-fired-in-really-add-in-dfg.js (262099 => 262100)
--- trunk/JSTests/stress/array-buffer-view-watchpoint-can-be-fired-in-really-add-in-dfg.js 2020-05-23 16:21:11 UTC (rev 262099)
+++ trunk/JSTests/stress/array-buffer-view-watchpoint-can-be-fired-in-really-add-in-dfg.js 2020-05-23 16:35:29 UTC (rev 262100)
@@ -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;
Modified: trunk/JSTests/stress/big-int-mod-memory-stress.js (262099 => 262100)
--- trunk/JSTests/stress/big-int-mod-memory-stress.js 2020-05-23 16:21:11 UTC (rev 262099)
+++ trunk/JSTests/stress/big-int-mod-memory-stress.js 2020-05-23 16:35:29 UTC (rev 262100)
@@ -1,4 +1,3 @@
-//@ skip if ["arm", "mips"].include?($architecture)
//@ runBigIntEnabled
function assert(a) {
Modified: trunk/JSTests/stress/incremental-marking-should-not-dead-lock-in-new-property-transition.js (262099 => 262100)
--- trunk/JSTests/stress/incremental-marking-should-not-dead-lock-in-new-property-transition.js 2020-05-23 16:21:11 UTC (rev 262099)
+++ trunk/JSTests/stress/incremental-marking-should-not-dead-lock-in-new-property-transition.js 2020-05-23 16:35:29 UTC (rev 262100)
@@ -1,4 +1,3 @@
-//@ skip if ["arm", "mips"].include?($architecture)
//@ skip if $hostOS == "playstation"
//@ runDefault("--gcIncrementScale=100", "--gcIncrementBytes=10", "--numberOfGCMarkers=1")
Modified: trunk/LayoutTests/ChangeLog (262099 => 262100)
--- trunk/LayoutTests/ChangeLog 2020-05-23 16:21:11 UTC (rev 262099)
+++ trunk/LayoutTests/ChangeLog 2020-05-23 16:35:29 UTC (rev 262100)
@@ -1,3 +1,12 @@
+2020-05-23 Caio Lima <ticaiol...@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 <za...@apple.com>
[LFC][TFC] Non-collapsing row border should not make the table wider/taller
Modified: trunk/LayoutTests/js/script-tests/stack-overflow-regexp.js (262099 => 262100)
--- trunk/LayoutTests/js/script-tests/stack-overflow-regexp.js 2020-05-23 16:21:11 UTC (rev 262099)
+++ trunk/LayoutTests/js/script-tests/stack-overflow-regexp.js 2020-05-23 16:35:29 UTC (rev 262100)
@@ -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');
Modified: trunk/Source/bmalloc/ChangeLog (262099 => 262100)
--- trunk/Source/bmalloc/ChangeLog 2020-05-23 16:21:11 UTC (rev 262099)
+++ trunk/Source/bmalloc/ChangeLog 2020-05-23 16:35:29 UTC (rev 262100)
@@ -1,3 +1,27 @@
+2020-05-23 Caio Lima <ticaiol...@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....@apple.com>
Implement a faster findBitInWord() using the hardware ctz instruction.
Modified: trunk/Source/bmalloc/bmalloc/ObjectTypeTable.cpp (262099 => 262100)
--- trunk/Source/bmalloc/bmalloc/ObjectTypeTable.cpp 2020-05-23 16:21:11 UTC (rev 262099)
+++ trunk/Source/bmalloc/bmalloc/ObjectTypeTable.cpp 2020-05-23 16:35:29 UTC (rev 262100)
@@ -41,20 +41,39 @@
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);