<!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>[168510] 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/168510">168510</a></dd>
<dt>Author</dt> <dd>mhahnenberg@apple.com</dd>
<dt>Date</dt> <dd>2014-05-08 17:22:30 -0700 (Thu, 08 May 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>Base case for get-by-id inline cache doesn't check for HasImpureGetOwnPropertySlot
https://bugs.webkit.org/show_bug.cgi?id=132695
Reviewed by Filip Pizlo.
We check in the case where we're accessing something other than the base object (e.g. the prototype),
but we fail to do so for the base object.
* jit/Repatch.cpp:
(JSC::tryCacheGetByID):
(JSC::tryBuildGetByIDList):
* jsc.cpp: Added some infrastructure to support this test. We don't currently trigger this bug anywhere in WebKit
because all of the values that are returned that could be impure are set to uncacheable anyways.
(WTF::ImpureGetter::ImpureGetter):
(WTF::ImpureGetter::createStructure):
(WTF::ImpureGetter::create):
(WTF::ImpureGetter::finishCreation):
(WTF::ImpureGetter::getOwnPropertySlot):
(WTF::ImpureGetter::visitChildren):
(WTF::ImpureGetter::setDelegate):
(GlobalObject::finishCreation):
(functionCreateImpureGetter):
(functionSetImpureGetterDelegate):
* tests/stress/impure-get-own-property-slot-inline-cache.js: Added.
(foo):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorejitRepatchcpp">trunk/Source/JavaScriptCore/jit/Repatch.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejsccpp">trunk/Source/JavaScriptCore/jsc.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoretestsstressimpuregetownpropertyslotinlinecachejs">trunk/Source/JavaScriptCore/tests/stress/impure-get-own-property-slot-inline-cache.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (168509 => 168510)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2014-05-09 00:22:05 UTC (rev 168509)
+++ trunk/Source/JavaScriptCore/ChangeLog 2014-05-09 00:22:30 UTC (rev 168510)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+2014-05-08 Mark Hahnenberg <mhahnenberg@apple.com>
+
+ Base case for get-by-id inline cache doesn't check for HasImpureGetOwnPropertySlot
+ https://bugs.webkit.org/show_bug.cgi?id=132695
+
+ Reviewed by Filip Pizlo.
+
+ We check in the case where we're accessing something other than the base object (e.g. the prototype),
+ but we fail to do so for the base object.
+
+ * jit/Repatch.cpp:
+ (JSC::tryCacheGetByID):
+ (JSC::tryBuildGetByIDList):
+ * jsc.cpp: Added some infrastructure to support this test. We don't currently trigger this bug anywhere in WebKit
+ because all of the values that are returned that could be impure are set to uncacheable anyways.
+ (WTF::ImpureGetter::ImpureGetter):
+ (WTF::ImpureGetter::createStructure):
+ (WTF::ImpureGetter::create):
+ (WTF::ImpureGetter::finishCreation):
+ (WTF::ImpureGetter::getOwnPropertySlot):
+ (WTF::ImpureGetter::visitChildren):
+ (WTF::ImpureGetter::setDelegate):
+ (GlobalObject::finishCreation):
+ (functionCreateImpureGetter):
+ (functionSetImpureGetterDelegate):
+ * tests/stress/impure-get-own-property-slot-inline-cache.js: Added.
+ (foo):
+
</ins><span class="cx"> 2014-05-08 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> deleteAllCompiledCode() shouldn't use the suspension worklist
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitRepatchcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/Repatch.cpp (168509 => 168510)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/Repatch.cpp 2014-05-09 00:22:05 UTC (rev 168509)
+++ trunk/Source/JavaScriptCore/jit/Repatch.cpp 2014-05-09 00:22:30 UTC (rev 168510)
</span><span class="lines">@@ -685,6 +685,9 @@
</span><span class="cx"> return false;
</span><span class="cx"> if (!structure->propertyAccessesAreCacheable())
</span><span class="cx"> return false;
</span><ins>+ TypeInfo typeInfo = structure->typeInfo();
+ if (typeInfo.hasImpureGetOwnPropertySlot() && !typeInfo.newImpurePropertyFiresWatchpoints())
+ return false;
</ins><span class="cx">
</span><span class="cx"> // Optimize self access.
</span><span class="cx"> if (slot.slotBase() == baseValue
</span><span class="lines">@@ -744,6 +747,10 @@
</span><span class="cx"> if (!structure->propertyAccessesAreCacheable())
</span><span class="cx"> return false;
</span><span class="cx">
</span><ins>+ TypeInfo typeInfo = structure->typeInfo();
+ if (typeInfo.hasImpureGetOwnPropertySlot() && !typeInfo.newImpurePropertyFiresWatchpoints())
+ return false;
+
</ins><span class="cx"> if (stubInfo.patch.spillMode == NeedToSpill) {
</span><span class="cx"> // We cannot do as much inline caching if the registers were not flushed prior to this GetById. In particular,
</span><span class="cx"> // non-Value cached properties require planting calls, which requires registers to have been flushed. Thus,
</span><span class="lines">@@ -757,8 +764,7 @@
</span><span class="cx"> size_t count = 0;
</span><span class="cx">
</span><span class="cx"> if (slot.slotBase() != baseValue) {
</span><del>- if (baseValue.asCell()->structure()->typeInfo().prohibitsPropertyCaching()
- || baseValue.asCell()->structure()->isDictionary())
</del><ins>+ if (typeInfo.prohibitsPropertyCaching() || structure->isDictionary())
</ins><span class="cx"> return false;
</span><span class="cx">
</span><span class="cx"> count = normalizePrototypeChainForChainAccess(
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejsccpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jsc.cpp (168509 => 168510)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jsc.cpp 2014-05-09 00:22:05 UTC (rev 168509)
+++ trunk/Source/JavaScriptCore/jsc.cpp 2014-05-09 00:22:30 UTC (rev 168510)
</span><span class="lines">@@ -219,9 +219,67 @@
</span><span class="cx"> Weak<Element> m_element;
</span><span class="cx"> };
</span><span class="cx">
</span><ins>+class ImpureGetter : public JSNonFinalObject {
+public:
+ ImpureGetter(VM& vm, Structure* structure)
+ : Base(vm, structure)
+ {
+ }
+
+ DECLARE_INFO;
+ typedef JSNonFinalObject Base;
+
+ static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
+ {
+ return Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info());
+ }
+
+ static ImpureGetter* create(VM& vm, Structure* structure, JSObject* delegate)
+ {
+ ImpureGetter* getter = new (NotNull, allocateCell<ImpureGetter>(vm.heap, sizeof(ImpureGetter))) ImpureGetter(vm, structure);
+ getter->finishCreation(vm, delegate);
+ return getter;
+ }
+
+ void finishCreation(VM& vm, JSObject* delegate)
+ {
+ Base::finishCreation(vm);
+ if (delegate)
+ m_delegate.set(vm, this, delegate);
+ }
+
+ static const unsigned StructureFlags = JSC::HasImpureGetOwnPropertySlot | JSC::OverridesGetOwnPropertySlot | JSC::OverridesVisitChildren | Base::StructureFlags;
+
+ static bool getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName name, PropertySlot& slot)
+ {
+ ImpureGetter* thisObject = jsCast<ImpureGetter*>(object);
+
+ if (thisObject->m_delegate && thisObject->m_delegate->getPropertySlot(exec, name, slot))
+ return true;
+
+ return Base::getOwnPropertySlot(object, exec, name, slot);
+ }
+
+ static void visitChildren(JSCell* cell, SlotVisitor& visitor)
+ {
+ Base::visitChildren(cell, visitor);
+ ImpureGetter* thisObject = jsCast<ImpureGetter*>(cell);
+ visitor.append(&thisObject->m_delegate);
+ }
+
+ void setDelegate(VM& vm, JSObject* delegate)
+ {
+ m_delegate.set(vm, this, delegate);
+ }
+
+private:
+ WriteBarrier<JSObject> m_delegate;
+};
+
</ins><span class="cx"> const ClassInfo Element::s_info = { "Element", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(Element) };
</span><span class="cx"> const ClassInfo Masquerader::s_info = { "Masquerader", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(Masquerader) };
</span><span class="cx"> const ClassInfo Root::s_info = { "Root", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(Root) };
</span><ins>+const ClassInfo ImpureGetter::s_info = { "ImpureGetter", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(ImpureGetter) };
</ins><span class="cx">
</span><span class="cx"> ElementHandleOwner* Element::handleOwner()
</span><span class="cx"> {
</span><span class="lines">@@ -242,6 +300,8 @@
</span><span class="cx"> static bool fillBufferWithContentsOfFile(const String& fileName, Vector<char>& buffer);
</span><span class="cx">
</span><span class="cx"> static EncodedJSValue JSC_HOST_CALL functionCreateProxy(ExecState*);
</span><ins>+static EncodedJSValue JSC_HOST_CALL functionCreateImpureGetter(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionSetImpureGetterDelegate(ExecState*);
</ins><span class="cx">
</span><span class="cx"> static EncodedJSValue JSC_HOST_CALL functionSetElementRoot(ExecState*);
</span><span class="cx"> static EncodedJSValue JSC_HOST_CALL functionCreateRoot(ExecState*);
</span><span class="lines">@@ -421,6 +481,9 @@
</span><span class="cx"> addFunction(vm, "makeMasquerader", functionMakeMasquerader, 0);
</span><span class="cx">
</span><span class="cx"> addFunction(vm, "createProxy", functionCreateProxy, 1);
</span><ins>+
+ addFunction(vm, "createImpureGetter", functionCreateImpureGetter, 1);
+ addFunction(vm, "setImpureGetterDelegate", functionSetImpureGetterDelegate, 2);
</ins><span class="cx">
</span><span class="cx"> JSArray* array = constructEmptyArray(globalExec(), 0);
</span><span class="cx"> for (size_t i = 0; i < arguments.size(); ++i)
</span><span class="lines">@@ -591,6 +654,32 @@
</span><span class="cx"> return JSValue::encode(proxy);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+EncodedJSValue JSC_HOST_CALL functionCreateImpureGetter(ExecState* exec)
+{
+ JSLockHolder lock(exec);
+ JSValue target = exec->argument(0);
+ JSObject* delegate = nullptr;
+ if (target.isObject())
+ delegate = asObject(target.asCell());
+ Structure* structure = ImpureGetter::createStructure(exec->vm(), exec->lexicalGlobalObject(), jsNull());
+ ImpureGetter* result = ImpureGetter::create(exec->vm(), structure, delegate);
+ return JSValue::encode(result);
+}
+
+EncodedJSValue JSC_HOST_CALL functionSetImpureGetterDelegate(ExecState* exec)
+{
+ JSLockHolder lock(exec);
+ JSValue base = exec->argument(0);
+ if (!base.isObject())
+ return JSValue::encode(jsUndefined());
+ JSValue delegate = exec->argument(1);
+ if (!delegate.isObject())
+ return JSValue::encode(jsUndefined());
+ ImpureGetter* impureGetter = jsCast<ImpureGetter*>(asObject(base.asCell()));
+ impureGetter->setDelegate(exec->vm(), asObject(delegate.asCell()));
+ return JSValue::encode(jsUndefined());
+}
+
</ins><span class="cx"> EncodedJSValue JSC_HOST_CALL functionGCAndSweep(ExecState* exec)
</span><span class="cx"> {
</span><span class="cx"> JSLockHolder lock(exec);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstressimpuregetownpropertyslotinlinecachejs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/impure-get-own-property-slot-inline-cache.js (0 => 168510)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/impure-get-own-property-slot-inline-cache.js (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/impure-get-own-property-slot-inline-cache.js 2014-05-09 00:22:30 UTC (rev 168510)
</span><span class="lines">@@ -0,0 +1,18 @@
</span><ins>+
+
+var ig = createImpureGetter(null);
+ig.x = 42;
+
+var foo = function(o) {
+ return o.x;
+};
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i)
+ foo(ig);
+
+setImpureGetterDelegate(ig, {x:"x"});
+
+if (foo(ig) !== "x")
+ throw new Error("Incorrect result!");
</ins></span></pre>
</div>
</div>
</body>
</html>