Changeset 109177 in webkit


Ignore:
Timestamp:
Feb 28, 2012 5:39:15 PM (12 years ago)
Author:
barraclough@apple.com
Message:

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.
Location:
trunk
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r109176 r109177  
     12012-02-28  Gavin Barraclough  <barraclough@apple.com>
     2
     3        [[Get]]/[[Put]] for primitives should not wrap on strict accessor call
     4        https://bugs.webkit.org/show_bug.cgi?id=79588
     5
     6        Reviewed by Oliver Hunt.
     7
     8        In the case of [[Get]], this is a pretty trivial bug - just don't wrap
     9        primitives at the point you call a getter.
     10
     11        For setters, this is a little more involved, since we have already wrapped
     12        the value up in a synthesized object. Stop doing so. There is also a further
     13        subtely, that in strict mode all attempts to create a new data property on
     14        the object should throw.
     15
     16        * fast/js/mozilla/strict/15.5.5.1-expected.txt:
     17        * fast/js/primitive-property-access-edge-cases-expected.txt:
     18        * fast/js/read-modify-eval-expected.txt:
     19        * fast/js/script-tests/primitive-property-access-edge-cases.js:
     20        * fast/js/script-tests/read-modify-eval.js:
     21            - Added new test cases & updated test results.
     22
    1232012-02-28  Daniel Cheng  <dcheng@chromium.org>
    224

  • trunk/LayoutTests/fast/js/mozilla/strict/15.5.5.1-expected.txt

    r78731 r109177  
    55PASS var s = str(); delete s.length is false
    66PASS true === true
    7 FAIL 'use strict'; "foo".length = 1 should throw an instance of TypeError
     7PASS 'use strict'; "foo".length = 1 threw exception of type TypeError.
    88PASS "foo".length = 1 is 1
    99PASS true === true

  • trunk/LayoutTests/fast/js/primitive-property-access-edge-cases-expected.txt

    r39374 r109177  
    44
    55
     6PASS checkGet(1, Number) is true
     7PASS checkGet('hello', String) is true
     8PASS checkGet(true, Boolean) is true
     9PASS checkSet(1, Number) is true
     10PASS checkSet('hello', String) is true
     11PASS checkSet(true, Boolean) is true
     12PASS checkGetStrict(1, Number) is true
     13PASS checkGetStrict('hello', String) is true
     14PASS checkGetStrict(true, Boolean) is true
     15PASS checkSetStrict(1, Number) is true
     16PASS checkSetStrict('hello', String) is true
     17PASS checkSetStrict(true, Boolean) is true
     18PASS checkRead(1, Number) is true
     19PASS checkRead('hello', String) is true
     20PASS checkRead(true, Boolean) is true
     21PASS checkWrite(1, Number) is true
     22PASS checkWrite('hello', String) is true
     23PASS checkWrite(true, Boolean) is true
     24PASS checkReadStrict(1, Number) is true
     25PASS checkReadStrict('hello', String) is true
     26PASS checkReadStrict(true, Boolean) is true
     27PASS checkWriteStrict(1, Number) threw exception TypeError: Attempted to assign to readonly property..
     28PASS checkWriteStrict('hello', String) threw exception TypeError: Attempted to assign to readonly property..
     29PASS checkWriteStrict(true, Boolean) threw exception TypeError: Attempted to assign to readonly property..
    630PASS didNotCrash is true
    731PASS successfullyParsed is true

  • trunk/LayoutTests/fast/js/read-modify-eval-expected.txt

    r91164 r109177  
    2020PASS postDecTest(); is true
    2121PASS primitiveThisTest.call(1); is true
    22 PASS strictThisTest.call(1); is true
     22PASS strictThisTest.call(1); threw exception TypeError: Attempted to assign to readonly property..
    2323PASS successfullyParsed is true
    2424

  • trunk/LayoutTests/fast/js/script-tests/primitive-property-access-edge-cases.js

    r98407 r109177  
    3838})();
    3939
     40
     41var checkOkay;
     42
     43function checkGet(x, constructor)
     44{
     45    checkOkay = false;
     46    Object.defineProperty(constructor.prototype, "foo", { get: function() { checkOkay = typeof this === 'object'; }, configurable: true });
     47    x.foo;
     48    delete constructor.prototype.foo;
     49    return checkOkay;
     50}
     51
     52function checkSet(x, constructor)
     53{
     54    checkOkay = false;
     55    Object.defineProperty(constructor.prototype, "foo", { set: function() { checkOkay = typeof this === 'object'; }, configurable: true });
     56    x.foo = null;
     57    delete constructor.prototype.foo;
     58    return checkOkay;
     59}
     60
     61function checkGetStrict(x, constructor)
     62{
     63    checkOkay = false;
     64    Object.defineProperty(constructor.prototype, "foo", { get: function() { "use strict"; checkOkay = typeof this !== 'object'; }, configurable: true });
     65    x.foo;
     66    delete constructor.prototype.foo;
     67    return checkOkay;
     68}
     69
     70function checkSetStrict(x, constructor)
     71{
     72    checkOkay = false;
     73    Object.defineProperty(constructor.prototype, "foo", { set: function() { "use strict"; checkOkay = typeof this !== 'object'; }, configurable: true });
     74    x.foo = null;
     75    delete constructor.prototype.foo;
     76    return checkOkay;
     77}
     78
     79shouldBeTrue("checkGet(1, Number)");
     80shouldBeTrue("checkGet('hello', String)");
     81shouldBeTrue("checkGet(true, Boolean)");
     82shouldBeTrue("checkSet(1, Number)");
     83shouldBeTrue("checkSet('hello', String)");
     84shouldBeTrue("checkSet(true, Boolean)");
     85shouldBeTrue("checkGetStrict(1, Number)");
     86shouldBeTrue("checkGetStrict('hello', String)");
     87shouldBeTrue("checkGetStrict(true, Boolean)");
     88shouldBeTrue("checkSetStrict(1, Number)");
     89shouldBeTrue("checkSetStrict('hello', String)");
     90shouldBeTrue("checkSetStrict(true, Boolean)");
     91
     92function checkRead(x, constructor)
     93{
     94    return x.foo === undefined;
     95}
     96
     97function checkWrite(x, constructor)
     98{
     99    x.foo = null;
     100    return x.foo === undefined;
     101}
     102
     103function checkReadStrict(x, constructor)
     104{
     105    "use strict";
     106    return x.foo === undefined;
     107}
     108
     109function checkWriteStrict(x, constructor)
     110{
     111    "use strict";
     112    x.foo = null;
     113    return x.foo === undefined;
     114}
     115
     116shouldBeTrue("checkRead(1, Number)");
     117shouldBeTrue("checkRead('hello', String)");
     118shouldBeTrue("checkRead(true, Boolean)");
     119shouldBeTrue("checkWrite(1, Number)");
     120shouldBeTrue("checkWrite('hello', String)");
     121shouldBeTrue("checkWrite(true, Boolean)");
     122shouldBeTrue("checkReadStrict(1, Number)");
     123shouldBeTrue("checkReadStrict('hello', String)");
     124shouldBeTrue("checkReadStrict(true, Boolean)");
     125shouldThrow("checkWriteStrict(1, Number)");
     126shouldThrow("checkWriteStrict('hello', String)");
     127shouldThrow("checkWriteStrict(true, Boolean)");
     128
    40129shouldBeTrue("didNotCrash");

  • trunk/LayoutTests/fast/js/script-tests/read-modify-eval.js

    r98407 r109177  
    120120    // In a strict mode function primitive this values are not converted, so
    121121    // the property access in the first eval is writing a value to a temporary
    122     // object, and should not be observed by the second eval.
     122    // object. This throws, per section 8.7.2.
    123123    "use strict";
    124124    eval('this.value = "Seekrit message";');
    … …  
    144144
    145145shouldBeTrue('primitiveThisTest.call(1);');
    146 shouldBeTrue('strictThisTest.call(1);');
     146shouldThrow('strictThisTest.call(1);');

  • trunk/Source/JavaScriptCore/ChangeLog

    r109174 r109177  
     12012-02-28  Gavin Barraclough  <barraclough@apple.com>
     2
     3        [[Get]]/[[Put]] for primitives should not wrap on strict accessor call
     4        https://bugs.webkit.org/show_bug.cgi?id=79588
     5
     6        Reviewed by Oliver Hunt.
     7
     8        In the case of [[Get]], this is a pretty trivial bug - just don't wrap
     9        primitives at the point you call a getter.
     10
     11        For setters, this is a little more involved, since we have already wrapped
     12        the value up in a synthesized object. Stop doing so. There is also a further
     13        subtely, that in strict mode all attempts to create a new data property on
     14        the object should throw.
     15
     16        * runtime/JSCell.cpp:
     17        (JSC::JSCell::put):
     18            - [[Put]] to a string primitive should use JSValue::putToPrimitive.
     19        * runtime/JSObject.cpp:
     20        (JSC::JSObject::put):
     21            - Remove static function called in one place.
     22        * runtime/JSObject.h:
     23        (JSC::JSValue::put):
     24            - [[Put]] to a non-cell JSValue should use JSValue::putToPrimitive.
     25        * runtime/JSValue.cpp:
     26        (JSC::JSValue::synthesizePrototype):
     27            - Add support for synthesizing the prototype of strings.
     28        (JSC::JSValue::putToPrimitive):
     29            - Added, implements [[Put]] for primitive bases, per 8.7.2.
     30        * runtime/JSValue.h:
     31        (JSValue):
     32            - Add declaration for JSValue::putToPrimitive.
     33        * runtime/PropertySlot.cpp:
     34        (JSC::PropertySlot::functionGetter):
     35            - Don't call ToObject on primitive this values.
     36
    1372012-02-28  Mark Hahnenberg  <mhahnenberg@apple.com>
    238

  • trunk/Source/JavaScriptCore/runtime/JSCell.cpp

    r107544 r109177  
    9898void JSCell::put(JSCell* cell, ExecState* exec, const Identifier& identifier, JSValue value, PutPropertySlot& slot)
    9999{
     100    if (cell->isString()) {
     101        JSValue(cell).putToPrimitive(exec, identifier, value, slot);
     102        return;
     103    }
    100104    JSObject* thisObject = cell->toObject(exec, exec->lexicalGlobalObject());
    101105    thisObject->methodTable()->put(thisObject, exec, identifier, value, slot);

  • trunk/Source/JavaScriptCore/runtime/JSObject.cpp

    r108427 r109177  
    120120}
    121121
    122 static void throwSetterError(ExecState* exec)
    123 {
    124     throwError(exec, createTypeError(exec, "setting a property that has only a getter"));
    125 }
    126 
    127122// ECMA 8.6.2.2
    128123void JSObject::put(JSCell* cell, ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot& slot)
    … …  
    162157                if (!setterFunc) {
    163158                    if (slot.isStrictMode())
    164                         throwSetterError(exec);
     159                        throwError(exec, createTypeError(exec, "setting a property that has only a getter"));
    165160                    return;
    166161                }

  • trunk/Source/JavaScriptCore/runtime/JSObject.h

    r108444 r109177  
    835835{
    836836    if (UNLIKELY(!isCell())) {
    837         JSObject* thisObject = synthesizeObject(exec);
    838         thisObject->methodTable()->put(thisObject, exec, propertyName, value, slot);
     837        putToPrimitive(exec, propertyName, value, slot);
    839838        return;
    840839    }

  • trunk/Source/JavaScriptCore/runtime/JSValue.cpp

    r108444 r109177  
    2828#include "Error.h"
    2929#include "ExceptionHelpers.h"
     30#include "GetterSetter.h"
    3031#include "JSGlobalObject.h"
    3132#include "JSFunction.h"
    … …  
    106107JSObject* JSValue::synthesizePrototype(ExecState* exec) const
    107108{
    108     ASSERT(!isCell());
     109    if (isCell()) {
     110        ASSERT(isString());
     111        return exec->lexicalGlobalObject()->stringPrototype();
     112    }
     113
    109114    if (isNumber())
    110115        return exec->lexicalGlobalObject()->numberPrototype();
    … …  
    115120    throwError(exec, createNotAnObjectError(exec, *this));
    116121    return JSNotAnObject::create(exec);
     122}
     123
     124// ECMA 8.7.2
     125void JSValue::putToPrimitive(ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot& slot)
     126{
     127    JSGlobalData& globalData = exec->globalData();
     128
     129    // Check if there are any setters or getters in the prototype chain
     130    JSObject* obj = synthesizePrototype(exec);
     131    JSValue prototype;
     132    if (propertyName != exec->propertyNames().underscoreProto) {
     133        for (; !obj->structure()->hasReadOnlyOrGetterSetterPropertiesExcludingProto(); obj = asObject(prototype)) {
     134            prototype = obj->prototype();
     135            if (prototype.isNull()) {
     136                if (slot.isStrictMode())
     137                    throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
     138                return;
     139            }
     140        }
     141    }
     142
     143    for (; ; obj = asObject(prototype)) {
     144        unsigned attributes;
     145        JSCell* specificValue;
     146        size_t offset = obj->structure()->get(globalData, propertyName, attributes, specificValue);
     147        if (offset != WTF::notFound) {
     148            if (attributes & ReadOnly) {
     149                if (slot.isStrictMode())
     150                    throwError(exec, createTypeError(exec, StrictModeReadonlyPropertyWriteError));
     151                return;
     152            }
     153
     154            JSValue gs = obj->getDirectOffset(offset);
     155            if (gs.isGetterSetter()) {
     156                JSObject* setterFunc = asGetterSetter(gs)->setter();       
     157                if (!setterFunc) {
     158                    if (slot.isStrictMode())
     159                        throwError(exec, createTypeError(exec, "setting a property that has only a getter"));
     160                    return;
     161                }
     162               
     163                CallData callData;
     164                CallType callType = setterFunc->methodTable()->getCallData(setterFunc, callData);
     165                MarkedArgumentBuffer args;
     166                args.append(value);
     167
     168                // If this is WebCore's global object then we need to substitute the shell.
     169                call(exec, setterFunc, callType, callData, *this, args);
     170                return;
     171            }
     172
     173            // If there's an existing property on the object or one of its
     174            // prototypes it should be replaced, so break here.
     175            break;
     176        }
     177
     178        prototype = obj->prototype();
     179        if (prototype.isNull())
     180            break;
     181    }
     182   
     183    if (slot.isStrictMode())
     184        throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
     185    return;
    117186}
    118187

  • trunk/Source/JavaScriptCore/runtime/JSValue.h

    r108444 r109177  
    222222        JSValue get(ExecState*, unsigned propertyName, PropertySlot&) const;
    223223        void put(ExecState*, const Identifier& propertyName, JSValue, PutPropertySlot&);
     224        void putToPrimitive(ExecState*, const Identifier& propertyName, JSValue, PutPropertySlot&);
    224225        void put(ExecState*, unsigned propertyName, JSValue);
    225226

  • trunk/Source/JavaScriptCore/runtime/PropertySlot.cpp

    r97097 r109177  
    3535    CallData callData;
    3636    CallType callType = m_data.getterFunc->methodTable()->getCallData(m_data.getterFunc, callData);
    37    
    38     // Only objects can have accessor properties.
    39     // If the base is WebCore's global object then we need to substitute the shell.
    40     ASSERT(m_slotBase.isObject());
    41     return call(exec, m_data.getterFunc, callType, callData, m_thisValue.toThisObject(exec), exec->emptyList());
     37    return call(exec, m_data.getterFunc, callType, callData, m_thisValue.isObject() ? m_thisValue.toThisObject(exec) : m_thisValue, exec->emptyList());
    4238}
    4339
Note: See TracChangeset for help on using the changeset viewer.