From b1202e094234da00123d189982e136fcd2933f1b Mon Sep 17 00:00:00 2001 From: Roy Tam Date: Tue, 20 Mar 2018 21:37:26 +0800 Subject: [PATCH] Cherry-picking from pcxfirefox patches repo: Bug 453200 - _cairo_hash_table_lookup_internal is O(n) slower than it should be Bug 616491 - Large number of groups in regex causes too-much-recursion crash (YARR) Bug 767939 - Editing a bookmark and changing URL drops tags. r=dietrich a=gavin Bug 872971 - Clamp regexp quantifiers to INT_MAX. r=jandem Bug 895964 - "Could not create service for entry Disk Space Watcher Service" Bug 941837 - Reduce JS heap fragmentation. r=billm, a=lsblakk Bug 941892 - Take an early return if nsPluginStreamListenerPeer gets passed an unknown stream. r=bsmedberg, a=bajaj Bug 1015780 - Make Moz2D's GetAlignedStride() faster --- .../base/nsPluginStreamListenerPeer.cpp | 6 +- .../windowwatcher/src/nsWindowWatcher.cpp | 29 +++++-- gfx/2d/Tools.h | 7 +- gfx/cairo/cairo/src/cairo-hash.c | 87 +++++++++++-------- js/src/gc/Memory.cpp | 10 +-- js/src/yarr/YarrParser.h | 11 +++ js/src/yarr/YarrPattern.cpp | 27 ++++++ .../diskspacewatcher/DiskSpaceWatcher.cpp | 2 + toolkit/components/places/PlacesUtils.jsm | 2 +- 9 files changed, 123 insertions(+), 58 deletions(-) diff --git a/dom/plugins/base/nsPluginStreamListenerPeer.cpp b/dom/plugins/base/nsPluginStreamListenerPeer.cpp index dbb68d0c9..50230838d 100644 --- a/dom/plugins/base/nsPluginStreamListenerPeer.cpp +++ b/dom/plugins/base/nsPluginStreamListenerPeer.cpp @@ -788,8 +788,10 @@ NS_IMETHODIMP nsPluginStreamListenerPeer::OnDataAvailable(nsIRequest *request, uint64_t sourceOffset, uint32_t aLength) { - NS_ASSERTION(mRequests.IndexOfObject(GetBaseRequest(request)) != -1, - "Received OnDataAvailable for untracked request."); + if (mRequests.IndexOfObject(GetBaseRequest(request)) == -1) { + MOZ_ASSERT(false, "Received OnDataAvailable for untracked request."); + return NS_ERROR_UNEXPECTED; + } if (mRequestFailed) return NS_ERROR_FAILURE; diff --git a/embedding/components/windowwatcher/src/nsWindowWatcher.cpp b/embedding/components/windowwatcher/src/nsWindowWatcher.cpp index 246340116..a0fcf38b6 100644 --- a/embedding/components/windowwatcher/src/nsWindowWatcher.cpp +++ b/embedding/components/windowwatcher/src/nsWindowWatcher.cpp @@ -2016,26 +2016,37 @@ nsWindowWatcher::SizeOpenedDocShellItem(nsIDocShellTreeItem *aDocShellItem, /* Unlike position, force size out-of-bounds check only if size actually was specified. Otherwise, intrinsically sized windows are broken. */ - if (height < 100) + if (height < 100) { height = 100; - if (winHeight > screenHeight) + winHeight = height + (sizeChromeHeight ? 0 : chromeHeight); + } + if (winHeight > screenHeight) { height = screenHeight - (sizeChromeHeight ? 0 : chromeHeight); - if (width < 100) + } + if (width < 100) { width = 100; - if (winWidth > screenWidth) + winWidth = width + (sizeChromeWidth ? 0 : chromeWidth); + } + if (winWidth > screenWidth) { width = screenWidth - (sizeChromeWidth ? 0 : chromeWidth); + } } - if (left + winWidth > screenLeft + screenWidth) + if (left + winWidth > screenLeft + screenWidth || left + winWidth < left) { left = screenLeft + screenWidth - winWidth; - if (left < screenLeft) + } + if (left < screenLeft) { left = screenLeft; - if (top + winHeight > screenTop + screenHeight) + } + if (top + winHeight > screenTop + screenHeight || top + winHeight < top) { top = screenTop + screenHeight - winHeight; - if (top < screenTop) + } + if (top < screenTop) { top = screenTop; - if (top != oldTop || left != oldLeft) + } + if (top != oldTop || left != oldLeft) { positionSpecified = true; + } } } diff --git a/gfx/2d/Tools.h b/gfx/2d/Tools.h index 0aca4eddf..1583183e7 100644 --- a/gfx/2d/Tools.h +++ b/gfx/2d/Tools.h @@ -132,11 +132,8 @@ struct AlignedArray template int32_t GetAlignedStride(int32_t aStride) { - if (aStride % alignment) { - return aStride + (alignment - (aStride % alignment)); - } - - return aStride; + const int32_t mask = alignment - 1; + return (aStride + mask) & ~mask; } } diff --git a/gfx/cairo/cairo/src/cairo-hash.c b/gfx/cairo/cairo/src/cairo-hash.c index 81a48a235..a5fa8dd9a 100644 --- a/gfx/cairo/cairo/src/cairo-hash.c +++ b/gfx/cairo/cairo/src/cairo-hash.c @@ -59,21 +59,20 @@ #define ENTRY_IS_DEAD(entry) ((entry) == DEAD_ENTRY) #define ENTRY_IS_LIVE(entry) ((entry) > DEAD_ENTRY) -/* We expect keys will not be destroyed frequently, so our table does not - * contain any explicit shrinking code nor any chain-coalescing code for - * entries randomly deleted by memory pressure (except during rehashing, of - * course). These assumptions are potentially bad, but they make the - * implementation straightforward. +/* + * This table is open-addressed with double hashing. Each table size + * is a prime and it makes for the "first" hash modulus; a second + * prime (2 less than the first prime) serves as the "second" hash + * modulus, which is smaller and thus guarantees a complete + * permutation of table indices. * - * Revisit later if evidence appears that we're using excessive memory from - * a mostly-dead table. + * Hash tables are rehashed in order to keep between 12.5% and 50% + * entries in the hash table alive and at least 25% free. When table + * size is changed, the new table has about 25% live elements. * - * This table is open-addressed with double hashing. Each table size is a - * prime chosen to be a little more than double the high water mark for a - * given arrangement, so the tables should remain < 50% full. The table - * size makes for the "first" hash modulus; a second prime (2 less than the - * first prime) serves as the "second" hash modulus, which is co-prime and - * thus guarantees a complete permutation of table indices. + * The free entries guarantee an expected constant-time lookup. + * Doubling/halving the table in the described fashion guarantees + * amortized O(1) insertion/removal. * * This structure, and accompanying table, is borrowed/modified from the * file xserver/render/glyph.c in the freedesktop.org x server, with @@ -124,6 +123,7 @@ struct _cairo_hash_table { cairo_hash_entry_t **entries; unsigned long live_entries; + unsigned long free_entries; unsigned long iterating; /* Iterating, no insert, no resize */ }; @@ -167,6 +167,7 @@ _cairo_hash_table_create (cairo_hash_keys_equal_func_t keys_equal) } hash_table->live_entries = 0; + hash_table->free_entries = hash_table->arrangement->size; hash_table->iterating = 0; return hash_table; @@ -236,44 +237,54 @@ _cairo_hash_table_lookup_unique_key (cairo_hash_table_t *hash_table, } /** - * _cairo_hash_table_resize: + * _cairo_hash_table_manage: * @hash_table: a hash table * * Resize the hash table if the number of entries has gotten much * bigger or smaller than the ideal number of entries for the current - * size. + * size and guarantee some free entries to be used as lookup + * termination points. * * Return value: %CAIRO_STATUS_SUCCESS if successful or * %CAIRO_STATUS_NO_MEMORY if out of memory. **/ static cairo_status_t -_cairo_hash_table_resize (cairo_hash_table_t *hash_table) +_cairo_hash_table_manage (cairo_hash_table_t *hash_table) { cairo_hash_table_t tmp; unsigned long new_size, i; - /* This keeps the hash table between 25% and 50% full. */ - unsigned long high = hash_table->arrangement->high_water_mark; - unsigned long low = high >> 2; - - if (hash_table->live_entries >= low && hash_table->live_entries <= high) - return CAIRO_STATUS_SUCCESS; + /* Keep between 12.5% and 50% entries in the hash table alive and + * at least 25% free. */ + unsigned long live_high = hash_table->arrangement->size >> 1; + unsigned long live_low = live_high >> 2; + unsigned long free_low = live_high >> 1; tmp = *hash_table; - if (hash_table->live_entries > high) + if (hash_table->live_entries > live_high) { tmp.arrangement = hash_table->arrangement + 1; /* This code is being abused if we can't make a table big enough. */ assert (tmp.arrangement - hash_table_arrangements < NUM_HASH_TABLE_ARRANGEMENTS); } - else /* hash_table->live_entries < low */ + else if (hash_table->live_entries < live_low) { /* Can't shrink if we're at the smallest size */ if (hash_table->arrangement == &hash_table_arrangements[0]) - return CAIRO_STATUS_SUCCESS; - tmp.arrangement = hash_table->arrangement - 1; + tmp.arrangement = hash_table->arrangement; + else + tmp.arrangement = hash_table->arrangement - 1; + } + + if (tmp.arrangement == hash_table->arrangement && + hash_table->free_entries > free_low) + { + /* The number of live entries is within the desired bounds + * (we're not going to resize the table) and we have enough + * free entries. Do nothing. */ + return CAIRO_STATUS_SUCCESS; } new_size = tmp.arrangement->size; @@ -291,6 +302,7 @@ _cairo_hash_table_resize (cairo_hash_table_t *hash_table) free (hash_table->entries); hash_table->entries = tmp.entries; hash_table->arrangement = tmp.arrangement; + hash_table->free_entries = new_size - hash_table->live_entries; return CAIRO_STATUS_SUCCESS; } @@ -340,6 +352,7 @@ _cairo_hash_table_lookup (cairo_hash_table_t *hash_table, return NULL; } while (++i < table_size); + ASSERT_NOT_REACHED; return NULL; } @@ -421,21 +434,23 @@ cairo_status_t _cairo_hash_table_insert (cairo_hash_table_t *hash_table, cairo_hash_entry_t *key_and_value) { + cairo_hash_entry_t **entry; cairo_status_t status; /* Insert is illegal while an iterator is running. */ assert (hash_table->iterating == 0); - hash_table->live_entries++; - status = _cairo_hash_table_resize (hash_table); - if (unlikely (status)) { - /* abort the insert... */ - hash_table->live_entries--; + status = _cairo_hash_table_manage (hash_table); + if (unlikely (status)) return status; - } - *_cairo_hash_table_lookup_unique_key (hash_table, - key_and_value) = key_and_value; + entry = _cairo_hash_table_lookup_unique_key (hash_table, key_and_value); + + if (ENTRY_IS_FREE (*entry)) + hash_table->free_entries--; + + *entry = key_and_value; + hash_table->live_entries++; return CAIRO_STATUS_SUCCESS; } @@ -496,7 +511,7 @@ _cairo_hash_table_remove (cairo_hash_table_t *hash_table, * memory to shrink the hash table. It does leave the table in a * consistent state, and we've already succeeded in removing the * entry, so we don't examine the failure status of this call. */ - _cairo_hash_table_resize (hash_table); + _cairo_hash_table_manage (hash_table); } } @@ -537,6 +552,6 @@ _cairo_hash_table_foreach (cairo_hash_table_t *hash_table, if (--hash_table->iterating == 0) { /* Should we fail to shrink the hash table, it is left unaltered, * and we don't need to propagate the error status. */ - _cairo_hash_table_resize (hash_table); + _cairo_hash_table_manage (hash_table); } } diff --git a/js/src/gc/Memory.cpp b/js/src/gc/Memory.cpp index 4336bc309..a2d7bd765 100644 --- a/js/src/gc/Memory.cpp +++ b/js/src/gc/Memory.cpp @@ -54,9 +54,9 @@ gc::MapAlignedPages(JSRuntime *rt, size_t size, size_t alignment) void *p = NULL; while (!p) { /* - * Over-allocate in order to map a memory region that is - * definitely large enough then deallocate and allocate again the - * correct sizee, within the over-sized mapping. + * Over-allocate in order to map a memory region that is definitely + * large enough, then deallocate and allocate again the correct size, + * within the over-sized mapping. * * Since we're going to unmap the whole thing anyway, the first * mapping doesn't have to commit pages. @@ -64,7 +64,7 @@ gc::MapAlignedPages(JSRuntime *rt, size_t size, size_t alignment) p = VirtualAlloc(NULL, size * 2, MEM_RESERVE, PAGE_READWRITE); if (!p) return NULL; - void *chunkStart = (void *)(uintptr_t(p) + (alignment - (uintptr_t(p) % alignment))); + void *chunkStart = (void *)AlignBytes(uintptr_t(p), alignment); UnmapPages(rt, p, size * 2); p = VirtualAlloc(chunkStart, size, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); @@ -241,7 +241,7 @@ gc::MapAlignedPages(JSRuntime *rt, size_t size, size_t alignment) uintptr_t offset = uintptr_t(region) % alignment; JS_ASSERT(offset < reqSize - size); - void *front = (void *)(uintptr_t(region) + (alignment - offset)); + void *front = (void *)AlignBytes(uintptr_t(region), alignment); void *end = (void *)(uintptr_t(front) + size); if (front != region) JS_ALWAYS_TRUE(0 == munmap(region, alignment - offset)); diff --git a/js/src/yarr/YarrParser.h b/js/src/yarr/YarrParser.h index ba8fc205a..412c535f7 100644 --- a/js/src/yarr/YarrParser.h +++ b/js/src/yarr/YarrParser.h @@ -621,12 +621,23 @@ private: unsigned min; if (!consumeNumber(min)) break; + // Clamping to INT_MAX is technically a spec deviation. In practice, it's + // undetectable, because we can't even allocate strings large enough for + // quantifiers this large to ever create different results than smaller ones. + if (min > INT_MAX) + min = INT_MAX; unsigned max = min; if (tryConsume(',')) { if (peekIsDigit()) { if (!consumeNumber(max)) break; + // Clamping to INT_MAX is technically a spec deviation. In practice, + // it's undetectable, because we can't even allocate strings large + // enough for quantifiers this large to ever create different results + // than smaller ones. + if (max > INT_MAX) + max = INT_MAX; } else { max = quantifyInfinite; } diff --git a/js/src/yarr/YarrPattern.cpp b/js/src/yarr/YarrPattern.cpp index 658d4eaa4..6437dedc0 100644 --- a/js/src/yarr/YarrPattern.cpp +++ b/js/src/yarr/YarrPattern.cpp @@ -277,6 +277,7 @@ class YarrPatternConstructor { public: YarrPatternConstructor(YarrPattern& pattern) : m_pattern(pattern) + , m_stackBase(nullptr) , m_characterClassConstructor(pattern.m_ignoreCase) , m_invertParentheticalAssertion(false) { @@ -298,6 +299,22 @@ public: m_alternative = m_pattern.m_body->addNewAlternative(); m_pattern.m_disjunctions.append(m_pattern.m_body); } + + void setStackBase(uint8_t *stackBase) { + m_stackBase = stackBase; + } + + bool isOverRecursed() { + /* + * Bug 616491: attempt detection of over-recursion. + * "256KB should be enough stack for anyone." + */ + uint8_t stackDummy_; + JS_ASSERT(m_stackBase != nullptr); + if (m_stackBase - &stackDummy_ > (1 << 18)) + return true; + return false; + } void assertionBOL() { @@ -569,6 +586,9 @@ public: ErrorCode setupAlternativeOffsets(PatternAlternative* alternative, unsigned currentCallFrameSize, unsigned initialInputPosition, unsigned *callFrameSizeOut) { + if (isOverRecursed()) + return PatternTooLarge; + alternative->m_hasFixedSize = true; Checked currentInputPosition = initialInputPosition; @@ -661,6 +681,9 @@ public: ErrorCode setupDisjunctionOffsets(PatternDisjunction* disjunction, unsigned initialCallFrameSize, unsigned initialInputPosition, unsigned *maximumCallFrameSizeOut) { + if (isOverRecursed()) + return PatternTooLarge; + if ((disjunction != m_pattern.m_body) && (disjunction->m_alternatives.size() > 1)) initialCallFrameSize += YarrStackSpaceForBackTrackInfoAlternative; @@ -836,6 +859,7 @@ public: private: YarrPattern& m_pattern; + uint8_t * m_stackBase; PatternAlternative* m_alternative; CharacterClassConstructor m_characterClassConstructor; bool m_invertCharacterClass; @@ -846,6 +870,9 @@ ErrorCode YarrPattern::compile(const String& patternString) { YarrPatternConstructor constructor(*this); + uint8_t stackDummy_; + constructor.setStackBase(&stackDummy_); + if (ErrorCode error = parse(constructor, patternString)) return error; diff --git a/toolkit/components/diskspacewatcher/DiskSpaceWatcher.cpp b/toolkit/components/diskspacewatcher/DiskSpaceWatcher.cpp index 231a50c93..a905a44b4 100644 --- a/toolkit/components/diskspacewatcher/DiskSpaceWatcher.cpp +++ b/toolkit/components/diskspacewatcher/DiskSpaceWatcher.cpp @@ -144,7 +144,9 @@ static const mozilla::Module::ContractIDEntry kDiskSpaceWatcherContracts[] = { }; static const mozilla::Module::CategoryEntry kDiskSpaceWatcherCategories[] = { +#ifdef MOZ_WIDGET_GONK { "profile-after-change", "Disk Space Watcher Service", DISKSPACEWATCHER_CONTRACTID }, +#endif { nullptr } }; diff --git a/toolkit/components/places/PlacesUtils.jsm b/toolkit/components/places/PlacesUtils.jsm index 236191200..d12a6f2ec 100644 --- a/toolkit/components/places/PlacesUtils.jsm +++ b/toolkit/components/places/PlacesUtils.jsm @@ -2675,7 +2675,7 @@ PlacesEditBookmarkURITransaction.prototype = { // only untag the old URI if this is the only bookmark if (PlacesUtils.getBookmarksForURI(this.item.uri, {}).length == 0) PlacesUtils.tagging.untagURI(this.item.uri, this.item.tags); - PlacesUtils.tagging.tagURI(this.new.URI, this.item.tags); + PlacesUtils.tagging.tagURI(this.new.uri, this.item.tags); } },