Commit dca6b2ea authored by barraclough@apple.com's avatar barraclough@apple.com

[[Get]]/[[Put]] for primitives should not wrap on strict accessor call

https://bugs.webkit.org/show_bug.cgi?id=79588

Reviewed by Oliver Hunt.

In the case of [[Get]], this is a pretty trivial bug - just don't wrap
primitives at the point you call a getter.

For setters, this is a little more involved, since we have already wrapped
the value up in a synthesized object. Stop doing so. There is also a further
subtely, that in strict mode all attempts to create a new data property on
the object should throw.

Source/JavaScriptCore: 

* runtime/JSCell.cpp:
(JSC::JSCell::put):
    - [[Put]] to a string primitive should use JSValue::putToPrimitive.
* runtime/JSObject.cpp:
(JSC::JSObject::put):
    - Remove static function called in one place.
* runtime/JSObject.h:
(JSC::JSValue::put):
    - [[Put]] to a non-cell JSValue should use JSValue::putToPrimitive.
* runtime/JSValue.cpp:
(JSC::JSValue::synthesizePrototype):
    - Add support for synthesizing the prototype of strings.
(JSC::JSValue::putToPrimitive):
    - Added, implements [[Put]] for primitive bases, per 8.7.2.
* runtime/JSValue.h:
(JSValue):
    - Add declaration for JSValue::putToPrimitive.
* runtime/PropertySlot.cpp:
(JSC::PropertySlot::functionGetter):
    - Don't call ToObject on primitive this values.

LayoutTests: 

* fast/js/mozilla/strict/15.5.5.1-expected.txt:
* fast/js/primitive-property-access-edge-cases-expected.txt:
* fast/js/read-modify-eval-expected.txt:
* fast/js/script-tests/primitive-property-access-edge-cases.js:
* fast/js/script-tests/read-modify-eval.js:
    - Added new test cases & updated test results.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@109177 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 5c1c9729
2012-02-28 Gavin Barraclough <barraclough@apple.com>
[[Get]]/[[Put]] for primitives should not wrap on strict accessor call
https://bugs.webkit.org/show_bug.cgi?id=79588
Reviewed by Oliver Hunt.
In the case of [[Get]], this is a pretty trivial bug - just don't wrap
primitives at the point you call a getter.
For setters, this is a little more involved, since we have already wrapped
the value up in a synthesized object. Stop doing so. There is also a further
subtely, that in strict mode all attempts to create a new data property on
the object should throw.
* fast/js/mozilla/strict/15.5.5.1-expected.txt:
* fast/js/primitive-property-access-edge-cases-expected.txt:
* fast/js/read-modify-eval-expected.txt:
* fast/js/script-tests/primitive-property-access-edge-cases.js:
* fast/js/script-tests/read-modify-eval.js:
- Added new test cases & updated test results.
2012-02-28 Daniel Cheng <dcheng@chromium.org> 2012-02-28 Daniel Cheng <dcheng@chromium.org>
Clipboard::getData should return an empty string instead of undefined Clipboard::getData should return an empty string instead of undefined
...@@ -4,7 +4,7 @@ PASS true === true ...@@ -4,7 +4,7 @@ PASS true === true
PASS 'use strict'; var s = str(); delete s.length threw exception of type TypeError. PASS 'use strict'; var s = str(); delete s.length threw exception of type TypeError.
PASS var s = str(); delete s.length is false PASS var s = str(); delete s.length is false
PASS true === true PASS true === true
FAIL 'use strict'; "foo".length = 1 should throw an instance of TypeError PASS 'use strict'; "foo".length = 1 threw exception of type TypeError.
PASS "foo".length = 1 is 1 PASS "foo".length = 1 is 1
PASS true === true PASS true === true
PASS 'use strict'; delete "foo".length threw exception of type TypeError. PASS 'use strict'; delete "foo".length threw exception of type TypeError.
......
...@@ -3,6 +3,30 @@ This page tests for assertion failures in edge cases of property lookup on primi ...@@ -3,6 +3,30 @@ This page tests for assertion failures in edge cases of property lookup on primi
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS checkGet(1, Number) is true
PASS checkGet('hello', String) is true
PASS checkGet(true, Boolean) is true
PASS checkSet(1, Number) is true
PASS checkSet('hello', String) is true
PASS checkSet(true, Boolean) is true
PASS checkGetStrict(1, Number) is true
PASS checkGetStrict('hello', String) is true
PASS checkGetStrict(true, Boolean) is true
PASS checkSetStrict(1, Number) is true
PASS checkSetStrict('hello', String) is true
PASS checkSetStrict(true, Boolean) is true
PASS checkRead(1, Number) is true
PASS checkRead('hello', String) is true
PASS checkRead(true, Boolean) is true
PASS checkWrite(1, Number) is true
PASS checkWrite('hello', String) is true
PASS checkWrite(true, Boolean) is true
PASS checkReadStrict(1, Number) is true
PASS checkReadStrict('hello', String) is true
PASS checkReadStrict(true, Boolean) is true
PASS checkWriteStrict(1, Number) threw exception TypeError: Attempted to assign to readonly property..
PASS checkWriteStrict('hello', String) threw exception TypeError: Attempted to assign to readonly property..
PASS checkWriteStrict(true, Boolean) threw exception TypeError: Attempted to assign to readonly property..
PASS didNotCrash is true PASS didNotCrash is true
PASS successfullyParsed is true PASS successfullyParsed is true
......
...@@ -19,7 +19,7 @@ PASS preDecTest(); is true ...@@ -19,7 +19,7 @@ PASS preDecTest(); is true
PASS postIncTest(); is true PASS postIncTest(); is true
PASS postDecTest(); is true PASS postDecTest(); is true
PASS primitiveThisTest.call(1); is true PASS primitiveThisTest.call(1); is true
PASS strictThisTest.call(1); is true PASS strictThisTest.call(1); threw exception TypeError: Attempted to assign to readonly property..
PASS successfullyParsed is true PASS successfullyParsed is true
TEST COMPLETE TEST COMPLETE
......
...@@ -37,4 +37,93 @@ var didNotCrash = true; ...@@ -37,4 +37,93 @@ var didNotCrash = true;
f(.5); f(.5);
})(); })();
var checkOkay;
function checkGet(x, constructor)
{
checkOkay = false;
Object.defineProperty(constructor.prototype, "foo", { get: function() { checkOkay = typeof this === 'object'; }, configurable: true });
x.foo;
delete constructor.prototype.foo;
return checkOkay;
}
function checkSet(x, constructor)
{
checkOkay = false;
Object.defineProperty(constructor.prototype, "foo", { set: function() { checkOkay = typeof this === 'object'; }, configurable: true });
x.foo = null;
delete constructor.prototype.foo;
return checkOkay;
}
function checkGetStrict(x, constructor)
{
checkOkay = false;
Object.defineProperty(constructor.prototype, "foo", { get: function() { "use strict"; checkOkay = typeof this !== 'object'; }, configurable: true });
x.foo;
delete constructor.prototype.foo;
return checkOkay;
}
function checkSetStrict(x, constructor)
{
checkOkay = false;
Object.defineProperty(constructor.prototype, "foo", { set: function() { "use strict"; checkOkay = typeof this !== 'object'; }, configurable: true });
x.foo = null;
delete constructor.prototype.foo;
return checkOkay;
}
shouldBeTrue("checkGet(1, Number)");
shouldBeTrue("checkGet('hello', String)");
shouldBeTrue("checkGet(true, Boolean)");
shouldBeTrue("checkSet(1, Number)");
shouldBeTrue("checkSet('hello', String)");
shouldBeTrue("checkSet(true, Boolean)");
shouldBeTrue("checkGetStrict(1, Number)");
shouldBeTrue("checkGetStrict('hello', String)");
shouldBeTrue("checkGetStrict(true, Boolean)");
shouldBeTrue("checkSetStrict(1, Number)");
shouldBeTrue("checkSetStrict('hello', String)");
shouldBeTrue("checkSetStrict(true, Boolean)");
function checkRead(x, constructor)
{
return x.foo === undefined;
}
function checkWrite(x, constructor)
{
x.foo = null;
return x.foo === undefined;
}
function checkReadStrict(x, constructor)
{
"use strict";
return x.foo === undefined;
}
function checkWriteStrict(x, constructor)
{
"use strict";
x.foo = null;
return x.foo === undefined;
}
shouldBeTrue("checkRead(1, Number)");
shouldBeTrue("checkRead('hello', String)");
shouldBeTrue("checkRead(true, Boolean)");
shouldBeTrue("checkWrite(1, Number)");
shouldBeTrue("checkWrite('hello', String)");
shouldBeTrue("checkWrite(true, Boolean)");
shouldBeTrue("checkReadStrict(1, Number)");
shouldBeTrue("checkReadStrict('hello', String)");
shouldBeTrue("checkReadStrict(true, Boolean)");
shouldThrow("checkWriteStrict(1, Number)");
shouldThrow("checkWriteStrict('hello', String)");
shouldThrow("checkWriteStrict(true, Boolean)");
shouldBeTrue("didNotCrash"); shouldBeTrue("didNotCrash");
...@@ -119,7 +119,7 @@ function strictThisTest() ...@@ -119,7 +119,7 @@ function strictThisTest()
{ {
// In a strict mode function primitive this values are not converted, so // In a strict mode function primitive this values are not converted, so
// the property access in the first eval is writing a value to a temporary // the property access in the first eval is writing a value to a temporary
// object, and should not be observed by the second eval. // object. This throws, per section 8.7.2.
"use strict"; "use strict";
eval('this.value = "Seekrit message";'); eval('this.value = "Seekrit message";');
return eval('this.value') === undefined; return eval('this.value') === undefined;
...@@ -143,4 +143,4 @@ shouldBeTrue('postIncTest();'); ...@@ -143,4 +143,4 @@ shouldBeTrue('postIncTest();');
shouldBeTrue('postDecTest();'); shouldBeTrue('postDecTest();');
shouldBeTrue('primitiveThisTest.call(1);'); shouldBeTrue('primitiveThisTest.call(1);');
shouldBeTrue('strictThisTest.call(1);'); shouldThrow('strictThisTest.call(1);');
2012-02-28 Gavin Barraclough <barraclough@apple.com>
[[Get]]/[[Put]] for primitives should not wrap on strict accessor call
https://bugs.webkit.org/show_bug.cgi?id=79588
Reviewed by Oliver Hunt.
In the case of [[Get]], this is a pretty trivial bug - just don't wrap
primitives at the point you call a getter.
For setters, this is a little more involved, since we have already wrapped
the value up in a synthesized object. Stop doing so. There is also a further
subtely, that in strict mode all attempts to create a new data property on
the object should throw.
* runtime/JSCell.cpp:
(JSC::JSCell::put):
- [[Put]] to a string primitive should use JSValue::putToPrimitive.
* runtime/JSObject.cpp:
(JSC::JSObject::put):
- Remove static function called in one place.
* runtime/JSObject.h:
(JSC::JSValue::put):
- [[Put]] to a non-cell JSValue should use JSValue::putToPrimitive.
* runtime/JSValue.cpp:
(JSC::JSValue::synthesizePrototype):
- Add support for synthesizing the prototype of strings.
(JSC::JSValue::putToPrimitive):
- Added, implements [[Put]] for primitive bases, per 8.7.2.
* runtime/JSValue.h:
(JSValue):
- Add declaration for JSValue::putToPrimitive.
* runtime/PropertySlot.cpp:
(JSC::PropertySlot::functionGetter):
- Don't call ToObject on primitive this values.
2012-02-28 Mark Hahnenberg <mhahnenberg@apple.com> 2012-02-28 Mark Hahnenberg <mhahnenberg@apple.com>
Re-enable parallel GC on Mac Re-enable parallel GC on Mac
...@@ -97,6 +97,10 @@ bool JSCell::getOwnPropertySlotByIndex(JSCell* cell, ExecState* exec, unsigned i ...@@ -97,6 +97,10 @@ bool JSCell::getOwnPropertySlotByIndex(JSCell* cell, ExecState* exec, unsigned i
void JSCell::put(JSCell* cell, ExecState* exec, const Identifier& identifier, JSValue value, PutPropertySlot& slot) void JSCell::put(JSCell* cell, ExecState* exec, const Identifier& identifier, JSValue value, PutPropertySlot& slot)
{ {
if (cell->isString()) {
JSValue(cell).putToPrimitive(exec, identifier, value, slot);
return;
}
JSObject* thisObject = cell->toObject(exec, exec->lexicalGlobalObject()); JSObject* thisObject = cell->toObject(exec, exec->lexicalGlobalObject());
thisObject->methodTable()->put(thisObject, exec, identifier, value, slot); thisObject->methodTable()->put(thisObject, exec, identifier, value, slot);
} }
......
...@@ -119,11 +119,6 @@ bool JSObject::getOwnPropertySlotByIndex(JSCell* cell, ExecState* exec, unsigned ...@@ -119,11 +119,6 @@ bool JSObject::getOwnPropertySlotByIndex(JSCell* cell, ExecState* exec, unsigned
return thisObject->methodTable()->getOwnPropertySlot(thisObject, exec, Identifier::from(exec, propertyName), slot); return thisObject->methodTable()->getOwnPropertySlot(thisObject, exec, Identifier::from(exec, propertyName), slot);
} }
static void throwSetterError(ExecState* exec)
{
throwError(exec, createTypeError(exec, "setting a property that has only a getter"));
}
// ECMA 8.6.2.2 // ECMA 8.6.2.2
void JSObject::put(JSCell* cell, ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot& slot) void JSObject::put(JSCell* cell, ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot& slot)
{ {
...@@ -161,7 +156,7 @@ void JSObject::put(JSCell* cell, ExecState* exec, const Identifier& propertyName ...@@ -161,7 +156,7 @@ void JSObject::put(JSCell* cell, ExecState* exec, const Identifier& propertyName
JSObject* setterFunc = asGetterSetter(gs)->setter(); JSObject* setterFunc = asGetterSetter(gs)->setter();
if (!setterFunc) { if (!setterFunc) {
if (slot.isStrictMode()) if (slot.isStrictMode())
throwSetterError(exec); throwError(exec, createTypeError(exec, "setting a property that has only a getter"));
return; return;
} }
......
...@@ -834,8 +834,7 @@ inline JSValue JSValue::get(ExecState* exec, unsigned propertyName, PropertySlot ...@@ -834,8 +834,7 @@ inline JSValue JSValue::get(ExecState* exec, unsigned propertyName, PropertySlot
inline void JSValue::put(ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot& slot) inline void JSValue::put(ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot& slot)
{ {
if (UNLIKELY(!isCell())) { if (UNLIKELY(!isCell())) {
JSObject* thisObject = synthesizeObject(exec); putToPrimitive(exec, propertyName, value, slot);
thisObject->methodTable()->put(thisObject, exec, propertyName, value, slot);
return; return;
} }
asCell()->methodTable()->put(asCell(), exec, propertyName, value, slot); asCell()->methodTable()->put(asCell(), exec, propertyName, value, slot);
......
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include "BooleanPrototype.h" #include "BooleanPrototype.h"
#include "Error.h" #include "Error.h"
#include "ExceptionHelpers.h" #include "ExceptionHelpers.h"
#include "GetterSetter.h"
#include "JSGlobalObject.h" #include "JSGlobalObject.h"
#include "JSFunction.h" #include "JSFunction.h"
#include "JSNotAnObject.h" #include "JSNotAnObject.h"
...@@ -105,7 +106,11 @@ JSObject* JSValue::synthesizeObject(ExecState* exec) const ...@@ -105,7 +106,11 @@ JSObject* JSValue::synthesizeObject(ExecState* exec) const
JSObject* JSValue::synthesizePrototype(ExecState* exec) const JSObject* JSValue::synthesizePrototype(ExecState* exec) const
{ {
ASSERT(!isCell()); if (isCell()) {
ASSERT(isString());
return exec->lexicalGlobalObject()->stringPrototype();
}
if (isNumber()) if (isNumber())
return exec->lexicalGlobalObject()->numberPrototype(); return exec->lexicalGlobalObject()->numberPrototype();
if (isBoolean()) if (isBoolean())
...@@ -116,6 +121,70 @@ JSObject* JSValue::synthesizePrototype(ExecState* exec) const ...@@ -116,6 +121,70 @@ JSObject* JSValue::synthesizePrototype(ExecState* exec) const
return JSNotAnObject::create(exec); return JSNotAnObject::create(exec);
} }
// ECMA 8.7.2
void JSValue::putToPrimitive(ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot& slot)
{
JSGlobalData& globalData = exec->globalData();
// Check if there are any setters or getters in the prototype chain
JSObject* obj = synthesizePrototype(exec);
JSValue prototype;
if (propertyName != exec->propertyNames().underscoreProto) {
for (; !obj->structure()->hasReadOnlyOrGetterSetterPropertiesExcludingProto(); obj = asObject(prototype)) {
prototype = obj->prototype();
if (prototype.isNull()) {
if (slot.isStrictMode())
throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
return;
}
}
}
for (; ; obj = asObject(prototype)) {
unsigned attributes;
JSCell* specificValue;
size_t offset = obj->structure()->get(globalData, propertyName, attributes, specificValue);
if (offset != WTF::notFound) {
if (attributes & ReadOnly) {
if (slot.isStrictMode())
throwError(exec, createTypeError(exec, StrictModeReadonlyPropertyWriteError));
return;
}
JSValue gs = obj->getDirectOffset(offset);
if (gs.isGetterSetter()) {
JSObject* setterFunc = asGetterSetter(gs)->setter();
if (!setterFunc) {
if (slot.isStrictMode())
throwError(exec, createTypeError(exec, "setting a property that has only a getter"));
return;
}
CallData callData;
CallType callType = setterFunc->methodTable()->getCallData(setterFunc, callData);
MarkedArgumentBuffer args;
args.append(value);
// If this is WebCore's global object then we need to substitute the shell.
call(exec, setterFunc, callType, callData, *this, args);
return;
}
// If there's an existing property on the object or one of its
// prototypes it should be replaced, so break here.
break;
}
prototype = obj->prototype();
if (prototype.isNull())
break;
}
if (slot.isStrictMode())
throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
return;
}
char* JSValue::description() char* JSValue::description()
{ {
static const size_t size = 128; static const size_t size = 128;
......
...@@ -221,6 +221,7 @@ namespace JSC { ...@@ -221,6 +221,7 @@ namespace JSC {
JSValue get(ExecState*, unsigned propertyName) const; JSValue get(ExecState*, unsigned propertyName) const;
JSValue get(ExecState*, unsigned propertyName, PropertySlot&) const; JSValue get(ExecState*, unsigned propertyName, PropertySlot&) const;
void put(ExecState*, const Identifier& propertyName, JSValue, PutPropertySlot&); void put(ExecState*, const Identifier& propertyName, JSValue, PutPropertySlot&);
void putToPrimitive(ExecState*, const Identifier& propertyName, JSValue, PutPropertySlot&);
void put(ExecState*, unsigned propertyName, JSValue); void put(ExecState*, unsigned propertyName, JSValue);
JSObject* toThisObject(ExecState*) const; JSObject* toThisObject(ExecState*) const;
......
...@@ -34,11 +34,7 @@ JSValue PropertySlot::functionGetter(ExecState* exec) const ...@@ -34,11 +34,7 @@ JSValue PropertySlot::functionGetter(ExecState* exec) const
CallData callData; CallData callData;
CallType callType = m_data.getterFunc->methodTable()->getCallData(m_data.getterFunc, callData); CallType callType = m_data.getterFunc->methodTable()->getCallData(m_data.getterFunc, callData);
return call(exec, m_data.getterFunc, callType, callData, m_thisValue.isObject() ? m_thisValue.toThisObject(exec) : m_thisValue, exec->emptyList());
// Only objects can have accessor properties.
// If the base is WebCore's global object then we need to substitute the shell.
ASSERT(m_slotBase.isObject());
return call(exec, m_data.getterFunc, callType, callData, m_thisValue.toThisObject(exec), exec->emptyList());
} }
} // namespace JSC } // namespace JSC
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment