- Revision
- 201654
- Author
- barraclo...@apple.com
- Date
- 2016-06-03 13:12:25 -0700 (Fri, 03 Jun 2016)
Log Message
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.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (201653 => 201654)
--- trunk/Source/_javascript_Core/ChangeLog 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-06-03 20:12:25 UTC (rev 201654)
@@ -1,3 +1,58 @@
+2016-06-02 Gavin & Ellie Barraclough <barraclo...@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.
+
2016-06-03 Mark Lam <mark....@apple.com>
ARMv7 vstm and vldm instructions can only operate on a maximum of 16 registers.
Modified: trunk/Source/_javascript_Core/runtime/JSFunction.cpp (201653 => 201654)
--- trunk/Source/_javascript_Core/runtime/JSFunction.cpp 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.cpp 2016-06-03 20:12:25 UTC (rev 201654)
@@ -458,7 +458,7 @@
{
JSFunction* thisObject = jsCast<JSFunction*>(cell);
// For non-host functions, don't let these properties by deleted - except by DefineOwnProperty.
- if (!thisObject->isHostOrBuiltinFunction() && !exec->vm().isInDefineOwnProperty()) {
+ if (!thisObject->isHostOrBuiltinFunction() && exec->vm().deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable) {
FunctionExecutable* executable = thisObject->jsExecutable();
if (propertyName == exec->propertyNames().arguments
|| (propertyName == exec->propertyNames().prototype && !executable->isArrowFunction())
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp (201653 => 201654)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp 2016-06-03 20:12:25 UTC (rev 201654)
@@ -857,7 +857,8 @@
void JSGlobalObject::addFunction(ExecState* exec, const Identifier& propertyName)
{
VM& vm = exec->vm();
- removeDirect(vm, propertyName); // Newly declared functions overwrite existing properties.
+ VM::DeletePropertyModeScope scope(vm, VM::DeletePropertyMode::IgnoreConfigurable);
+ methodTable(vm)->deleteProperty(this, exec, propertyName);
addGlobalVar(propertyName);
}
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (201653 => 201654)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2016-06-03 20:12:25 UTC (rev 201654)
@@ -1497,7 +1497,7 @@
if (!thisObject->staticFunctionsReified()) {
if (auto* entry = thisObject->findPropertyHashEntry(propertyName)) {
- if (entry->attributes() & DontDelete)
+ if (entry->attributes() & DontDelete && vm.deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable)
return false;
thisObject->reifyAllStaticProperties(exec);
}
@@ -1505,7 +1505,7 @@
unsigned attributes;
if (isValidOffset(thisObject->structure(vm)->get(vm, propertyName, attributes))) {
- if (attributes & DontDelete && !vm.isInDefineOwnProperty())
+ if (attributes & DontDelete && vm.deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable)
return false;
thisObject->removeDirect(vm, propertyName);
}
@@ -2933,24 +2933,6 @@
return putDirect(exec->vm(), propertyName, value);
}
-class DefineOwnPropertyScope {
-public:
- DefineOwnPropertyScope(ExecState* exec)
- : m_vm(exec->vm())
- {
- m_vm.setInDefineOwnProperty(true);
- }
-
- ~DefineOwnPropertyScope()
- {
- m_vm.setInDefineOwnProperty(false);
- }
-
-private:
- VM& m_vm;
-};
-
-
// 9.1.6.3 of the spec
// http://www.ecma-international.org/ecma-262/6.0/index.html#sec-validateandapplypropertydescriptor
bool validateAndApplyPropertyDescriptor(ExecState* exec, JSObject* object, PropertyName propertyName, bool isExtensible,
@@ -3104,7 +3086,7 @@
// Currently DefineOwnProperty uses delete to remove properties when they are being replaced
// (particularly when changing attributes), however delete won't allow non-configurable (i.e.
// DontDelete) properties to be deleted. For now, we can use this flag to make this work.
- DefineOwnPropertyScope scope(exec);
+ VM::DeletePropertyModeScope scope(exec->vm(), VM::DeletePropertyMode::IgnoreConfigurable);
PropertyDescriptor current;
bool isCurrentDefined = getOwnPropertyDescriptor(exec, propertyName, current);
bool isExtensible = this->isExtensible(exec);
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (201653 => 201654)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2016-06-03 20:12:25 UTC (rev 201654)
@@ -188,7 +188,6 @@
#if !ENABLE(JIT)
, m_jsStackLimit(0)
#endif
- , m_inDefineOwnProperty(false)
, m_codeCache(std::make_unique<CodeCache>())
, m_builtinExecutables(std::make_unique<BuiltinExecutables>(*this))
, m_typeProfilerEnabledCount(0)
Modified: trunk/Source/_javascript_Core/runtime/VM.h (201653 => 201654)
--- trunk/Source/_javascript_Core/runtime/VM.h 2016-06-03 19:54:39 UTC (rev 201653)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2016-06-03 20:12:25 UTC (rev 201654)
@@ -344,16 +344,38 @@
AtomicStringTable* atomicStringTable() const { return m_atomicStringTable; }
WTF::SymbolRegistry& symbolRegistry() { return m_symbolRegistry; }
- void setInDefineOwnProperty(bool inDefineOwnProperty)
- {
- m_inDefineOwnProperty = inDefineOwnProperty;
- }
+ 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
+ };
- bool isInDefineOwnProperty()
+ DeletePropertyMode deletePropertyMode()
{
- return m_inDefineOwnProperty;
+ return m_deletePropertyMode;
}
+ 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;
+ };
+
#if ENABLE(JIT)
bool canUseJIT() { return m_canUseJIT; }
#else
@@ -638,7 +660,7 @@
Exception* m_exception { nullptr };
Exception* m_lastException { nullptr };
bool m_failNextNewCodeBlock { false };
- bool m_inDefineOwnProperty;
+ DeletePropertyMode m_deletePropertyMode { DeletePropertyMode::Default };
bool m_globalConstRedeclarationShouldThrow { true };
bool m_shouldBuildPCToCodeOriginMapping { false };
std::unique_ptr<CodeCache> m_codeCache;
