<!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>[210208] releases/WebKitGTK/webkit-2.14/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/210208">210208</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2016-12-30 01:39:26 -0800 (Fri, 30 Dec 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>Merge <a href="http://trac.webkit.org/projects/webkit/changeset/208018">r208018</a> - JSFunction::put() should not allow caching of lazily reified properties.
https://bugs.webkit.org/show_bug.cgi?id=164081

Reviewed by Geoffrey Garen.

It is incorrect for JSFunction::put() to return PutPropertySlots that indicates
that its lazily reified properties (e.g. .caller, and .arguments) are cacheable.
The reason for this is:

1. Currently, a cacheable put may only consist of the following types of put
   operations:
   a. putting a new property at an offset in the object storage.
   b. changing the value of an existing property at an offset in the object storage.
   c. invoking the setter for a property at an offset in the object storage.

   Returning a PutPropertySlot that indicates the property is cacheable means that
   the property put must be one of the above operations.

   For lazily reified properties, JSFunction::put() implements complex conditional
   behavior that is different than the set of cacheable put operations above.
   Hence, it should not claim that the property put is cacheable.

2. Cacheable puts are cached on the original structure of the object before the
   put operation.

   Reifying a lazy property will trigger a structure transition.  Even though
   subsequent puts to such a property may be cacheable after the structure
   transition, it is incorrect to indicate that the property put is cacheable
   because the caching is on the original structure, not the new transitioned
   structure.

Also fixed some missing exception checks.

* jit/JITOperations.cpp:
* runtime/JSFunction.cpp:
(JSC::JSFunction::put):
(JSC::JSFunction::reifyLazyPropertyIfNeeded):
(JSC::JSFunction::reifyBoundNameIfNeeded):
* runtime/JSFunction.h:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#releasesWebKitGTKwebkit214SourceJavaScriptCoreChangeLog">releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#releasesWebKitGTKwebkit214SourceJavaScriptCorejitJITOperationscpp">releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/jit/JITOperations.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit214SourceJavaScriptCoreruntimeJSFunctioncpp">releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/runtime/JSFunction.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit214SourceJavaScriptCoreruntimeJSFunctionh">releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/runtime/JSFunction.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="releasesWebKitGTKwebkit214SourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/ChangeLog (210207 => 210208)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/ChangeLog        2016-12-30 09:02:51 UTC (rev 210207)
+++ releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/ChangeLog        2016-12-30 09:39:26 UTC (rev 210208)
</span><span class="lines">@@ -1,3 +1,45 @@
</span><ins>+2016-10-27  Mark Lam  &lt;mark.lam@apple.com&gt;
+
+        JSFunction::put() should not allow caching of lazily reified properties.
+        https://bugs.webkit.org/show_bug.cgi?id=164081
+
+        Reviewed by Geoffrey Garen.
+
+        It is incorrect for JSFunction::put() to return PutPropertySlots that indicates
+        that its lazily reified properties (e.g. .caller, and .arguments) are cacheable.
+        The reason for this is:
+
+        1. Currently, a cacheable put may only consist of the following types of put
+           operations:
+           a. putting a new property at an offset in the object storage.
+           b. changing the value of an existing property at an offset in the object storage.
+           c. invoking the setter for a property at an offset in the object storage.
+
+           Returning a PutPropertySlot that indicates the property is cacheable means that
+           the property put must be one of the above operations.
+
+           For lazily reified properties, JSFunction::put() implements complex conditional
+           behavior that is different than the set of cacheable put operations above.
+           Hence, it should not claim that the property put is cacheable.
+    
+        2. Cacheable puts are cached on the original structure of the object before the
+           put operation.
+
+           Reifying a lazy property will trigger a structure transition.  Even though
+           subsequent puts to such a property may be cacheable after the structure
+           transition, it is incorrect to indicate that the property put is cacheable
+           because the caching is on the original structure, not the new transitioned
+           structure.
+
+        Also fixed some missing exception checks.
+
+        * jit/JITOperations.cpp:
+        * runtime/JSFunction.cpp:
+        (JSC::JSFunction::put):
+        (JSC::JSFunction::reifyLazyPropertyIfNeeded):
+        (JSC::JSFunction::reifyBoundNameIfNeeded):
+        * runtime/JSFunction.h:
+
</ins><span class="cx"> 2016-10-12  Joseph Pecoraro  &lt;pecoraro@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Web Inspector: Improve support for logging Proxy objects in console
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit214SourceJavaScriptCorejitJITOperationscpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/jit/JITOperations.cpp (210207 => 210208)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/jit/JITOperations.cpp        2016-12-30 09:02:51 UTC (rev 210207)
+++ releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/jit/JITOperations.cpp        2016-12-30 09:39:26 UTC (rev 210208)
</span><span class="lines">@@ -201,6 +201,9 @@
</span><span class="cx">     PropertySlot slot(baseValue, PropertySlot::InternalMethodType::VMInquiry);
</span><span class="cx"> 
</span><span class="cx">     baseValue.getPropertySlot(exec, ident, slot);
</span><ins>+    if (UNLIKELY(vm-&gt;exception()))
+        return encodedJSValue();
+
</ins><span class="cx">     if (stubInfo-&gt;considerCaching(baseValue.structureOrNull()) &amp;&amp; !slot.isTaintedByOpaqueObject() &amp;&amp; (slot.isCacheableValue() || slot.isCacheableGetter() || slot.isUnset()))
</span><span class="cx">         repatchGetByID(exec, baseValue, ident, slot, *stubInfo, GetByIDKind::Pure);
</span><span class="cx"> 
</span><span class="lines">@@ -398,7 +401,9 @@
</span><span class="cx"> 
</span><span class="cx">     Structure* structure = baseValue.isCell() ? baseValue.asCell()-&gt;structure(*vm) : nullptr;
</span><span class="cx">     baseValue.putInline(exec, ident, value, slot);
</span><del>-    
</del><ins>+    if (UNLIKELY(vm-&gt;exception()))
+        return;
+
</ins><span class="cx">     if (accessType != static_cast&lt;AccessType&gt;(stubInfo-&gt;accessType))
</span><span class="cx">         return;
</span><span class="cx">     
</span><span class="lines">@@ -423,7 +428,9 @@
</span><span class="cx"> 
</span><span class="cx">     Structure* structure = baseValue.isCell() ? baseValue.asCell()-&gt;structure(*vm) : nullptr;    
</span><span class="cx">     baseValue.putInline(exec, ident, value, slot);
</span><del>-    
</del><ins>+    if (UNLIKELY(vm-&gt;exception()))
+        return;
+
</ins><span class="cx">     if (accessType != static_cast&lt;AccessType&gt;(stubInfo-&gt;accessType))
</span><span class="cx">         return;
</span><span class="cx">     
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit214SourceJavaScriptCoreruntimeJSFunctioncpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/runtime/JSFunction.cpp (210207 => 210208)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/runtime/JSFunction.cpp        2016-12-30 09:02:51 UTC (rev 210207)
+++ releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/runtime/JSFunction.cpp        2016-12-30 09:39:26 UTC (rev 210208)
</span><span class="lines">@@ -437,23 +437,26 @@
</span><span class="cx">         return ordinarySetSlow(exec, thisObject, propertyName, value, slot.thisValue(), slot.isStrictMode());
</span><span class="cx"> 
</span><span class="cx">     if (thisObject-&gt;isHostOrBuiltinFunction()) {
</span><del>-        thisObject-&gt;reifyBoundNameIfNeeded(vm, exec, propertyName);
</del><ins>+        LazyPropertyType propType = thisObject-&gt;reifyBoundNameIfNeeded(vm, exec, propertyName);
+        if (propType == LazyPropertyType::IsLazyProperty)
+            slot.disableCaching();
</ins><span class="cx">         return Base::put(thisObject, exec, propertyName, value, slot);
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     if (propertyName == vm.propertyNames-&gt;prototype) {
</span><ins>+        slot.disableCaching();
</ins><span class="cx">         // Make sure prototype has been reified, such that it can only be overwritten
</span><span class="cx">         // following the rules set out in ECMA-262 8.12.9.
</span><del>-        PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
-        thisObject-&gt;methodTable(vm)-&gt;getOwnPropertySlot(thisObject, exec, propertyName, slot);
</del><ins>+        PropertySlot getSlot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
+        thisObject-&gt;methodTable(vm)-&gt;getOwnPropertySlot(thisObject, exec, propertyName, getSlot);
</ins><span class="cx">         if (thisObject-&gt;m_rareData)
</span><span class="cx">             thisObject-&gt;m_rareData-&gt;clear(&quot;Store to prototype property of a function&quot;);
</span><del>-        // Don't allow this to be cached, since a [[Put]] must clear m_rareData.
-        PutPropertySlot dontCache(thisObject);
</del><span class="cx">         scope.release();
</span><del>-        return Base::put(thisObject, exec, propertyName, value, dontCache);
</del><ins>+        return Base::put(thisObject, exec, propertyName, value, slot);
</ins><span class="cx">     }
</span><ins>+
</ins><span class="cx">     if (thisObject-&gt;jsExecutable()-&gt;isStrictMode() &amp;&amp; (propertyName == vm.propertyNames-&gt;arguments || propertyName == vm.propertyNames-&gt;caller)) {
</span><ins>+        slot.disableCaching();
</ins><span class="cx">         // This will trigger the property to be reified, if this is not already the case!
</span><span class="cx">         bool okay = thisObject-&gt;hasProperty(exec, propertyName);
</span><span class="cx">         ASSERT_UNUSED(okay, okay);
</span><span class="lines">@@ -461,11 +464,14 @@
</span><span class="cx">         return Base::put(thisObject, exec, propertyName, value, slot);
</span><span class="cx">     }
</span><span class="cx">     if (propertyName == vm.propertyNames-&gt;arguments || propertyName == vm.propertyNames-&gt;caller) {
</span><ins>+        slot.disableCaching();
</ins><span class="cx">         if (slot.isStrictMode())
</span><span class="cx">             throwTypeError(exec, scope, StrictModeReadonlyPropertyWriteError);
</span><span class="cx">         return false;
</span><span class="cx">     }
</span><del>-    thisObject-&gt;reifyLazyPropertyIfNeeded(vm, exec, propertyName);
</del><ins>+    LazyPropertyType propType = thisObject-&gt;reifyLazyPropertyIfNeeded(vm, exec, propertyName);
+    if (propType == LazyPropertyType::IsLazyProperty)
+        slot.disableCaching();
</ins><span class="cx">     scope.release();
</span><span class="cx">     return Base::put(thisObject, exec, propertyName, value, slot);
</span><span class="cx"> }
</span><span class="lines">@@ -671,25 +677,28 @@
</span><span class="cx">     rareData-&gt;setHasReifiedName();
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void JSFunction::reifyLazyPropertyIfNeeded(VM&amp; vm, ExecState* exec, PropertyName propertyName)
</del><ins>+JSFunction::LazyPropertyType JSFunction::reifyLazyPropertyIfNeeded(VM&amp; vm, ExecState* exec, PropertyName propertyName)
</ins><span class="cx"> {
</span><span class="cx">     if (propertyName == vm.propertyNames-&gt;length) {
</span><span class="cx">         if (!hasReifiedLength())
</span><span class="cx">             reifyLength(vm);
</span><ins>+        return LazyPropertyType::IsLazyProperty;
</ins><span class="cx">     } else if (propertyName == vm.propertyNames-&gt;name) {
</span><span class="cx">         if (!hasReifiedName())
</span><span class="cx">             reifyName(vm, exec);
</span><ins>+        return LazyPropertyType::IsLazyProperty;
</ins><span class="cx">     }
</span><ins>+    return LazyPropertyType::NotLazyProperty;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><del>-void JSFunction::reifyBoundNameIfNeeded(VM&amp; vm, ExecState* exec, PropertyName propertyName)
</del><ins>+JSFunction::LazyPropertyType JSFunction::reifyBoundNameIfNeeded(VM&amp; vm, ExecState* exec, PropertyName propertyName)
</ins><span class="cx"> {
</span><span class="cx">     const Identifier&amp; nameIdent = vm.propertyNames-&gt;name;
</span><span class="cx">     if (propertyName != nameIdent)
</span><del>-        return;
</del><ins>+        return LazyPropertyType::NotLazyProperty;
</ins><span class="cx"> 
</span><span class="cx">     if (hasReifiedName())
</span><del>-        return;
</del><ins>+        return LazyPropertyType::IsLazyProperty;
</ins><span class="cx"> 
</span><span class="cx">     if (this-&gt;inherits(JSBoundFunction::info())) {
</span><span class="cx">         FunctionRareData* rareData = this-&gt;rareData(vm);
</span><span class="lines">@@ -698,6 +707,7 @@
</span><span class="cx">         putDirect(vm, nameIdent, jsString(exec, name), initialAttributes);
</span><span class="cx">         rareData-&gt;setHasReifiedName();
</span><span class="cx">     }
</span><ins>+    return LazyPropertyType::IsLazyProperty;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit214SourceJavaScriptCoreruntimeJSFunctionh"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/runtime/JSFunction.h (210207 => 210208)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/runtime/JSFunction.h        2016-12-30 09:02:51 UTC (rev 210207)
+++ releases/WebKitGTK/webkit-2.14/Source/JavaScriptCore/runtime/JSFunction.h        2016-12-30 09:39:26 UTC (rev 210208)
</span><span class="lines">@@ -191,10 +191,12 @@
</span><span class="cx">     bool hasReifiedName() const;
</span><span class="cx">     void reifyLength(VM&amp;);
</span><span class="cx">     void reifyName(VM&amp;, ExecState*);
</span><del>-    void reifyBoundNameIfNeeded(VM&amp;, ExecState*, PropertyName);
</del><span class="cx">     void reifyName(VM&amp;, ExecState*, String name);
</span><del>-    void reifyLazyPropertyIfNeeded(VM&amp;, ExecState*, PropertyName propertyName);
</del><span class="cx"> 
</span><ins>+    enum class LazyPropertyType { NotLazyProperty, IsLazyProperty };
+    LazyPropertyType reifyLazyPropertyIfNeeded(VM&amp;, ExecState*, PropertyName);
+    LazyPropertyType reifyBoundNameIfNeeded(VM&amp;, ExecState*, PropertyName);
+
</ins><span class="cx">     friend class LLIntOffsetsExtractor;
</span><span class="cx"> 
</span><span class="cx">     static EncodedJSValue argumentsGetter(ExecState*, EncodedJSValue, PropertyName);
</span></span></pre>
</div>
</div>

</body>
</html>