<!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>[201654] 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/201654">201654</a></dd>
<dt>Author</dt> <dd>barraclough@apple.com</dd>
<dt>Date</dt> <dd>2016-06-03 13:12:25 -0700 (Fri, 03 Jun 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>JSGlobalObject::addFunction should call deleteProperty rather than removeDirect
https://bugs.webkit.org/show_bug.cgi?id=158295
Reviewed by Saam Barati.
When a function in declared in program code, this replaces any previosly existing
property from the global object. JSGlobalObject::addFunction is currently calling
removeDirect rather than deleteProperty to remove the existing property. This fails
to remove any properties from static tables.
We currently get away with this because (a) JSObject & JSGlobalObject don't currently
have any properties in static tables, and (b) the current quirky property precedence
means that the symbol table properties end up taking precedence over JSDOMWindow's
static table, so window object properties end up being shadowed.
As a part of bug #158178 the precedence of static tables will change, requiring this
to be fixed.
The deleteProperty function does what we want (has the ability to remove properties,
including those from the static tables). Normally deleteProperty will not remove
properties that are non-configurable (DontDelete) - we need to do so. The function
does already support this, through a flag on VM named 'isInDefineOwnProperty', which
causes configurability to be ignored. Generalize this mechanism for use outside of
defineOwnProperty, renaming & moving DefineOwnPropertyScope helper class out to VM.
* runtime/JSFunction.cpp:
(JSC::JSFunction::deleteProperty):
- isInDefineOwnProperty -> deletePropertyMode.
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::addFunction):
- removeDirect -> deleteProperty.
* runtime/JSObject.cpp:
(JSC::JSObject::deleteProperty):
- isInDefineOwnProperty -> deletePropertyMode.
(JSC::JSObject::defineOwnNonIndexProperty):
- DefineOwnPropertyScope -> VM::DeletePropertyModeScope.
(JSC::DefineOwnPropertyScope::DefineOwnPropertyScope): Deleted.
(JSC::DefineOwnPropertyScope::~DefineOwnPropertyScope): Deleted.
- DefineOwnPropertyScope -> VM::DeletePropertyModeScope.
* runtime/VM.cpp:
(JSC::VM::VM):
- removed m_inDefineOwnProperty.
* runtime/VM.h:
(JSC::VM::deletePropertyMode):
- isInDefineOwnProperty -> deletePropertyMode.
(JSC::VM::DeletePropertyModeScope::DeletePropertyModeScope):
(JSC::VM::DeletePropertyModeScope::~DeletePropertyModeScope):
- DefineOwnPropertyScope -> VM::DeletePropertyModeScope.
(JSC::VM::setInDefineOwnProperty): Deleted.
- Replaced with deletePropertyMode, can now only be set via VM::DeletePropertyModeScope.
(JSC::VM::isInDefineOwnProperty): Deleted.
- isInDefineOwnProperty -> deletePropertyMode.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSFunctioncpp">trunk/Source/JavaScriptCore/runtime/JSFunction.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSGlobalObjectcpp">trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSObjectcpp">trunk/Source/JavaScriptCore/runtime/JSObject.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeVMcpp">trunk/Source/JavaScriptCore/runtime/VM.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeVMh">trunk/Source/JavaScriptCore/runtime/VM.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (201653 => 201654)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-06-03 20:12:25 UTC (rev 201654)
</span><span class="lines">@@ -1,3 +1,58 @@
</span><ins>+2016-06-02 Gavin & Ellie Barraclough <barraclough@apple.com>
+
+ JSGlobalObject::addFunction should call deleteProperty rather than removeDirect
+ https://bugs.webkit.org/show_bug.cgi?id=158295
+
+ Reviewed by Saam Barati.
+
+ When a function in declared in program code, this replaces any previosly existing
+ property from the global object. JSGlobalObject::addFunction is currently calling
+ removeDirect rather than deleteProperty to remove the existing property. This fails
+ to remove any properties from static tables.
+
+ We currently get away with this because (a) JSObject & JSGlobalObject don't currently
+ have any properties in static tables, and (b) the current quirky property precedence
+ means that the symbol table properties end up taking precedence over JSDOMWindow's
+ static table, so window object properties end up being shadowed.
+
+ As a part of bug #158178 the precedence of static tables will change, requiring this
+ to be fixed.
+
+ The deleteProperty function does what we want (has the ability to remove properties,
+ including those from the static tables). Normally deleteProperty will not remove
+ properties that are non-configurable (DontDelete) - we need to do so. The function
+ does already support this, through a flag on VM named 'isInDefineOwnProperty', which
+ causes configurability to be ignored. Generalize this mechanism for use outside of
+ defineOwnProperty, renaming & moving DefineOwnPropertyScope helper class out to VM.
+
+ * runtime/JSFunction.cpp:
+ (JSC::JSFunction::deleteProperty):
+ - isInDefineOwnProperty -> deletePropertyMode.
+ * runtime/JSGlobalObject.cpp:
+ (JSC::JSGlobalObject::addFunction):
+ - removeDirect -> deleteProperty.
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::deleteProperty):
+ - isInDefineOwnProperty -> deletePropertyMode.
+ (JSC::JSObject::defineOwnNonIndexProperty):
+ - DefineOwnPropertyScope -> VM::DeletePropertyModeScope.
+ (JSC::DefineOwnPropertyScope::DefineOwnPropertyScope): Deleted.
+ (JSC::DefineOwnPropertyScope::~DefineOwnPropertyScope): Deleted.
+ - DefineOwnPropertyScope -> VM::DeletePropertyModeScope.
+ * runtime/VM.cpp:
+ (JSC::VM::VM):
+ - removed m_inDefineOwnProperty.
+ * runtime/VM.h:
+ (JSC::VM::deletePropertyMode):
+ - isInDefineOwnProperty -> deletePropertyMode.
+ (JSC::VM::DeletePropertyModeScope::DeletePropertyModeScope):
+ (JSC::VM::DeletePropertyModeScope::~DeletePropertyModeScope):
+ - DefineOwnPropertyScope -> VM::DeletePropertyModeScope.
+ (JSC::VM::setInDefineOwnProperty): Deleted.
+ - Replaced with deletePropertyMode, can now only be set via VM::DeletePropertyModeScope.
+ (JSC::VM::isInDefineOwnProperty): Deleted.
+ - isInDefineOwnProperty -> deletePropertyMode.
+
</ins><span class="cx"> 2016-06-03 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><span class="cx"> ARMv7 vstm and vldm instructions can only operate on a maximum of 16 registers.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSFunctioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSFunction.cpp (201653 => 201654)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSFunction.cpp 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/JavaScriptCore/runtime/JSFunction.cpp 2016-06-03 20:12:25 UTC (rev 201654)
</span><span class="lines">@@ -458,7 +458,7 @@
</span><span class="cx"> {
</span><span class="cx"> JSFunction* thisObject = jsCast<JSFunction*>(cell);
</span><span class="cx"> // For non-host functions, don't let these properties by deleted - except by DefineOwnProperty.
</span><del>- if (!thisObject->isHostOrBuiltinFunction() && !exec->vm().isInDefineOwnProperty()) {
</del><ins>+ if (!thisObject->isHostOrBuiltinFunction() && exec->vm().deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable) {
</ins><span class="cx"> FunctionExecutable* executable = thisObject->jsExecutable();
</span><span class="cx"> if (propertyName == exec->propertyNames().arguments
</span><span class="cx"> || (propertyName == exec->propertyNames().prototype && !executable->isArrowFunction())
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSGlobalObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp (201653 => 201654)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp 2016-06-03 20:12:25 UTC (rev 201654)
</span><span class="lines">@@ -857,7 +857,8 @@
</span><span class="cx"> void JSGlobalObject::addFunction(ExecState* exec, const Identifier& propertyName)
</span><span class="cx"> {
</span><span class="cx"> VM& vm = exec->vm();
</span><del>- removeDirect(vm, propertyName); // Newly declared functions overwrite existing properties.
</del><ins>+ VM::DeletePropertyModeScope scope(vm, VM::DeletePropertyMode::IgnoreConfigurable);
+ methodTable(vm)->deleteProperty(this, exec, propertyName);
</ins><span class="cx"> addGlobalVar(propertyName);
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSObject.cpp (201653 => 201654)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSObject.cpp 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/JavaScriptCore/runtime/JSObject.cpp 2016-06-03 20:12:25 UTC (rev 201654)
</span><span class="lines">@@ -1497,7 +1497,7 @@
</span><span class="cx">
</span><span class="cx"> if (!thisObject->staticFunctionsReified()) {
</span><span class="cx"> if (auto* entry = thisObject->findPropertyHashEntry(propertyName)) {
</span><del>- if (entry->attributes() & DontDelete)
</del><ins>+ if (entry->attributes() & DontDelete && vm.deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable)
</ins><span class="cx"> return false;
</span><span class="cx"> thisObject->reifyAllStaticProperties(exec);
</span><span class="cx"> }
</span><span class="lines">@@ -1505,7 +1505,7 @@
</span><span class="cx">
</span><span class="cx"> unsigned attributes;
</span><span class="cx"> if (isValidOffset(thisObject->structure(vm)->get(vm, propertyName, attributes))) {
</span><del>- if (attributes & DontDelete && !vm.isInDefineOwnProperty())
</del><ins>+ if (attributes & DontDelete && vm.deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable)
</ins><span class="cx"> return false;
</span><span class="cx"> thisObject->removeDirect(vm, propertyName);
</span><span class="cx"> }
</span><span class="lines">@@ -2933,24 +2933,6 @@
</span><span class="cx"> return putDirect(exec->vm(), propertyName, value);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-class DefineOwnPropertyScope {
-public:
- DefineOwnPropertyScope(ExecState* exec)
- : m_vm(exec->vm())
- {
- m_vm.setInDefineOwnProperty(true);
- }
-
- ~DefineOwnPropertyScope()
- {
- m_vm.setInDefineOwnProperty(false);
- }
-
-private:
- VM& m_vm;
-};
-
-
</del><span class="cx"> // 9.1.6.3 of the spec
</span><span class="cx"> // http://www.ecma-international.org/ecma-262/6.0/index.html#sec-validateandapplypropertydescriptor
</span><span class="cx"> bool validateAndApplyPropertyDescriptor(ExecState* exec, JSObject* object, PropertyName propertyName, bool isExtensible,
</span><span class="lines">@@ -3104,7 +3086,7 @@
</span><span class="cx"> // Currently DefineOwnProperty uses delete to remove properties when they are being replaced
</span><span class="cx"> // (particularly when changing attributes), however delete won't allow non-configurable (i.e.
</span><span class="cx"> // DontDelete) properties to be deleted. For now, we can use this flag to make this work.
</span><del>- DefineOwnPropertyScope scope(exec);
</del><ins>+ VM::DeletePropertyModeScope scope(exec->vm(), VM::DeletePropertyMode::IgnoreConfigurable);
</ins><span class="cx"> PropertyDescriptor current;
</span><span class="cx"> bool isCurrentDefined = getOwnPropertyDescriptor(exec, propertyName, current);
</span><span class="cx"> bool isExtensible = this->isExtensible(exec);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeVMcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/VM.cpp (201653 => 201654)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/VM.cpp 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/JavaScriptCore/runtime/VM.cpp 2016-06-03 20:12:25 UTC (rev 201654)
</span><span class="lines">@@ -188,7 +188,6 @@
</span><span class="cx"> #if !ENABLE(JIT)
</span><span class="cx"> , m_jsStackLimit(0)
</span><span class="cx"> #endif
</span><del>- , m_inDefineOwnProperty(false)
</del><span class="cx"> , m_codeCache(std::make_unique<CodeCache>())
</span><span class="cx"> , m_builtinExecutables(std::make_unique<BuiltinExecutables>(*this))
</span><span class="cx"> , m_typeProfilerEnabledCount(0)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeVMh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/VM.h (201653 => 201654)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/VM.h 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/JavaScriptCore/runtime/VM.h 2016-06-03 20:12:25 UTC (rev 201654)
</span><span class="lines">@@ -344,16 +344,38 @@
</span><span class="cx"> AtomicStringTable* atomicStringTable() const { return m_atomicStringTable; }
</span><span class="cx"> WTF::SymbolRegistry& symbolRegistry() { return m_symbolRegistry; }
</span><span class="cx">
</span><del>- void setInDefineOwnProperty(bool inDefineOwnProperty)
- {
- m_inDefineOwnProperty = inDefineOwnProperty;
- }
</del><ins>+ enum class DeletePropertyMode {
+ // Default behaviour of deleteProperty, matching the spec.
+ Default,
+ // This setting causes deleteProperty to force deletion of all
+ // properties including those that are non-configurable (DontDelete).
+ IgnoreConfigurable
+ };
</ins><span class="cx">
</span><del>- bool isInDefineOwnProperty()
</del><ins>+ DeletePropertyMode deletePropertyMode()
</ins><span class="cx"> {
</span><del>- return m_inDefineOwnProperty;
</del><ins>+ return m_deletePropertyMode;
</ins><span class="cx"> }
</span><span class="cx">
</span><ins>+ class DeletePropertyModeScope {
+ public:
+ DeletePropertyModeScope(VM& vm, DeletePropertyMode mode)
+ : m_vm(vm)
+ , m_previousMode(vm.m_deletePropertyMode)
+ {
+ m_vm.m_deletePropertyMode = mode;
+ }
+
+ ~DeletePropertyModeScope()
+ {
+ m_vm.m_deletePropertyMode = m_previousMode;
+ }
+
+ private:
+ VM& m_vm;
+ DeletePropertyMode m_previousMode;
+ };
+
</ins><span class="cx"> #if ENABLE(JIT)
</span><span class="cx"> bool canUseJIT() { return m_canUseJIT; }
</span><span class="cx"> #else
</span><span class="lines">@@ -638,7 +660,7 @@
</span><span class="cx"> Exception* m_exception { nullptr };
</span><span class="cx"> Exception* m_lastException { nullptr };
</span><span class="cx"> bool m_failNextNewCodeBlock { false };
</span><del>- bool m_inDefineOwnProperty;
</del><ins>+ DeletePropertyMode m_deletePropertyMode { DeletePropertyMode::Default };
</ins><span class="cx"> bool m_globalConstRedeclarationShouldThrow { true };
</span><span class="cx"> bool m_shouldBuildPCToCodeOriginMapping { false };
</span><span class="cx"> std::unique_ptr<CodeCache> m_codeCache;
</span></span></pre>
</div>
</div>
</body>
</html>