Changeset 104886 in webkit

Timestamp:

Jan 12, 2012 5:40:22 PM (12 years ago)

Author:

barraclough@apple.com

Message:

Clean up putDirect (part 1)
​https://bugs.webkit.org/show_bug.cgi?id=76232

Reviewed by Sam Weinig.

putDirect has ambiguous semantics, clean these up a bit.

putDirect generally behaves a bit like a fast defineOwnProperty, but one that
always creates the property, with no checking to validate the put it permitted.

It also encompasses two slightly different behaviors.
(1) a fast form of put for JSActivation, which doesn't have to handle searching

the prototype chain, getter/setter properties, or the magic proto value.
Break this out as a new method, 'putOwnDataProperty'.

(2) the version of putDirect on JSValue will also check for overwriting ReadOnly

values, in strict mode. This is, however, not so smart on a few level, since
it is only called from op_put_by_id with direct set, which is only used with
an object as the base, and is only used to put new properties onto objects.

• dfg/DFGOperations.cpp:
• interpreter/Interpreter.cpp:

(JSC::Interpreter::privateExecute):

• jit/JITStubs.cpp:

(JSC::DEFINE_STUB_FUNCTION):

• runtime/JSActivation.cpp:

(JSC::JSActivation::put):

• runtime/JSFunction.cpp:

(JSC::JSFunction::getOwnPropertySlot):

• runtime/JSObject.h:

(JSC::JSObject::putOwnDataProperty):

• runtime/JSValue.h:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGOperations.cpp (modified) (4 diffs)
• interpreter/Interpreter.cpp (modified) (2 diffs)
• jit/JITStubs.cpp (modified) (3 diffs)
• runtime/JSActivation.cpp (modified) (1 diff)
• runtime/JSFunction.cpp (modified) (1 diff)
• runtime/JSObject.h (modified) (4 diffs)
• runtime/JSValue.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2012-01-12  Gavin Barraclough  <barraclough@apple.com>
+
+        Clean up putDirect (part 1)
+        https://bugs.webkit.org/show_bug.cgi?id=76232
+
+        Reviewed by Sam Weinig.
+
+        putDirect has ambiguous semantics, clean these up a bit.
+
+        putDirect generally behaves a bit like a fast defineOwnProperty, but one that
+        always creates the property, with no checking to validate the put it permitted.
+
+        It also encompasses two slightly different behaviors.
+        (1) a fast form of put for JSActivation, which doesn't have to handle searching
+            the prototype chain, getter/setter properties, or the magic __proto__ value.
+            Break this out as a new method, 'putOwnDataProperty'.
+        (2) the version of putDirect on JSValue will also check for overwriting ReadOnly
+            values, in strict mode. This is, however, not so smart on a few level, since
+            it is only called from op_put_by_id with direct set, which is only used with
+            an object as the base, and is only used to put new properties onto objects.
+
+        * dfg/DFGOperations.cpp:
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::privateExecute):
+        * jit/JITStubs.cpp:
+        (JSC::DEFINE_STUB_FUNCTION):
+        * runtime/JSActivation.cpp:
+        (JSC::JSActivation::put):
+        * runtime/JSFunction.cpp:
+        (JSC::JSFunction::getOwnPropertySlot):
+        * runtime/JSObject.h:
+        (JSC::JSObject::putOwnDataProperty):
+        * runtime/JSValue.h:
+
 2012-01-12  Gavin Barraclough  <barraclough@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp

@@ -432 +432 @@
 {
     PutPropertySlot slot(true);
-    JSValue(base).putDirect(exec, *propertyName, JSValue::decode(encodedValue), slot);
+    ASSERT(base->isObject());
+    asObject(base)->putDirect(exec->globalData(), *propertyName, JSValue::decode(encodedValue), slot);
 }
 
@@ -438 +439 @@
 {
     PutPropertySlot slot(false);
-    JSValue(base).putDirect(exec, *propertyName, JSValue::decode(encodedValue), slot);
+    ASSERT(base->isObject());
+    asObject(base)->putDirect(exec->globalData(), *propertyName, JSValue::decode(encodedValue), slot);
 }
 
@@ -477 +479 @@
 {
     JSValue value = JSValue::decode(encodedValue);
-    JSValue baseValue(base);
     PutPropertySlot slot(true);
     
-    baseValue.putDirect(exec, *propertyName, value, slot);
+    ASSERT(base->isObject());
+    asObject(base)->putDirect(exec->globalData(), *propertyName, value, slot);
     
     StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
     if (stubInfo.seen)
-        dfgRepatchPutByID(exec, baseValue, *propertyName, slot, stubInfo, Direct);
+        dfgRepatchPutByID(exec, base, *propertyName, slot, stubInfo, Direct);
     else
         stubInfo.seen = true;
@@ -493 +495 @@
 {
     JSValue value = JSValue::decode(encodedValue);
-    JSValue baseValue(base);
     PutPropertySlot slot(false);
     
-    baseValue.putDirect(exec, *propertyName, value, slot);
+    ASSERT(base->isObject());
+    asObject(base)->putDirect(exec->globalData(), *propertyName, value, slot);
     
     StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
     if (stubInfo.seen)
-        dfgRepatchPutByID(exec, baseValue, *propertyName, slot, stubInfo, Direct);
+        dfgRepatchPutByID(exec, base, *propertyName, slot, stubInfo, Direct);
     else
         stubInfo.seen = true;

trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

@@ -3309 +3309 @@
 
         JSValue baseValue = callFrame->r(base).jsValue();
+        ASSERT(baseValue.isObject());
+        JSObject* baseObject = asObject(baseValue);
         Identifier& ident = codeBlock->identifier(property);
         PutPropertySlot slot(codeBlock->isStrictMode());
         if (direct)
-            baseValue.putDirect(callFrame, ident, callFrame->r(value).jsValue(), slot);
+            baseObject->putDirect(*globalData, ident, callFrame->r(value).jsValue(), slot);
         else
             baseValue.put(callFrame, ident, callFrame->r(value).jsValue(), slot);
@@ -3427 +3429 @@
 
         JSValue baseValue = callFrame->r(base).jsValue();
+        ASSERT(baseValue.isObject());
+        JSObject* baseObject = asObject(baseValue);
         Identifier& ident = codeBlock->identifier(property);
         PutPropertySlot slot(codeBlock->isStrictMode());
         if (direct)
-            baseValue.putDirect(callFrame, ident, callFrame->r(value).jsValue(), slot);
+            baseObject->putDirect(*globalData, ident, callFrame->r(value).jsValue(), slot);
         else
             baseValue.put(callFrame, ident, callFrame->r(value).jsValue(), slot);

trunk/Source/JavaScriptCore/jit/JITStubs.cpp

@@ -1383 +1383 @@
     
     PutPropertySlot slot(stackFrame.callFrame->codeBlock()->isStrictMode());
-    stackFrame.args[0].jsValue().putDirect(stackFrame.callFrame, stackFrame.args[1].identifier(), stackFrame.args[2].jsValue(), slot);
+    JSValue baseValue = stackFrame.args[0].jsValue();
+    ASSERT(baseValue.isObject());
+    asObject(baseValue)->putDirect(stackFrame.callFrame->globalData(), stackFrame.args[1].identifier(), stackFrame.args[2].jsValue(), slot);
     CHECK_FOR_EXCEPTION_AT_END();
 }
@@ -1428 +1430 @@
     
     PutPropertySlot slot(callFrame->codeBlock()->isStrictMode());
-    stackFrame.args[0].jsValue().putDirect(callFrame, ident, stackFrame.args[2].jsValue(), slot);
+    JSValue baseValue = stackFrame.args[0].jsValue();
+    ASSERT(baseValue.isObject());
+    asObject(baseValue)->putDirect(callFrame->globalData(), ident, stackFrame.args[2].jsValue(), slot);
     
     CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
@@ -1461 +1465 @@
     
     PutPropertySlot slot(callFrame->codeBlock()->isStrictMode());
-    stackFrame.args[0].jsValue().putDirect(callFrame, ident, stackFrame.args[2].jsValue(), slot);
+    JSValue baseValue = stackFrame.args[0].jsValue();
+    ASSERT(baseValue.isObject());
+    asObject(baseValue)->putDirect(callFrame->globalData(), ident, stackFrame.args[2].jsValue(), slot);
     
     CHECK_FOR_EXCEPTION_AT_END();

trunk/Source/JavaScriptCore/runtime/JSActivation.cpp

@@ -185 +185 @@
     // expose in the activation object.
     ASSERT(!thisObject->hasGetterSetterProperties());
-    thisObject->putDirect(exec->globalData(), propertyName, value, 0, true, slot);
+    thisObject->putOwnDataProperty(exec->globalData(), propertyName, value, slot);
 }
 

trunk/Source/JavaScriptCore/runtime/JSFunction.cpp

@@ -205 +205 @@
             JSObject* prototype = constructEmptyObject(exec, thisObject->globalObject()->emptyObjectStructure());
             prototype->putDirect(exec->globalData(), exec->propertyNames().constructor, thisObject, DontEnum);
-            PutPropertySlot slot;
-            thisObject->putDirect(exec->globalData(), exec->propertyNames().prototype, prototype, DontDelete | DontEnum, false, slot);
+            thisObject->putDirect(exec->globalData(), exec->propertyNames().prototype, prototype, DontDelete | DontEnum);
             location = thisObject->getDirectLocation(exec->globalData(), exec->propertyNames().prototype);
         }

trunk/Source/JavaScriptCore/runtime/JSObject.h

@@ -108 +108 @@
         static void putByIndex(JSCell*, ExecState*, unsigned propertyName, JSValue);
 
+        // putWithAttributes is effectively an unchecked vesion of 'defineOwnProperty':
+        //  - the prototype chain is not consulted
+        //  - accessors are not called.
+        //  - attributes will be respected (after the call the property will exist with the given attributes)
         static void putWithAttributes(JSObject*, ExecState*, const Identifier& propertyName, JSValue, unsigned attributes);
         void putWithAttributes(JSGlobalData*, const Identifier& propertyName, JSValue, unsigned attributes);
@@ -171 +175 @@
         bool hasGetterSetterProperties() { return structure()->hasGetterSetterProperties(); }
 
-        bool putDirect(JSGlobalData&, const Identifier& propertyName, JSValue, unsigned attr, bool checkReadOnly, PutPropertySlot&);
+        // putOwnDataProperty has 'put' like semantics, however this method:
+        //  - assumes the object contains no own getter/setter properties.
+        //  - provides no special handling for __proto__
+        //  - does not walk the prototype chain (to check for accessors or non-writable properties).
+        // This is used by JSActivation.
+        bool putOwnDataProperty(JSGlobalData&, const Identifier& propertyName, JSValue, PutPropertySlot&);
+
         void putDirect(JSGlobalData&, const Identifier& propertyName, JSValue, unsigned attr = 0);
         bool putDirect(JSGlobalData&, const Identifier& propertyName, JSValue, PutPropertySlot&);
@@ -739 +749 @@
 }
 
-inline bool JSObject::putDirect(JSGlobalData& globalData, const Identifier& propertyName, JSValue value, unsigned attributes, bool checkReadOnly, PutPropertySlot& slot)
+inline bool JSObject::putOwnDataProperty(JSGlobalData& globalData, const Identifier& propertyName, JSValue value, PutPropertySlot& slot)
 {
     ASSERT(value);
     ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(this));
-
-    return putDirectInternal(globalData, propertyName, value, attributes, checkReadOnly, slot, getJSFunction(value));
+    ASSERT(!structure()->hasGetterSetterProperties());
+
+    return putDirectInternal(globalData, propertyName, value, 0, true, slot, getJSFunction(value));
 }
 
@@ -841 +852 @@
 }
 
-inline void JSValue::putDirect(ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot& slot)
-{
-    ASSERT(isCell() && isObject());
-    if (!asObject(asCell())->putDirect(exec->globalData(), propertyName, value, slot) && slot.isStrictMode())
-        throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
-}
-
 inline void JSValue::put(ExecState* exec, unsigned propertyName, JSValue value)
 {

trunk/Source/JavaScriptCore/runtime/JSValue.h

@@ -219 +219 @@
         JSValue get(ExecState*, unsigned propertyName, PropertySlot&) const;
         void put(ExecState*, const Identifier& propertyName, JSValue, PutPropertySlot&);
-        void putDirect(ExecState*, const Identifier& propertyName, JSValue, PutPropertySlot&);
         void put(ExecState*, unsigned propertyName, JSValue);