diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 198b65be594d150654a746f3504bebe1d4c42770..647d8a0d818151ca1e37336958bc193eb13c0ad1 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,20 @@ +2012-02-27 Gavin Barraclough + + RegExp lastIndex should behave as a regular property + https://bugs.webkit.org/show_bug.cgi?id=79446 + + Reviewed by Sam Weinig. + + lastIndex should be a regular data descriptor, with the attributes configurable:false, + enumerable:false, writable:true. As such, it should be possible to reconfigure writable + as false. If the lastIndex property is reconfigured to be read-only, we should respect + this correctly. + + * fast/regex/lastIndex-expected.txt: Added. + * fast/regex/lastIndex.html: Added. + * fast/regex/script-tests/lastIndex.js: Added. + - Added test cases for correct handling of lastIndex. + 2012-02-27 Adrienne Walker [chromium] Unreviewed gardening, mark getPutImageDataPair as slow diff --git a/LayoutTests/fast/regex/lastIndex-expected.txt b/LayoutTests/fast/regex/lastIndex-expected.txt new file mode 100644 index 0000000000000000000000000000000000000000..1fae27180eef06cc2232b153595a169f5a2b41db --- /dev/null +++ b/LayoutTests/fast/regex/lastIndex-expected.txt @@ -0,0 +1,30 @@ +This page tests that a RegExp object's lastIndex behaves like a regular property. + +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". + + +PASS delete /x/.lastIndex is false +PASS 'use strict'; delete /x/.lastIndex threw exception TypeError: Unable to delete property.. +PASS 'lastIndex' in /x/ is true +PASS for (property in /x/) if (property === 'lastIndex') throw false; true is true +PASS var re = /x/; re.lastIndex = re; re.lastIndex === re is true +PASS Object.defineProperty(/x/, {get:function(){}}) threw exception TypeError: Property description must be an object.. +PASS Object.defineProperty(/x/, 'lastIndex', {enumerable:true}); true threw exception TypeError: Attempting to change enumerable attribute of unconfigurable property.. +PASS Object.defineProperty(/x/, 'lastIndex', {enumerable:false}); true is true +PASS Object.defineProperty(/x/, 'lastIndex', {configurable:true}); true threw exception TypeError: Attempting to change configurable attribute of unconfigurable property.. +PASS Object.defineProperty(/x/, 'lastIndex', {configurable:false}); true is true +PASS var re = Object.defineProperty(/x/, 'lastIndex', {writable:true}); re.lastIndex = 42; re.lastIndex is 42 +PASS var re = Object.defineProperty(/x/, 'lastIndex', {writable:false}); re.lastIndex = 42; re.lastIndex is 0 +PASS var re = Object.defineProperty(/x/, 'lastIndex', {value:42}); re.lastIndex is 42 +PASS Object.defineProperty(Object.defineProperty(/x/, 'lastIndex', {writable:false}), 'lastIndex', {writable:true}); true threw exception TypeError: Attempting to change writable attribute of unconfigurable property.. +PASS Object.defineProperty(Object.defineProperty(/x/, 'lastIndex', {writable:false}), 'lastIndex', {value:42}); true threw exception TypeError: Attempting to change value of a readonly property.. +PASS Object.defineProperty(Object.defineProperty(/x/, 'lastIndex', {writable:false}), 'lastIndex', {value:0}); true is true +PASS Object.defineProperty(/x/, 'lastIndex', {writable:false}).exec('') is null +PASS Object.defineProperty(/x/, 'lastIndex', {writable:false}).exec('x') is ["x"] +PASS Object.defineProperty(/x/g, 'lastIndex', {writable:false}).exec('') threw exception TypeError: Attempted to assign to readonly property.. +PASS Object.defineProperty(/x/g, 'lastIndex', {writable:false}).exec('x') threw exception TypeError: Attempted to assign to readonly property.. +PASS var re = /x/; Object.freeze(re); Object.isFrozen(re); is true +PASS successfullyParsed is true + +TEST COMPLETE + diff --git a/LayoutTests/fast/regex/lastIndex.html b/LayoutTests/fast/regex/lastIndex.html new file mode 100644 index 0000000000000000000000000000000000000000..64c447a6d3314c3253159b34d75bda3b12e5f640 --- /dev/null +++ b/LayoutTests/fast/regex/lastIndex.html @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/LayoutTests/fast/regex/script-tests/lastIndex.js b/LayoutTests/fast/regex/script-tests/lastIndex.js new file mode 100644 index 0000000000000000000000000000000000000000..53d5124ffa816cb8581cb6513115c5ef6780d28f --- /dev/null +++ b/LayoutTests/fast/regex/script-tests/lastIndex.js @@ -0,0 +1,48 @@ +description( +"This page tests that a RegExp object's lastIndex behaves like a regular property." +); + +// lastIndex is not configurable +shouldBeFalse("delete /x/.lastIndex"); +shouldThrow("'use strict'; delete /x/.lastIndex"); + +// lastIndex is not enumerable +shouldBeTrue("'lastIndex' in /x/"); +shouldBeTrue("for (property in /x/) if (property === 'lastIndex') throw false; true"); + +// lastIndex is writable +shouldBeTrue("var re = /x/; re.lastIndex = re; re.lastIndex === re"); + +// Cannot redefine lastIndex as an accessor +shouldThrow("Object.defineProperty(/x/, {get:function(){}})"); + +// Cannot redefine lastIndex as enumerable +shouldThrow("Object.defineProperty(/x/, 'lastIndex', {enumerable:true}); true"); +shouldBeTrue("Object.defineProperty(/x/, 'lastIndex', {enumerable:false}); true"); + +// Cannot redefine lastIndex as configurable +shouldThrow("Object.defineProperty(/x/, 'lastIndex', {configurable:true}); true"); +shouldBeTrue("Object.defineProperty(/x/, 'lastIndex', {configurable:false}); true"); + +// Can redefine lastIndex as read-only +shouldBe("var re = Object.defineProperty(/x/, 'lastIndex', {writable:true}); re.lastIndex = 42; re.lastIndex", '42'); +shouldBe("var re = Object.defineProperty(/x/, 'lastIndex', {writable:false}); re.lastIndex = 42; re.lastIndex", '0'); + +// Can redefine the value +shouldBe("var re = Object.defineProperty(/x/, 'lastIndex', {value:42}); re.lastIndex", '42'); + +// Cannot redefine read-only lastIndex as writable +shouldThrow("Object.defineProperty(Object.defineProperty(/x/, 'lastIndex', {writable:false}), 'lastIndex', {writable:true}); true"); + +// Can only redefine the value of a read-only lastIndex as the current value +shouldThrow("Object.defineProperty(Object.defineProperty(/x/, 'lastIndex', {writable:false}), 'lastIndex', {value:42}); true"); +shouldBeTrue("Object.defineProperty(Object.defineProperty(/x/, 'lastIndex', {writable:false}), 'lastIndex', {value:0}); true"); + +// Trying to run a global regular expression should throw, if lastIndex is read-only +shouldBe("Object.defineProperty(/x/, 'lastIndex', {writable:false}).exec('')", 'null'); +shouldBe("Object.defineProperty(/x/, 'lastIndex', {writable:false}).exec('x')", '["x"]'); +shouldThrow("Object.defineProperty(/x/g, 'lastIndex', {writable:false}).exec('')"); +shouldThrow("Object.defineProperty(/x/g, 'lastIndex', {writable:false}).exec('x')"); + +// Should be able to freeze a regular expression object. +shouldBeTrue("var re = /x/; Object.freeze(re); Object.isFrozen(re);"); diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog index b2af8436b00dba37decfe953c0a038c492ec1eaa..6e1d99524d66b3577bd2f6835c0381fb17e0e306 100644 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,45 @@ +2012-02-27 Gavin Barraclough + + RegExp lastIndex should behave as a regular property + https://bugs.webkit.org/show_bug.cgi?id=79446 + + Reviewed by Sam Weinig. + + lastIndex should be a regular data descriptor, with the attributes configurable:false, + enumerable:false, writable:true. As such, it should be possible to reconfigure writable + as false. If the lastIndex property is reconfigured to be read-only, we should respect + this correctly. + + * runtime/CommonIdentifiers.h: + - Removed some unused identifiers, added lastIndex. + * runtime/RegExpObject.cpp: + (JSC::RegExpObject::getOwnPropertySlot): + - lastIndex is no longer a static value, provided specific handling. + (JSC::RegExpObject::getOwnPropertyDescriptor): + - lastIndex is no longer a static value, provided specific handling. + (JSC::RegExpObject::deleteProperty): + - lastIndex is no longer a static value, provided specific handling. + (JSC::RegExpObject::getOwnPropertyNames): + - lastIndex is no longer a static value, provided specific handling. + (JSC::RegExpObject::getPropertyNames): + - lastIndex is no longer a static value, provided specific handling. + (JSC::reject): + - helper function for defineOwnProperty. + (JSC::RegExpObject::defineOwnProperty): + - lastIndex is no longer a static value, provided specific handling. + (JSC::RegExpObject::put): + - lastIndex is no longer a static value, provided specific handling. + (JSC::RegExpObject::match): + - Pass setLastIndex an ExecState, so it can throw if read-only. + * runtime/RegExpObject.h: + (JSC::RegExpObject::setLastIndex): + - Pass setLastIndex an ExecState, so it can throw if read-only. + (RegExpObjectData): + - Added lastIndexIsWritable. + * runtime/RegExpPrototype.cpp: + (JSC::regExpProtoFuncCompile): + - Pass setLastIndex an ExecState, so it can throw if read-only. + 2012-02-27 Gavin Barraclough Implement support for op_negate and op_bitnot in the DFG JIT diff --git a/Source/JavaScriptCore/runtime/CommonIdentifiers.h b/Source/JavaScriptCore/runtime/CommonIdentifiers.h index d79e5c783a369f64224c913f384d33ab38e5ba5b..0d95801972251ddd72d7c82d9dddeba023d53a28 100644 --- a/Source/JavaScriptCore/runtime/CommonIdentifiers.h +++ b/Source/JavaScriptCore/runtime/CommonIdentifiers.h @@ -27,10 +27,6 @@ // MarkedArgumentBuffer of property names, passed to a macro so we can do set them up various // ways without repeating the list. #define JSC_COMMON_IDENTIFIERS_EACH_PROPERTY_NAME(macro) \ - macro(__defineGetter__) \ - macro(__defineSetter__) \ - macro(__lookupGetter__) \ - macro(__lookupSetter__) \ macro(apply) \ macro(arguments) \ macro(bind) \ @@ -52,6 +48,7 @@ macro(input) \ macro(isArray) \ macro(isPrototypeOf) \ + macro(lastIndex) \ macro(length) \ macro(message) \ macro(multiline) \ diff --git a/Source/JavaScriptCore/runtime/RegExpObject.cpp b/Source/JavaScriptCore/runtime/RegExpObject.cpp index 4c192ff9028aea6bdd824e18bb3b0bd6320d96f5..9cfd89e392999c10dc3b6d6e18a02f961e7ae973 100644 --- a/Source/JavaScriptCore/runtime/RegExpObject.cpp +++ b/Source/JavaScriptCore/runtime/RegExpObject.cpp @@ -40,8 +40,6 @@ static JSValue regExpObjectGlobal(ExecState*, JSValue, const Identifier&); static JSValue regExpObjectIgnoreCase(ExecState*, JSValue, const Identifier&); static JSValue regExpObjectMultiline(ExecState*, JSValue, const Identifier&); static JSValue regExpObjectSource(ExecState*, JSValue, const Identifier&); -static JSValue regExpObjectLastIndex(ExecState*, JSValue, const Identifier&); -static void setRegExpObjectLastIndex(ExecState*, JSObject*, JSValue); } // namespace JSC @@ -59,7 +57,6 @@ const ClassInfo RegExpObject::s_info = { "RegExp", &JSNonFinalObject::s_info, 0, ignoreCase regExpObjectIgnoreCase DontDelete|ReadOnly|DontEnum multiline regExpObjectMultiline DontDelete|ReadOnly|DontEnum source regExpObjectSource DontDelete|ReadOnly|DontEnum - lastIndex regExpObjectLastIndex DontDelete|DontEnum @end */ @@ -95,14 +92,79 @@ void RegExpObject::visitChildren(JSCell* cell, SlotVisitor& visitor) bool RegExpObject::getOwnPropertySlot(JSCell* cell, ExecState* exec, const Identifier& propertyName, PropertySlot& slot) { + if (propertyName == exec->propertyNames().lastIndex) { + RegExpObject* regExp = asRegExpObject(cell); + slot.setValue(regExp, regExp->getLastIndex()); + return true; + } return getStaticValueSlot(exec, ExecState::regExpTable(exec), jsCast(cell), propertyName, slot); } bool RegExpObject::getOwnPropertyDescriptor(JSObject* object, ExecState* exec, const Identifier& propertyName, PropertyDescriptor& descriptor) { + if (propertyName == exec->propertyNames().lastIndex) { + RegExpObject* regExp = asRegExpObject(object); + descriptor.setDescriptor(regExp->getLastIndex(), regExp->d->lastIndexIsWritable ? DontDelete | DontEnum : DontDelete | DontEnum | ReadOnly); + return true; + } return getStaticValueDescriptor(exec, ExecState::regExpTable(exec), jsCast(object), propertyName, descriptor); } +bool RegExpObject::deleteProperty(JSCell* cell, ExecState* exec, const Identifier& propertyName) +{ + if (propertyName == exec->propertyNames().lastIndex) + return false; + return Base::deleteProperty(cell, exec, propertyName); +} + +void RegExpObject::getOwnPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNames, EnumerationMode mode) +{ + if (mode == IncludeDontEnumProperties) + propertyNames.add(exec->propertyNames().lastIndex); + Base::getOwnPropertyNames(object, exec, propertyNames, mode); +} + +void RegExpObject::getPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNames, EnumerationMode mode) +{ + if (mode == IncludeDontEnumProperties) + propertyNames.add(exec->propertyNames().lastIndex); + Base::getPropertyNames(object, exec, propertyNames, mode); +} + +static bool reject(ExecState* exec, bool throwException, const char* message) +{ + if (throwException) + throwTypeError(exec, message); + return false; +} + +bool RegExpObject::defineOwnProperty(JSObject* object, ExecState* exec, const Identifier& propertyName, PropertyDescriptor& descriptor, bool shouldThrow) +{ + if (propertyName == exec->propertyNames().lastIndex) { + RegExpObject* regExp = asRegExpObject(object); + if (descriptor.configurablePresent() && descriptor.configurable()) + return reject(exec, shouldThrow, "Attempting to change configurable attribute of unconfigurable property."); + if (descriptor.enumerablePresent() && descriptor.enumerable()) + return reject(exec, shouldThrow, "Attempting to change enumerable attribute of unconfigurable property."); + if (descriptor.isAccessorDescriptor()) + return reject(exec, shouldThrow, "Attempting to change access mechanism for an unconfigurable property."); + if (!regExp->d->lastIndexIsWritable) { + if (descriptor.writablePresent() && descriptor.writable()) + return reject(exec, shouldThrow, "Attempting to change writable attribute of unconfigurable property."); + if (!sameValue(exec, regExp->getLastIndex(), descriptor.value())) + return reject(exec, shouldThrow, "Attempting to change value of a readonly property."); + return true; + } + if (descriptor.writablePresent() && !descriptor.writable()) + regExp->d->lastIndexIsWritable = false; + if (descriptor.value()) + regExp->setLastIndex(exec, descriptor.value(), false); + return true; + } + + return Base::defineOwnProperty(object, exec, propertyName, descriptor, shouldThrow); +} + JSValue regExpObjectGlobal(ExecState*, JSValue slotBase, const Identifier&) { return jsBoolean(asRegExpObject(slotBase)->regExp()->global()); @@ -200,21 +262,15 @@ JSValue regExpObjectSource(ExecState* exec, JSValue slotBase, const Identifier&) return jsString(exec, result.toUString()); } -JSValue regExpObjectLastIndex(ExecState*, JSValue slotBase, const Identifier&) -{ - return asRegExpObject(slotBase)->getLastIndex(); -} - void RegExpObject::put(JSCell* cell, ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot& slot) { + if (propertyName == exec->propertyNames().lastIndex) { + asRegExpObject(cell)->setLastIndex(exec, value, slot.isStrictMode()); + return; + } lookupPut(exec, propertyName, value, ExecState::regExpTable(exec), jsCast(cell), slot); } -void setRegExpObjectLastIndex(ExecState* exec, JSObject* baseObject, JSValue value) -{ - asRegExpObject(baseObject)->setLastIndex(exec->globalData(), value); -} - JSValue RegExpObject::test(ExecState* exec) { return jsBoolean(match(exec)); @@ -245,13 +301,13 @@ bool RegExpObject::match(ExecState* exec) if (LIKELY(jsLastIndex.isUInt32())) { lastIndex = jsLastIndex.asUInt32(); if (lastIndex > input.length()) { - setLastIndex(0); + setLastIndex(exec, 0); return false; } } else { double doubleLastIndex = jsLastIndex.toInteger(exec); if (doubleLastIndex < 0 || doubleLastIndex > input.length()) { - setLastIndex(0); + setLastIndex(exec, 0); return false; } lastIndex = static_cast(doubleLastIndex); @@ -261,11 +317,11 @@ bool RegExpObject::match(ExecState* exec) int length = 0; regExpConstructor->performMatch(*globalData, d->regExp.get(), input, lastIndex, position, length); if (position < 0) { - setLastIndex(0); + setLastIndex(exec, 0); return false; } - setLastIndex(position + length); + setLastIndex(exec, position + length); return true; } diff --git a/Source/JavaScriptCore/runtime/RegExpObject.h b/Source/JavaScriptCore/runtime/RegExpObject.h index 081a7f11116851af29892700aa63921e16226309..8ed2618bb86edc59fefa229cf176e29c1d197bd9 100644 --- a/Source/JavaScriptCore/runtime/RegExpObject.h +++ b/Source/JavaScriptCore/runtime/RegExpObject.h @@ -47,13 +47,20 @@ namespace JSC { void setRegExp(JSGlobalData& globalData, RegExp* r) { d->regExp.set(globalData, this, r); } RegExp* regExp() const { return d->regExp.get(); } - void setLastIndex(size_t lastIndex) + void setLastIndex(ExecState* exec, size_t lastIndex) { d->lastIndex.setWithoutWriteBarrier(jsNumber(lastIndex)); + if (LIKELY(d->lastIndexIsWritable)) + d->lastIndex.setWithoutWriteBarrier(jsNumber(lastIndex)); + else + throwTypeError(exec, StrictModeReadonlyPropertyWriteError); } - void setLastIndex(JSGlobalData& globalData, JSValue lastIndex) + void setLastIndex(ExecState* exec, JSValue lastIndex, bool shouldThrow) { - d->lastIndex.set(globalData, this, lastIndex); + if (LIKELY(d->lastIndexIsWritable)) + d->lastIndex.set(exec->globalData(), this, lastIndex); + else if (shouldThrow) + throwTypeError(exec, StrictModeReadonlyPropertyWriteError); } JSValue getLastIndex() const { @@ -83,6 +90,11 @@ namespace JSC { static void visitChildren(JSCell*, SlotVisitor&); + JS_EXPORT_PRIVATE static bool deleteProperty(JSCell*, ExecState*, const Identifier& propertyName); + JS_EXPORT_PRIVATE static void getOwnPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode); + JS_EXPORT_PRIVATE static void getPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode); + JS_EXPORT_PRIVATE static bool defineOwnProperty(JSObject*, ExecState*, const Identifier& propertyName, PropertyDescriptor&, bool shouldThrow); + private: bool match(ExecState*); @@ -91,12 +103,14 @@ namespace JSC { public: RegExpObjectData(JSGlobalData& globalData, RegExpObject* owner, RegExp* regExp) : regExp(globalData, owner, regExp) + , lastIndexIsWritable(true) { lastIndex.setWithoutWriteBarrier(jsNumber(0)); } WriteBarrier regExp; WriteBarrier lastIndex; + bool lastIndexIsWritable; }; #if COMPILER(MSVC) friend void WTF::deleteOwnedPtr(RegExpObjectData*); diff --git a/Source/JavaScriptCore/runtime/RegExpPrototype.cpp b/Source/JavaScriptCore/runtime/RegExpPrototype.cpp index 9074e97c322e0b8713bf3c8eb29611fb699cff1b..e5bebfd8f706763861472bd6aae43e04543ab569 100644 --- a/Source/JavaScriptCore/runtime/RegExpPrototype.cpp +++ b/Source/JavaScriptCore/runtime/RegExpPrototype.cpp @@ -129,7 +129,7 @@ EncodedJSValue JSC_HOST_CALL regExpProtoFuncCompile(ExecState* exec) return throwVMError(exec, createSyntaxError(exec, regExp->errorMessage())); asRegExpObject(thisValue)->setRegExp(exec->globalData(), regExp); - asRegExpObject(thisValue)->setLastIndex(0); + asRegExpObject(thisValue)->setLastIndex(exec, 0); return JSValue::encode(jsUndefined()); }