The following tests started to fail around r261680 and skipped on r261808. The following JSC stress test failures have been introduced: jsc-layout-tests.yaml/js/script-tests/stack-overflow-regexp.js.layout jsc-layout-tests.yaml/js/script-tests/stack-overflow-regexp.js.layout-dfg-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/stack-overflow-regexp.js.layout-no-cjit jsc-layout-tests.yaml/js/script-tests/stack-overflow-regexp.js.layout-no-llint stress/array-buffer-view-watchpoint-can-be-fired-in-really-add-in-dfg.js.default These should not fail and might be related to recent changes regarding memory management.
Created attachment 400042 [details] Patch
Comment on attachment 400042 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400042&action=review > Source/bmalloc/bmalloc/ObjectTypeTable.cpp:48 > } else if (index < bits->begin()) { Doesn't `if (bits == &sentinelBits) {` case require the underflow check too?
Comment on attachment 400042 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400042&action=review >> Source/bmalloc/bmalloc/ObjectTypeTable.cpp:48 >> } else if (index < bits->begin()) { > > Doesn't `if (bits == &sentinelBits) {` case require the underflow check too? I missed it and I think it does. I'll add those.
Created attachment 400068 [details] Patch
Comment on attachment 400068 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400068&action=review r=me with comments. > Source/bmalloc/bmalloc/ObjectTypeTable.cpp:47 > + newBegin = index < ObjectTypeTable::Bits::bitCountPerWord * 4 ? 0 : std::min<unsigned>(index, index - ObjectTypeTable::Bits::bitCountPerWord * 4); Can you wrtie it like, constexpr unsigned offsetForInitialAllocation = ObjectTypeTable::Bits::bitCountPerWord * 4; if (index < offsetForInitialAllocation) newBegin = 0; else newBegin = std::min<unsigned>(index, index - offsetForInitialAllocation); > Source/bmalloc/bmalloc/ObjectTypeTable.cpp:56 > + newBegin = bits->begin() < bits->count() ? 0 : std::min<unsigned>(index, bits->begin() - bits->count()); Ditto like this, since it is hard to read. if (bits->begin() < bits->count()) newBegin = 0; else newBegin = std::min<unsigned>(index, bits->begin() - bits->count()); > Source/bmalloc/bmalloc/ObjectTypeTable.cpp:66 > + newEnd = std::numeric_limits<unsigned>::max() - bits->count() < bits->end() ? std::numeric_limits<unsigned>::max() : std::max<unsigned>(index + 1, bits->end() + bits->count()); Ditto.
Comment on attachment 400068 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400068&action=review >> Source/bmalloc/bmalloc/ObjectTypeTable.cpp:47 >> + newBegin = index < ObjectTypeTable::Bits::bitCountPerWord * 4 ? 0 : std::min<unsigned>(index, index - ObjectTypeTable::Bits::bitCountPerWord * 4); > > Can you wrtie it like, > > constexpr unsigned offsetForInitialAllocation = ObjectTypeTable::Bits::bitCountPerWord * 4; > if (index < offsetForInitialAllocation) > newBegin = 0; > else > newBegin = std::min<unsigned>(index, index - offsetForInitialAllocation); Changed. The only difference I applied was to use `newBegin = index - offsetForInitialAllocation;`, since it is not possible that `index` will ever be lower than `index - offsetForInitialAllocation` when `index >= offsetForInitialAllocation`. >> Source/bmalloc/bmalloc/ObjectTypeTable.cpp:56 >> + newBegin = bits->begin() < bits->count() ? 0 : std::min<unsigned>(index, bits->begin() - bits->count()); > > Ditto like this, since it is hard to read. > > if (bits->begin() < bits->count()) > newBegin = 0; > else > newBegin = std::min<unsigned>(index, bits->begin() - bits->count()); Changed. >> Source/bmalloc/bmalloc/ObjectTypeTable.cpp:66 >> + newEnd = std::numeric_limits<unsigned>::max() - bits->count() < bits->end() ? std::numeric_limits<unsigned>::max() : std::max<unsigned>(index + 1, bits->end() + bits->count()); > > Ditto. Changed.
Created attachment 400120 [details] Patch
Comment on attachment 400120 [details] Patch Thank you very much for the review
Committed r262100: <https://trac.webkit.org/changeset/262100> All reviewed patches have been landed. Closing bug and clearing flags on attachment 400120 [details].
<rdar://problem/63570045>