Bug 110854 – Unused Structure property tables waste 14MB on Membuster.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Andreas Kling]

<rdar://problem/13292104>

[Andreas Kling]

Created attachment 190221 [details]
Proposed patch

Lay it on me, fellas.

[Filip Pizlo]

Comment on attachment 190221 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=190221&action=review

Basically, r=me, but with the slight changes.

> Source/JavaScriptCore/runtime/PropertyMapHashTable.h:280
> +inline PropertyTable* PropertyTable::create(JSGlobalData& globalData, unsigned initialCapacity)
> +{
> +    PropertyTable* table = new (NotNull, allocateCell<PropertyTable>(globalData.heap)) PropertyTable(globalData, initialCapacity);
> +    table->finishCreation(globalData);
> +    return table;
> +}
> +
> +inline PropertyTable* PropertyTable::clone(JSGlobalData& globalData, JSCell* owner, const PropertyTable& other)
> +{
> +    PropertyTable* table = new (NotNull, allocateCell<PropertyTable>(globalData.heap)) PropertyTable(globalData, owner, other);
> +    table->finishCreation(globalData);
> +    return table;
> +}
> +
> +inline PropertyTable* PropertyTable::clone(JSGlobalData& globalData, JSCell* owner, unsigned initialCapacity, const PropertyTable& other)
> +{
> +    PropertyTable* table = new (NotNull, allocateCell<PropertyTable>(globalData.heap)) PropertyTable(globalData, owner, initialCapacity, other);
> +    table->finishCreation(globalData);
> +    return table;
> +}
> +
> +inline PropertyTable::PropertyTable(JSGlobalData& globalData, unsigned initialCapacity)
> +    : JSCell(globalData, globalData.propertyTableStructure.get())
> +    , m_indexSize(sizeForCapacity(initialCapacity))

While you're creating PropertyMapHashTable.cpp, you could out-of-line these methods.  They do no good being inline.

> Source/JavaScriptCore/runtime/Structure.cpp:78
> +const ClassInfo PropertyTable::s_info = { "PropertyTable", 0, 0, 0, CREATE_METHOD_TABLE(PropertyTable) };
> +
> +void PropertyTable::visitChildren(JSCell* cell, SlotVisitor& visitor)
> +{
> +    PropertyTable* thisObject = jsCast<PropertyTable*>(cell);
> +    ASSERT_GC_OBJECT_INHERITS(thisObject, &s_info);
> +    ASSERT(thisObject->structure()->typeInfo().overridesVisitChildren());
> +
> +    JSCell::visitChildren(thisObject, visitor);
> +
> +    PropertyTable::iterator end = thisObject->end();
> +    for (PropertyTable::iterator ptr = thisObject->begin(); ptr != end; ++ptr)
> +        visitor.append(&ptr->specificValue);
> +}
> +
>  #if DUMP_STRUCTURE_ID_STATISTICS
>  static HashSet<Structure*>& liveStructureSet = *(new HashSet<Structure*>);

These should be in PropertyMapHashTable.cpp.  Or just PropertyTable.cpp.  Your pick of name.  But I wouldn't pollute Structure.cpp with these things.

(I know, adding .cpp files to WebKit is soooper hard, but at least here you don't have to do gyp.  So one less build system.)

> Source/JavaScriptCore/runtime/StructureInlines.h:200
> +inline bool Structure::putWillGrowOutOfLineStorage()
> +{
> +    checkOffsetConsistency();
> +
> +    ASSERT(outOfLineCapacity() >= outOfLineSize());
> +
> +    if (!m_propertyTable) {
> +        unsigned currentSize = numberOfOutOfLineSlotsForLastOffset(m_offset);
> +        ASSERT(outOfLineCapacity() >= currentSize);
> +        return currentSize == outOfLineCapacity();
> +    }
> +
> +    ASSERT(totalStorageCapacity() >= m_propertyTable->propertyStorageSize());
> +    if (m_propertyTable->hasDeletedOffset())
> +        return false;
> +
> +    ASSERT(totalStorageCapacity() >= m_propertyTable->size());
> +    return m_propertyTable->size() == totalStorageCapacity();

Might as well drop this method into Structure.cpp instead of having it be inline.  It meets the basic criteria for not being inline: it's not called often enough and it has a fair bit of code (multiple loads and branches).

[EFL EWS Bot]

Comment on attachment 190221 [details]
Proposed patch

Attachment 190221 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/16744860

[Early Warning System Bot]

Comment on attachment 190221 [details]
Proposed patch

Attachment 190221 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/16752800

[Early Warning System Bot]

Comment on attachment 190221 [details]
Proposed patch

Attachment 190221 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/16747817

[Andreas Kling]

(In reply to comment #2)
> While you're creating PropertyMapHashTable.cpp, you could out-of-line these methods.  They do no good being inline.

Sure.

> These should be in PropertyMapHashTable.cpp.  Or just PropertyTable.cpp.  Your pick of name.  But I wouldn't pollute Structure.cpp with these things.
> 
> (I know, adding .cpp files to WebKit is soooper hard, but at least here you don't have to do gyp.  So one less build system.)

Source/JavaScriptCore/JavaScriptCore.gypi :(
But sure.

> > Source/JavaScriptCore/runtime/StructureInlines.h:200
> > +inline bool Structure::putWillGrowOutOfLineStorage()
> Might as well drop this method into Structure.cpp instead of having it be inline.  It meets the basic criteria for not being inline: it's not called often enough and it has a fair bit of code (multiple loads and branches).

I'm getting some wonky build failures in 'jsc' from out-of-lining that, not sure what's going on. Let's do separately.

More importantly, the patch I uploaded was missing a PropertyTable::destroy(JSCell*), I'm adding that and reuploading for EWS to chew etc.

[Andreas Kling]

Created attachment 190248 [details]
Patch for landing

...if EWS permits.

[Andreas Kling]

Created attachment 190251 [details]
Patch for landing

Not sure why it doesn't apply on EWS, let's try again.

[Andreas Kling]

Created attachment 190263 [details]
ews pls

[Andreas Kling]

Committed r144054: <http://trac.webkit.org/changeset/144054>

[WebKit Review Bot]

Re-opened since this is blocked by bug 110882

[Andreas Kling]

Reverted r144054 for reason:

broke builds

Committed r144056: <http://trac.webkit.org/changeset/144056>

[Andreas Kling]

Created attachment 190287 [details]
Come on EWS, not today.

[Andreas Kling]

Created attachment 190288 [details]
Same patch tweaked for Windows line endings

In case this is a CRLF problem, I tweaked the diff for JavaScriptCore.vcproj

[WebKit Review Bot]

Attachment 190288 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.gypi', u'Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj', u'Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/Target.pri', u'Source/JavaScriptCore/runtime/JSGlobalData.cpp', u'Source/JavaScriptCore/runtime/JSGlobalData.h', u'Source/JavaScriptCore/runtime/PropertyMapHashTable.h', u'Source/JavaScriptCore/runtime/PropertyTable.cpp', u'Source/JavaScriptCore/runtime/Structure.cpp', u'Source/JavaScriptCore/runtime/Structure.h', u'Source/JavaScriptCore/runtime/StructureInlines.h']" exit_code: 1
Source/JavaScriptCore/runtime/PropertyTable.cpp:28:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EFL EWS Bot]

Comment on attachment 190288 [details]
Same patch tweaked for Windows line endings

Attachment 190288 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/16778169

[Early Warning System Bot]

Comment on attachment 190288 [details]
Same patch tweaked for Windows line endings

Attachment 190288 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/16773224

[Early Warning System Bot]

Comment on attachment 190288 [details]
Same patch tweaked for Windows line endings

Attachment 190288 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/16769267

[Andreas Kling]

Created attachment 190293 [details]
Add a cpp file, they said. It's not that hard, they said.

[WebKit Review Bot]

Attachment 190293 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.gypi', u'Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj', u'Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/Target.pri', u'Source/JavaScriptCore/runtime/JSGlobalData.cpp', u'Source/JavaScriptCore/runtime/JSGlobalData.h', u'Source/JavaScriptCore/runtime/PropertyMapHashTable.h', u'Source/JavaScriptCore/runtime/PropertyTable.cpp', u'Source/JavaScriptCore/runtime/Structure.cpp', u'Source/JavaScriptCore/runtime/Structure.h', u'Source/JavaScriptCore/runtime/StructureInlines.h']" exit_code: 1
Source/JavaScriptCore/runtime/PropertyTable.cpp:28:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 190293 [details]
Add a cpp file, they said. It's not that hard, they said.

Clearing flags on attachment: 190293

Committed r144074: <http://trac.webkit.org/changeset/144074>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by bug 110897

[Brady Eidson]

This is causing 20+ crashes and therefore early exits on Mac bots.

[Andreas Kling]

Okay, so if I understand correctly, this broke Structure::checkOffsetConsistency() in debug builds, because Structure::m_propertyTable may now get clear()ed out by one marking thread while this code executes in another. That means that the !m_propertyTable check is not enough to avoid dereferencing a null pointer.

I'm not sure what the best course of action is here. I assume we want to keep these sanity checks somehow. Geoff/Phil/Mark?

Example of a crashing thread:
Thread 11 Crashed:: JavaScriptCore::Marking
0   com.apple.JavaScriptCore      	0x0000000108abb814 JSC::WriteBarrierBase<JSC::PropertyTable>::operator->() const + 84 (WriteBarrier.h:116)
1   com.apple.JavaScriptCore      	0x0000000108abb478 JSC::Structure::checkOffsetConsistency() const + 184 (StructureInlines.h:210)
2   com.apple.JavaScriptCore      	0x0000000108abb5b9 JSC::Structure::outOfLineSize() const + 25 (Structure.h:190)
3   com.apple.JavaScriptCore      	0x0000000108aba80a JSC::Structure::outOfLineCapacity() const + 106 (Structure.h:176)
4   com.apple.JavaScriptCore      	0x0000000108dc33ca JSC::JSObject::visitButterfly(JSC::SlotVisitor&, JSC::Butterfly*, unsigned long) + 138 (JSObject.cpp:172)
5   com.apple.JavaScriptCore      	0x0000000108dba41f JSC::JSFinalObject::visitChildren(JSC::JSCell*, JSC::SlotVisitor&) + 463 (JSObject.cpp:249)
6   com.apple.JavaScriptCore      	0x0000000108ef678e JSC::visitChildren(JSC::SlotVisitor&, JSC::JSCell const*) + 206 (SlotVisitor.cpp:87)
7   com.apple.JavaScriptCore      	0x0000000108ef663f JSC::SlotVisitor::drain() + 255 (SlotVisitor.cpp:136)
8   com.apple.JavaScriptCore      	0x0000000108ef6cc9 JSC::SlotVisitor::drainFromShared(JSC::SlotVisitor::SharedDrainMode) + 1241 (SlotVisitor.cpp:226)
9   com.apple.JavaScriptCore      	0x0000000108ced9b7 JSC::GCThread::gcThreadMain() + 183 (GCThread.cpp:109)
10  com.apple.JavaScriptCore      	0x0000000108cedabd JSC::GCThread::gcThreadStartFunc(void*) + 29 (GCThread.cpp:136)
11  com.apple.JavaScriptCore      	0x0000000108fbd1c0 WTF::threadEntryPoint(void*) + 144 (Threading.cpp:70)
12  com.apple.JavaScriptCore      	0x0000000108fbdbb8 WTF::wtfThreadEntryPoint(void*) + 104 (ThreadingPthreads.cpp:197)
13  libsystem_c.dylib             	0x00007fff8a2b2742 _pthread_start + 327
14  libsystem_c.dylib             	0x00007fff8a29f181 thread_start + 13

[Geoffrey Garen]

> Okay, so if I understand correctly, this broke Structure::checkOffsetConsistency() in debug builds, because Structure::m_propertyTable may now get clear()ed out by one marking thread while this code executes in another.

My reading is that this bug only effects checkOffsetConsistency(). I think you could fix it by using a local temporary for m_propertyTable, instead of re-reading the data member. That way, you'll get null or non-null consistently throughout the function, which should be fine.

Perhaps you could beef up my reading by making a propertyTable() helper function, having that function ASSERT that the GC is not running, and then allowing checkOffsetConsistency() to call propertyTableUnsafe(), which does not ASSERT, or something like that.

[Andreas Kling]

(In reply to comment #26)
> > Okay, so if I understand correctly, this broke Structure::checkOffsetConsistency() in debug builds, because Structure::m_propertyTable may now get clear()ed out by one marking thread while this code executes in another.
> 
> My reading is that this bug only effects checkOffsetConsistency(). I think you could fix it by using a local temporary for m_propertyTable, instead of re-reading the data member. That way, you'll get null or non-null consistently throughout the function, which should be fine.
> 
> Perhaps you could beef up my reading by making a propertyTable() helper function, having that function ASSERT that the GC is not running, and then allowing checkOffsetConsistency() to call propertyTableUnsafe(), which does not ASSERT, or something like that.

Sure, could do something like this for checkOffsetConsistency():

inline const PropertyTable* Structure::propertyTableUnsafe() const
{
    return static_cast<const PropertyTable*>(*const_cast<Structure*>(this)->m_propertyTable.slot());
}

What can I assert in Structure::propertyTable() to verify that GC isn't in progress? Heap::isBusy()?

[Geoffrey Garen]

> inline const PropertyTable* Structure::propertyTableUnsafe() const
> {
>     return static_cast<const PropertyTable*>(*const_cast<Structure*>(this)->m_propertyTable.slot());
> }

Are you trying to get around multiple read problems in WriteBarrier<T>::get()? How about just "return m_propertyTable.get()", and fix WriteBarrier<T>::get() to put m_cell in a local variable, too, with a comment as to why.

> What can I assert in Structure::propertyTable() to verify that GC isn't in progress? Heap::isBusy()?

Yup!

[Eric Seidel (no email)]

Comment on attachment 190221 [details]
Proposed patch

Cleared Filip Pizlo's review+ from obsolete attachment 190221 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Andreas Kling]

(In reply to comment #28)
> > What can I assert in Structure::propertyTable() to verify that GC isn't in progress? Heap::isBusy()?
> 
> Yup!

Looks like this would necessitate passing a JSGlobalData& to propertyTable() everywhere, which gets pretty ugly. Do we still want to do it?

[Geoffrey Garen]

> Looks like this would necessitate passing a JSGlobalData& to propertyTable() everywhere, which gets pretty ugly. Do we still want to do it?

If that's too cumbersome, you can do this instead:

ASSERT(!globalObject() || !globalObject()->globalData()->heap.isBusy())

Every Structure has a pointer to the global object from which it originated, except for a few special permanent structures that don't belong to any global object.

[Andreas Kling]

Created attachment 191298 [details]
Patch II: The Repatchening

Yeah okay, let's try this again.

[WebKit Review Bot]

Attachment 191298 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.gypi', u'Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/Target.pri', u'Source/JavaScriptCore/runtime/JSGlobalData.cpp', u'Source/JavaScriptCore/runtime/JSGlobalData.h', u'Source/JavaScriptCore/runtime/PropertyMapHashTable.h', u'Source/JavaScriptCore/runtime/PropertyTable.cpp', u'Source/JavaScriptCore/runtime/Structure.cpp', u'Source/JavaScriptCore/runtime/Structure.h', u'Source/JavaScriptCore/runtime/StructureInlines.h', u'Source/JavaScriptCore/runtime/WriteBarrier.h']" exit_code: 1
Source/JavaScriptCore/runtime/PropertyTable.cpp:28:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 191298 [details]
Patch II: The Repatchening

View in context: https://bugs.webkit.org/attachment.cgi?id=191298&action=review

r=me

> Source/JavaScriptCore/runtime/Structure.h:457
> +    // Should be accessed through propertyTable().

Would be nice to elaborate in this comment that what makes m_propertyTableUnsafe unsafe is that, during GC, it may be set to 0 by another thread.

> Source/JavaScriptCore/runtime/WriteBarrier.h:102
> +        // Copy cell to a local to avoid multiple-read issues. See <http://webkit.org/b/110854>)

Missing a parenthesis here.

[Andreas Kling]

Created attachment 191323 [details]
Patch for lunar landing

[WebKit Review Bot]

Comment on attachment 191323 [details]
Patch for lunar landing

Rejecting attachment 191323 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-02', 'validate-changelog', '--non-interactive', 191323, '--port=chromium-xvfb']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Geoff Garen found in /mnt/git/webkit-commit-queue/Source/JavaScriptCore/ChangeLog does not appear to be a valid reviewer according to committers.py.
/mnt/git/webkit-commit-queue/Source/JavaScriptCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-commit-queue.appspot.com/results/16903145

[Andreas Kling]

Created attachment 191324 [details]
Geoffrey not Geoff.

[WebKit Review Bot]

Comment on attachment 191324 [details]
Geoffrey not Geoff.

Clearing flags on attachment: 191324

Committed r144708: <http://trac.webkit.org/changeset/144708>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by bug 111447

[Andreas Kling]

0xbbadbeef
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x0000000101dd797d JSC::PropertyTable::add(JSC::PropertyMapEntry const&, int&, JSC::PropertyTable::EffectOnPropertyOffset) + 509 (PropertyMapHashTable.h:374)
1   com.apple.JavaScriptCore      	0x0000000101dd4fa4 JSC::Structure::materializePropertyMap(JSC::JSGlobalData&) + 660 (Structure.cpp:265)
2   com.apple.JavaScriptCore      	0x0000000101dd55a8 JSC::Structure::addPropertyTransition(JSC::JSGlobalData&, JSC::Structure*, JSC::PropertyName, unsigned int, JSC::JSCell*, int&) + 744 (Structure.cpp:386)
3   com.apple.JavaScriptCore      	0x0000000101b49255 bool JSC::JSObject::putDirectInternal<(JSC::JSObject::PutMode)1>(JSC::JSGlobalData&, JSC::PropertyName, JSC::JSValue, unsigned int, JSC::PutPropertySlot&, JSC::JSCell*) + 901 (JSObject.h:1320)
4   com.apple.JavaScriptCore      	0x0000000101d3ed76 llint_slow_path_put_by_id + 262 (JSObject.h:1375)
5   com.apple.JavaScriptCore      	0x0000000101d46bb0 llint_op_put_by_id + 133
6   com.apple.JavaScriptCore      	0x0000000101c6fded JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) + 4253 (JSCJSValueInlines.h:363)
7   com.apple.JavaScriptCore      	0x0000000101b9533b JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*) + 619 (Completion.cpp:75)
8   com.apple.WebCore             	0x0000000102fb141a WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld*) + 442 (JSMainThreadExecState.h:77)
9   com.apple.WebCore             	0x0000000102fb15a9 WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&) + 41 (ScriptController.cpp:158)
10  com.apple.WebCore             	0x0000000102fba854 WebCore::ScriptElement::executeScript(WebCore::ScriptSourceCode const&) + 500 (ScriptValue.h:52)
11  com.apple.WebCore             	0x00000001028c5164 WebCore::HTMLScriptRunner::executePendingScriptAndDispatchEvent(WebCore::PendingScript&) + 228 (HTMLScriptRunner.cpp:144)
12  com.apple.WebCore             	0x00000001028c5063 WebCore::HTMLScriptRunner::executeParsingBlockingScript() + 259 (HTMLScriptRunner.cpp:119)
13  com.apple.WebCore             	0x00000001028c5748 WebCore::HTMLScriptRunner::executeParsingBlockingScripts() + 24 (RefPtr.h:58)
14  com.apple.WebCore             	0x00000001028704d4 WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder() + 84 (PassRefPtr.h:68)
15  com.apple.WebCore             	0x0000000102870558 WebCore::HTMLDocumentParser::canTakeNextToken(WebCore::HTMLDocumentParser::SynchronousMode, WebCore::PumpSession&) + 88 (HTMLDocumentParser.cpp:287)
16  com.apple.WebCore             	0x0000000102870236 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) + 390 (HTMLDocumentParser.cpp:520)
17  com.apple.WebCore             	0x0000000102870b22 WebCore::HTMLDocumentParser::append(WTF::PassRefPtr<WTF::StringImpl>) + 498 (HTMLDocumentParser.cpp:732)
18  com.apple.WebCore             	0x00000001026189e5 WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter*, char const*, unsigned long) + 117 (PassRefPtr.h:68)
19  com.apple.WebCore             	0x000000010264d29c WebCore::DocumentLoader::commitData(char const*, unsigned long) + 508 (DocumentLoader.cpp:376)
20  com.apple.WebKit              	0x00000001021ab4a4 -[WebHTMLRepresentation receivedData:withDataSource:] + 100 (WebHTMLRepresentation.mm:186)
21  com.apple.WebKit              	0x000000010217e69d -[WebDataSource(WebInternal) _receivedData:] + 77 (WebDataSource.mm:216)
22  com.apple.WebKit              	0x00000001021964f7 WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) + 103 (WebFrameLoaderClient.mm:848)
23  com.apple.WebCore             	0x000000010264d340 WebCore::DocumentLoader::commitLoad(char const*, int) + 144 (RefCounted.h:148)
24  com.apple.WebCore             	0x0000000102d66ccb WebCore::MainResourceLoader::dataReceived(WebCore::CachedResource*, char const*, int) + 715 (RefCounted.h:148)
25  com.apple.WebCore             	0x00000001024ec472 WebCore::CachedRawResource::didAddClient(WebCore::CachedResourceClient*) + 802 (HashTable.h:996)
26  com.apple.WebCore             	0x000000010317c78f WebCore::ThreadTimers::sharedTimerFiredInternal() + 175 (ThreadTimers.cpp:132)
27  com.apple.WebCore             	0x0000000103006cf3 _ZN7WebCoreL10timerFiredEP16__CFRunLoopTimerPv + 51 (SharedTimerMac.mm:167)
28  com.apple.CoreFoundation      	0x00007fff8b610934 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
29  com.apple.CoreFoundation      	0x00007fff8b610486 __CFRunLoopDoTimer + 534
30  com.apple.CoreFoundation      	0x00007fff8b5f0e11 __CFRunLoopRun + 1617
31  com.apple.CoreFoundation      	0x00007fff8b5f0486 CFRunLoopRunSpecific + 230
32  com.apple.Foundation          	0x00007fff884baf7b -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 267
33  DumpRenderTree                	0x0000000101a310be _ZL7runTestRKNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEE + 1639 (DumpRenderTree.mm:1375)
34  DumpRenderTree                	0x0000000101a30852 dumpRenderTree(int, char const**) + 1727 (DumpRenderTree.mm:832)
35  DumpRenderTree                	0x0000000101a31427 main + 86 (DumpRenderTree.mm:925)
36  DumpRenderTree                	0x0000000101a25024 start + 52

[Andreas Kling]

Relanded in <https://trac.webkit.org/r144910>.

The issue was that transition->m_offset was not always initialized prior to calling transition->materializePropertyMap(). It was trivial to reproduce before with COLLECT_ON_EVERY_ALLOCATION.

Added a method to share the "take ownership of existing property map or clone it if it's pinned" logic between addPropertyTransition() and nonPropertyTransition().