<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[210947] trunk/Source/JavaScriptCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/210947">210947</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2017-01-19 18:38:45 -0800 (Thu, 19 Jan 2017)</dd>
</dl>

<h3>Log Message</h3>
<pre>Structure::pin() needs to be called while holding a lock
https://bugs.webkit.org/show_bug.cgi?id=167220

Reviewed by Saam Barati.

Imagine this race: the mutator calls pin() and the collector calls visitChildren(),
on the same Structure at the same time. In trunk pin() does not require a lock to be
held and it doesn't grab any locks. Meanwhile visitChildren() grabs the lock, checks
if the structure is pinned, and if not, it removes it by overwriting with zero. Now
imagine how this plays out when pin() runs. Since pin() grabs no locks, it is
irrelevant that visitChildren() grabs any locks. So, visitChildren() might check if
the table is pinned before pin() pins it, and then clear the table after it was
already pinned.

The problem here is that pin() should be holding a lock. We could either make pin()
grab that lock by itself, or what this patch does is makes the caller grab the lock.
This is great because it means that sometimes we don't have to introduce any new
locking.

This fixes a materializePropertyTable() checkOffsetConsistency() crash that happens
very rarely, but I was able to get it to reproduce with run-webkit-tests and
aggressive GC settings.

* runtime/ConcurrentJSLock.h:
* runtime/Structure.cpp:
(JSC::Structure::materializePropertyTable):
(JSC::Structure::changePrototypeTransition):
(JSC::Structure::attributeChangeTransition):
(JSC::Structure::toDictionaryTransition):
(JSC::Structure::nonPropertyTransition):
(JSC::Structure::pin):
(JSC::Structure::pinForCaching):
(JSC::Structure::add):
* runtime/Structure.h:
* runtime/StructureInlines.h:
(JSC::Structure::checkOffsetConsistency):
(JSC::Structure::add):
(JSC::Structure::addPropertyWithoutTransition):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeConcurrentJSLockh">trunk/Source/JavaScriptCore/runtime/ConcurrentJSLock.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeStructurecpp">trunk/Source/JavaScriptCore/runtime/Structure.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeStructureh">trunk/Source/JavaScriptCore/runtime/Structure.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeStructureInlinesh">trunk/Source/JavaScriptCore/runtime/StructureInlines.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (210946 => 210947)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2017-01-20 01:35:58 UTC (rev 210946)
+++ trunk/Source/JavaScriptCore/ChangeLog        2017-01-20 02:38:45 UTC (rev 210947)
</span><span class="lines">@@ -1,5 +1,46 @@
</span><span class="cx"> 2017-01-19  Filip Pizlo  &lt;fpizlo@apple.com&gt;
</span><span class="cx"> 
</span><ins>+        Structure::pin() needs to be called while holding a lock
+        https://bugs.webkit.org/show_bug.cgi?id=167220
+
+        Reviewed by Saam Barati.
+
+        Imagine this race: the mutator calls pin() and the collector calls visitChildren(),
+        on the same Structure at the same time. In trunk pin() does not require a lock to be
+        held and it doesn't grab any locks. Meanwhile visitChildren() grabs the lock, checks
+        if the structure is pinned, and if not, it removes it by overwriting with zero. Now
+        imagine how this plays out when pin() runs. Since pin() grabs no locks, it is
+        irrelevant that visitChildren() grabs any locks. So, visitChildren() might check if
+        the table is pinned before pin() pins it, and then clear the table after it was
+        already pinned.
+
+        The problem here is that pin() should be holding a lock. We could either make pin()
+        grab that lock by itself, or what this patch does is makes the caller grab the lock.
+        This is great because it means that sometimes we don't have to introduce any new
+        locking.
+
+        This fixes a materializePropertyTable() checkOffsetConsistency() crash that happens
+        very rarely, but I was able to get it to reproduce with run-webkit-tests and
+        aggressive GC settings.
+
+        * runtime/ConcurrentJSLock.h:
+        * runtime/Structure.cpp:
+        (JSC::Structure::materializePropertyTable):
+        (JSC::Structure::changePrototypeTransition):
+        (JSC::Structure::attributeChangeTransition):
+        (JSC::Structure::toDictionaryTransition):
+        (JSC::Structure::nonPropertyTransition):
+        (JSC::Structure::pin):
+        (JSC::Structure::pinForCaching):
+        (JSC::Structure::add):
+        * runtime/Structure.h:
+        * runtime/StructureInlines.h:
+        (JSC::Structure::checkOffsetConsistency):
+        (JSC::Structure::add):
+        (JSC::Structure::addPropertyWithoutTransition):
+
+2017-01-19  Filip Pizlo  &lt;fpizlo@apple.com&gt;
+
</ins><span class="cx">         The mutator needs to fire a barrier after memmoving stuff around in an object that the GC scans
</span><span class="cx">         https://bugs.webkit.org/show_bug.cgi?id=167208
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeConcurrentJSLockh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/ConcurrentJSLock.h (210946 => 210947)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/ConcurrentJSLock.h        2017-01-20 01:35:58 UTC (rev 210946)
+++ trunk/Source/JavaScriptCore/runtime/ConcurrentJSLock.h        2017-01-20 02:38:45 UTC (rev 210947)
</span><span class="lines">@@ -40,7 +40,7 @@
</span><span class="cx"> typedef NoLockLocker ConcurrentJSLockerImpl;
</span><span class="cx"> #endif
</span><span class="cx"> 
</span><del>-class ConcurrentJSLockerBase {
</del><ins>+class ConcurrentJSLockerBase : public AbstractLocker {
</ins><span class="cx">     WTF_MAKE_NONCOPYABLE(ConcurrentJSLockerBase);
</span><span class="cx"> public:
</span><span class="cx">     explicit ConcurrentJSLockerBase(ConcurrentJSLock&amp; lockable)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeStructurecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/Structure.cpp (210946 => 210947)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/Structure.cpp        2017-01-20 01:35:58 UTC (rev 210946)
+++ trunk/Source/JavaScriptCore/runtime/Structure.cpp        2017-01-20 02:38:45 UTC (rev 210947)
</span><span class="lines">@@ -357,7 +357,17 @@
</span><span class="cx">         table-&gt;add(entry, m_offset, PropertyTable::PropertyOffsetMustNotChange);
</span><span class="cx">     }
</span><span class="cx">     
</span><del>-    checkOffsetConsistency();
</del><ins>+    checkOffsetConsistency(
+        table,
+        [&amp;] () {
+            dataLog(&quot;Detected in materializePropertyTable.\n&quot;);
+            dataLog(&quot;Found structure = &quot;, RawPointer(structure), &quot;\n&quot;);
+            dataLog(&quot;structures = &quot;);
+            CommaPrinter comma;
+            for (Structure* structure : structures)
+                dataLog(comma, RawPointer(structure));
+            dataLog(&quot;\n&quot;);
+        });
</ins><span class="cx">     
</span><span class="cx">     return table;
</span><span class="cx"> }
</span><span class="lines">@@ -546,8 +556,9 @@
</span><span class="cx">     Structure* transition = create(vm, structure);
</span><span class="cx"> 
</span><span class="cx">     transition-&gt;m_prototype.set(vm, transition, prototype);
</span><del>-    
-    transition-&gt;pin(vm, structure-&gt;copyPropertyTableForPinning(vm));
</del><ins>+
+    PropertyTable* table = structure-&gt;copyPropertyTableForPinning(vm);
+    transition-&gt;pin(holdLock(transition-&gt;m_lock), vm, table);
</ins><span class="cx">     transition-&gt;m_offset = structure-&gt;m_offset;
</span><span class="cx">     
</span><span class="cx">     transition-&gt;checkOffsetConsistency();
</span><span class="lines">@@ -558,8 +569,9 @@
</span><span class="cx"> {
</span><span class="cx">     if (!structure-&gt;isUncacheableDictionary()) {
</span><span class="cx">         Structure* transition = create(vm, structure);
</span><del>-        
-        transition-&gt;pin(vm, structure-&gt;copyPropertyTableForPinning(vm));
</del><ins>+
+        PropertyTable* table = structure-&gt;copyPropertyTableForPinning(vm);
+        transition-&gt;pin(holdLock(transition-&gt;m_lock), vm, table);
</ins><span class="cx">         transition-&gt;m_offset = structure-&gt;m_offset;
</span><span class="cx">         
</span><span class="cx">         structure = transition;
</span><span class="lines">@@ -580,7 +592,8 @@
</span><span class="cx">     
</span><span class="cx">     Structure* transition = create(vm, structure, deferred);
</span><span class="cx"> 
</span><del>-    transition-&gt;pin(vm, structure-&gt;copyPropertyTableForPinning(vm));
</del><ins>+    PropertyTable* table = structure-&gt;copyPropertyTableForPinning(vm);
+    transition-&gt;pin(holdLock(transition-&gt;m_lock), vm, table);
</ins><span class="cx">     transition-&gt;m_offset = structure-&gt;m_offset;
</span><span class="cx">     transition-&gt;setDictionaryKind(kind);
</span><span class="cx">     transition-&gt;setHasBeenDictionary(true);
</span><span class="lines">@@ -667,11 +680,12 @@
</span><span class="cx">         // We pin the property table on transitions that do wholesale editing of the property
</span><span class="cx">         // table, since our logic for walking the property transition chain to rematerialize the
</span><span class="cx">         // table doesn't know how to take into account such wholesale edits.
</span><del>-        
-        transition-&gt;pinForCaching(vm, structure-&gt;copyPropertyTableForPinning(vm));
</del><ins>+
+        PropertyTable* table = structure-&gt;copyPropertyTableForPinning(vm);
+        transition-&gt;pinForCaching(holdLock(transition-&gt;m_lock), vm, table);
</ins><span class="cx">         transition-&gt;m_offset = structure-&gt;m_offset;
</span><span class="cx">         
</span><del>-        PropertyTable* table = transition-&gt;propertyTableOrNull();
</del><ins>+        table = transition-&gt;propertyTableOrNull();
</ins><span class="cx">         RELEASE_ASSERT(table);
</span><span class="cx">         for (auto&amp; entry : *table) {
</span><span class="cx">             if (setsDontDeleteOnAllProperties(transitionKind))
</span><span class="lines">@@ -689,10 +703,11 @@
</span><span class="cx">         &amp;&amp; !transition-&gt;propertyTableOrNull()-&gt;isEmpty())
</span><span class="cx">         transition-&gt;setHasReadOnlyOrGetterSetterPropertiesExcludingProto(true);
</span><span class="cx">     
</span><del>-    if (structure-&gt;isDictionary())
-        transition-&gt;pin(vm, transition-&gt;ensurePropertyTable(vm));
-    else {
-        ConcurrentJSLocker locker(structure-&gt;m_lock);
</del><ins>+    if (structure-&gt;isDictionary()) {
+        PropertyTable* table = transition-&gt;ensurePropertyTable(vm);
+        transition-&gt;pin(holdLock(transition-&gt;m_lock), vm, table);
+    } else {
+        auto locker = holdLock(structure-&gt;m_lock);
</ins><span class="cx">         structure-&gt;m_transitionTable.add(vm, transition);
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="lines">@@ -804,7 +819,7 @@
</span><span class="cx">     return this;
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void Structure::pin(VM&amp; vm, PropertyTable* table)
</del><ins>+void Structure::pin(const AbstractLocker&amp;, VM&amp; vm, PropertyTable* table)
</ins><span class="cx"> {
</span><span class="cx">     setIsPinnedPropertyTable(true);
</span><span class="cx">     setPropertyTable(vm, table);
</span><span class="lines">@@ -812,7 +827,7 @@
</span><span class="cx">     m_nameInPrevious = nullptr;
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void Structure::pinForCaching(VM&amp; vm, PropertyTable* table)
</del><ins>+void Structure::pinForCaching(const AbstractLocker&amp;, VM&amp; vm, PropertyTable* table)
</ins><span class="cx"> {
</span><span class="cx">     setIsPinnedPropertyTable(true);
</span><span class="cx">     setPropertyTable(vm, table);
</span><span class="lines">@@ -978,7 +993,7 @@
</span><span class="cx"> 
</span><span class="cx"> PropertyOffset Structure::add(VM&amp; vm, PropertyName propertyName, unsigned attributes)
</span><span class="cx"> {
</span><del>-    return add(
</del><ins>+    return add&lt;ShouldPin::No&gt;(
</ins><span class="cx">         vm, propertyName, attributes,
</span><span class="cx">         [this] (const GCSafeConcurrentJSLocker&amp;, PropertyOffset, PropertyOffset newLastOffset) {
</span><span class="cx">             setLastOffset(newLastOffset);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeStructureh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/Structure.h (210946 => 210947)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/Structure.h        2017-01-20 01:35:58 UTC (rev 210946)
+++ trunk/Source/JavaScriptCore/runtime/Structure.h        2017-01-20 02:38:45 UTC (rev 210947)
</span><span class="lines">@@ -660,8 +660,9 @@
</span><span class="cx">     void findStructuresAndMapForMaterialization(Vector&lt;Structure*, 8&gt;&amp; structures, Structure*&amp;, PropertyTable*&amp;);
</span><span class="cx">     
</span><span class="cx">     static Structure* toDictionaryTransition(VM&amp;, Structure*, DictionaryKind, DeferredStructureTransitionWatchpointFire* = nullptr);
</span><del>-    
-    template&lt;typename Func&gt;
</del><ins>+
+    enum class ShouldPin { No, Yes };
+    template&lt;ShouldPin, typename Func&gt;
</ins><span class="cx">     PropertyOffset add(VM&amp;, PropertyName, unsigned attributes, const Func&amp;);
</span><span class="cx">     PropertyOffset add(VM&amp;, PropertyName, unsigned attributes);
</span><span class="cx">     template&lt;typename Func&gt;
</span><span class="lines">@@ -725,9 +726,10 @@
</span><span class="cx"> 
</span><span class="cx">     bool isValid(JSGlobalObject*, StructureChain* cachedPrototypeChain) const;
</span><span class="cx">     bool isValid(ExecState*, StructureChain* cachedPrototypeChain) const;
</span><del>-        
-    JS_EXPORT_PRIVATE void pin(VM&amp;, PropertyTable*);
-    void pinForCaching(VM&amp;, PropertyTable*);
</del><ins>+
+    // You have to hold the structure lock to do these.
+    JS_EXPORT_PRIVATE void pin(const AbstractLocker&amp;, VM&amp;, PropertyTable*);
+    void pinForCaching(const AbstractLocker&amp;, VM&amp;, PropertyTable*);
</ins><span class="cx">     
</span><span class="cx">     bool isRareData(JSCell* cell) const
</span><span class="cx">     {
</span><span class="lines">@@ -740,6 +742,8 @@
</span><span class="cx">         return static_cast&lt;StructureRareData*&gt;(m_previousOrRareData.get());
</span><span class="cx">     }
</span><span class="cx"> 
</span><ins>+    template&lt;typename DetailsFunc&gt;
+    bool checkOffsetConsistency(PropertyTable*, const DetailsFunc&amp;) const;
</ins><span class="cx">     bool checkOffsetConsistency() const;
</span><span class="cx"> 
</span><span class="cx">     JS_EXPORT_PRIVATE void allocateRareData(VM&amp;);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeStructureInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/StructureInlines.h (210946 => 210947)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/StructureInlines.h        2017-01-20 01:35:58 UTC (rev 210946)
+++ trunk/Source/JavaScriptCore/runtime/StructureInlines.h        2017-01-20 02:38:45 UTC (rev 210947)
</span><span class="lines">@@ -243,15 +243,9 @@
</span><span class="cx">     return map-&gt;get(offset);
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-ALWAYS_INLINE bool Structure::checkOffsetConsistency() const
</del><ins>+template&lt;typename DetailsFunc&gt;
+ALWAYS_INLINE bool Structure::checkOffsetConsistency(PropertyTable* propertyTable, const DetailsFunc&amp; detailsFunc) const
</ins><span class="cx"> {
</span><del>-    PropertyTable* propertyTable = propertyTableOrNull();
-
-    if (!propertyTable) {
-        ASSERT(!isPinnedPropertyTable());
-        return true;
-    }
-
</del><span class="cx">     // We cannot reliably assert things about the property table in the concurrent
</span><span class="cx">     // compilation thread. It is possible for the table to be stolen and then have
</span><span class="cx">     // things added to it, which leads to the offsets being all messed up. We could
</span><span class="lines">@@ -272,6 +266,7 @@
</span><span class="cx">         dataLog(&quot;totalSize = &quot;, totalSize, &quot;\n&quot;);
</span><span class="cx">         dataLog(&quot;inlineOverflowAccordingToTotalSize = &quot;, inlineOverflowAccordingToTotalSize, &quot;\n&quot;);
</span><span class="cx">         dataLog(&quot;numberOfOutOfLineSlotsForLastOffset = &quot;, numberOfOutOfLineSlotsForLastOffset(m_offset), &quot;\n&quot;);
</span><ins>+        detailsFunc();
</ins><span class="cx">         UNREACHABLE_FOR_PLATFORM();
</span><span class="cx">     };
</span><span class="cx">     
</span><span class="lines">@@ -283,6 +278,25 @@
</span><span class="cx">     return true;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+ALWAYS_INLINE bool Structure::checkOffsetConsistency() const
+{
+    PropertyTable* propertyTable = propertyTableOrNull();
+
+    if (!propertyTable) {
+        ASSERT(!isPinnedPropertyTable());
+        return true;
+    }
+
+    // We cannot reliably assert things about the property table in the concurrent
+    // compilation thread. It is possible for the table to be stolen and then have
+    // things added to it, which leads to the offsets being all messed up. We could
+    // get around this by grabbing a lock here, but I think that would be overkill.
+    if (isCompilationThread())
+        return true;
+
+    return checkOffsetConsistency(propertyTable, [] () { });
+}
+
</ins><span class="cx"> inline void Structure::checkConsistency()
</span><span class="cx"> {
</span><span class="cx">     checkOffsetConsistency();
</span><span class="lines">@@ -302,15 +316,22 @@
</span><span class="cx">     rareData()-&gt;setObjectToStringValue(exec, vm, this, value, toStringTagSymbolSlot);
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-template&lt;typename Func&gt;
</del><ins>+template&lt;Structure::ShouldPin shouldPin, typename Func&gt;
</ins><span class="cx"> inline PropertyOffset Structure::add(VM&amp; vm, PropertyName propertyName, unsigned attributes, const Func&amp; func)
</span><span class="cx"> {
</span><span class="cx">     PropertyTable* table = ensurePropertyTable(vm);
</span><span class="cx"> 
</span><span class="cx">     GCSafeConcurrentJSLocker locker(m_lock, vm.heap);
</span><ins>+
+    switch (shouldPin) {
+    case ShouldPin::Yes:
+        pin(locker, vm, table);
+        break;
+    case ShouldPin::No:
+        setPropertyTable(vm, table);
+        break;
+    }
</ins><span class="cx">     
</span><del>-    setPropertyTable(vm, table);
-    
</del><span class="cx">     ASSERT(!JSC::isValidOffset(get(vm, propertyName)));
</span><span class="cx"> 
</span><span class="cx">     checkConsistency();
</span><span class="lines">@@ -366,9 +387,7 @@
</span><span class="cx"> template&lt;typename Func&gt;
</span><span class="cx"> inline PropertyOffset Structure::addPropertyWithoutTransition(VM&amp; vm, PropertyName propertyName, unsigned attributes, const Func&amp; func)
</span><span class="cx"> {
</span><del>-    pin(vm, ensurePropertyTable(vm));
-    
-    return add(vm, propertyName, attributes, func);
</del><ins>+    return add&lt;ShouldPin::Yes&gt;(vm, propertyName, attributes, func);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template&lt;typename Func&gt;
</span></span></pre>
</div>
</div>

</body>
</html>