Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (109672 => 109673)
--- trunk/Source/_javascript_Core/ChangeLog 2012-03-04 05:04:16 UTC (rev 109672)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-03-04 05:56:24 UTC (rev 109673)
@@ -1,3 +1,34 @@
+2012-03-03 Gavin Barraclough <[email protected]>
+
+ Split JSArray's [[Put]] & [[DefineOwnProperty]] traps.
+ https://bugs.webkit.org/show_bug.cgi?id=80217
+
+ Reviewed by Filip Pizlo.
+
+ putByIndex() provides similar behavior to put(), but for indexed property names.
+ Many places in ArrayPrototype call putByIndex() where they really mean to call
+ [[DefineOwnProperty]]. This is only okay due to a bug – putByIndex should be
+ calling numeric accessors (& respecting numeric read only properties) on the
+ prototype chain, but isn't. Add a new putDirectIndex (matching JSObject's
+ putDirect* methods), to correctly provide a fast [[DefineOwnProperty]] interface.
+
+ * runtime/ArrayPrototype.cpp:
+ (JSC::arrayProtoFuncConcat):
+ (JSC::arrayProtoFuncSlice):
+ (JSC::arrayProtoFuncFilter):
+ (JSC::arrayProtoFuncMap):
+ * runtime/JSArray.cpp:
+ (JSC):
+ (JSC::reject):
+ (JSC::SparseArrayValueMap::putDirect):
+ (JSC::JSArray::defineOwnNumericProperty):
+ (JSC::JSArray::putByIndexBeyondVectorLength):
+ (JSC::JSArray::putDirectIndexBeyondVectorLength):
+ * runtime/JSArray.h:
+ (SparseArrayValueMap):
+ (JSArray):
+ (JSC::JSArray::putDirectIndex):
+
2012-03-03 Benjamin Poulain <[email protected]>
Implement the basis of KURLWTFURL
Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (109672 => 109673)
--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2012-03-04 05:04:16 UTC (rev 109672)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2012-03-04 05:56:24 UTC (rev 109673)
@@ -368,11 +368,11 @@
if (exec->hadException())
return JSValue::encode(jsUndefined());
if (v)
- arr->methodTable()->putByIndex(arr, exec, n, v);
+ arr->putDirectIndex(exec, n, v);
n++;
}
} else {
- arr->methodTable()->putByIndex(arr, exec, n, curArg);
+ arr->putDirectIndex(exec, n, curArg);
n++;
}
if (i == argCount)
@@ -521,7 +521,7 @@
if (exec->hadException())
return JSValue::encode(jsUndefined());
if (v)
- resObj->methodTable()->putByIndex(resObj, exec, n, v);
+ resObj->putDirectIndex(exec, n, v);
}
resObj->setLength(exec, n);
return JSValue::encode(result);
@@ -732,7 +732,7 @@
JSValue result = cachedCall.call();
if (result.toBoolean(exec))
- resultArray->methodTable()->putByIndex(resultArray, exec, filterIndex++, v);
+ resultArray->putDirectIndex(exec, filterIndex++, v);
}
if (k == length)
return JSValue::encode(resultArray);
@@ -753,7 +753,7 @@
JSValue result = call(exec, function, callType, callData, applyThis, eachArguments);
if (result.toBoolean(exec))
- resultArray->methodTable()->putByIndex(resultArray, exec, filterIndex++, v);
+ resultArray->putDirectIndex(exec, filterIndex++, v);
}
return JSValue::encode(resultArray);
}
@@ -788,7 +788,7 @@
cachedCall.setArgument(1, jsNumber(k));
cachedCall.setArgument(2, thisObj);
- JSArray::putByIndex(resultArray, exec, k, cachedCall.call());
+ resultArray->putDirectIndex(exec, k, cachedCall.call());
}
}
for (; k < length && !exec->hadException(); ++k) {
@@ -809,7 +809,7 @@
return JSValue::encode(jsUndefined());
JSValue result = call(exec, function, callType, callData, applyThis, eachArguments);
- resultArray->methodTable()->putByIndex(resultArray, exec, k, result);
+ resultArray->putDirectIndex(exec, k, result);
}
return JSValue::encode(resultArray);
Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (109672 => 109673)
--- trunk/Source/_javascript_Core/runtime/JSArray.cpp 2012-03-04 05:04:16 UTC (rev 109672)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp 2012-03-04 05:56:24 UTC (rev 109673)
@@ -41,6 +41,7 @@
namespace JSC {
+
ASSERT_CLASS_FITS_IN_CELL(JSArray);
ASSERT_HAS_TRIVIAL_DESTRUCTOR(JSArray);
@@ -109,6 +110,13 @@
return length <= MIN_SPARSE_ARRAY_INDEX || length / minDensityMultiplier <= numValues;
}
+static bool reject(ExecState* exec, bool throwException, const char* message)
+{
+ if (throwException)
+ throwTypeError(exec, message);
+ return false;
+}
+
#if !CHECK_ARRAY_CONSISTENCY
inline void JSArray::checkConsistency(ConsistencyCheckType)
@@ -241,6 +249,24 @@
call(exec, setter, callType, callData, array, args);
}
+inline bool SparseArrayValueMap::putDirect(ExecState* exec, JSArray* array, unsigned i, JSValue value, bool shouldThrow)
+{
+ std::pair<SparseArrayValueMap::iterator, bool> result = add(array, i);
+ SparseArrayEntry& entry = result.first->second;
+
+ // To save a separate find & add, we first always add to the sparse map.
+ // In the uncommon case that this is a new property, and the array is not
+ // extensible, this is not the right thing to have done - so remove again.
+ if (result.second && !array->isExtensible()) {
+ remove(result.first);
+ return reject(exec, shouldThrow, "Attempting to define property on object that is not extensible.");
+ }
+
+ entry.attributes = 0;
+ entry.set(exec->globalData(), array, value);
+ return true;
+}
+
inline void SparseArrayEntry::get(PropertySlot& slot) const
{
JSValue value = Base::get();
@@ -381,13 +407,6 @@
entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor);
}
-static bool reject(ExecState* exec, bool throwException, const char* message)
-{
- if (throwException)
- throwTypeError(exec, message);
- return false;
-}
-
// Defined in ES5.1 8.12.9
bool JSArray::defineOwnNumericProperty(ExecState* exec, unsigned index, PropertyDescriptor& descriptor, bool throwException)
{
@@ -400,8 +419,7 @@
// state (i.e. defineOwnProperty could be used to set a value without needing to entering 'SparseMode').
if (!descriptor.attributes()) {
ASSERT(!descriptor.isAccessorDescriptor());
- putByIndex(this, exec, index, descriptor.value());
- return true;
+ return putDirectIndex(exec, index, descriptor.value(), throwException);
}
enterDictionaryMode(exec->globalData());
@@ -761,7 +779,7 @@
thisObject->checkConsistency();
}
-NEVER_INLINE void JSArray::putByIndexBeyondVectorLength(ExecState* exec, unsigned i, JSValue value)
+void JSArray::putByIndexBeyondVectorLength(ExecState* exec, unsigned i, JSValue value)
{
JSGlobalData& globalData = exec->globalData();
@@ -834,6 +852,77 @@
valueSlot.set(globalData, this, value);
}
+bool JSArray::putDirectIndexBeyondVectorLength(ExecState* exec, unsigned i, JSValue value, bool shouldThrow)
+{
+ JSGlobalData& globalData = exec->globalData();
+
+ // i should be a valid array index that is outside of the current vector.
+ ASSERT(i >= m_vectorLength);
+ ASSERT(i <= MAX_ARRAY_INDEX);
+
+ ArrayStorage* storage = m_storage;
+ SparseArrayValueMap* map = m_sparseValueMap;
+
+ // First, handle cases where we don't currently have a sparse map.
+ if (LIKELY(!map)) {
+ // If the array is not extensible, we should have entered dictionary mode, and created the spare map.
+ ASSERT(isExtensible());
+
+ // Update m_length if necessary.
+ if (i >= storage->m_length)
+ storage->m_length = i + 1;
+
+ // Check that it is sensible to still be using a vector, and then try to grow the vector.
+ if (LIKELY((isDenseEnoughForVector(i, storage->m_numValuesInVector)) && increaseVectorLength(globalData, i + 1))) {
+ // success! - reread m_storage since it has likely been reallocated, and store to the vector.
+ storage = m_storage;
+ storage->m_vector[i].set(globalData, this, value);
+ ++storage->m_numValuesInVector;
+ return true;
+ }
+ // We don't want to, or can't use a vector to hold this property - allocate a sparse map & add the value.
+ allocateSparseMap(exec->globalData());
+ map = m_sparseValueMap;
+ return map->putDirect(exec, this, i, value, shouldThrow);
+ }
+
+ // Update m_length if necessary.
+ unsigned length = storage->m_length;
+ if (i >= length) {
+ // Prohibit growing the array if length is not writable.
+ if (map->lengthIsReadOnly())
+ return reject(exec, shouldThrow, StrictModeReadonlyPropertyWriteError);
+ if (!isExtensible())
+ return reject(exec, shouldThrow, "Attempting to define property on object that is not extensible.");
+ length = i + 1;
+ storage->m_length = length;
+ }
+
+ // We are currently using a map - check whether we still want to be doing so.
+ // We will continue to use a sparse map if SparseMode is set, a vector would be too sparse, or if allocation fails.
+ unsigned numValuesInArray = storage->m_numValuesInVector + map->size();
+ if (map->sparseMode() || !isDenseEnoughForVector(length, numValuesInArray) || !increaseVectorLength(exec->globalData(), length))
+ return map->putDirect(exec, this, i, value, shouldThrow);
+
+ // Reread m_storage afterincreaseVectorLength, update m_numValuesInVector.
+ storage = m_storage;
+ storage->m_numValuesInVector = numValuesInArray;
+
+ // Copy all values from the map into the vector, and delete the map.
+ WriteBarrier<Unknown>* vector = storage->m_vector;
+ SparseArrayValueMap::const_iterator end = map->end();
+ for (SparseArrayValueMap::const_iterator it = map->begin(); it != end; ++it)
+ vector[it->first].set(globalData, this, it->second.getNonSparseMode());
+ deallocateSparseMap();
+
+ // Store the new property into the vector.
+ WriteBarrier<Unknown>& valueSlot = vector[i];
+ if (!valueSlot)
+ ++storage->m_numValuesInVector;
+ valueSlot.set(globalData, this, value);
+ return true;
+}
+
bool JSArray::deleteProperty(JSCell* cell, ExecState* exec, const Identifier& propertyName)
{
JSArray* thisObject = jsCast<JSArray*>(cell);
Modified: trunk/Source/_javascript_Core/runtime/JSArray.h (109672 => 109673)
--- trunk/Source/_javascript_Core/runtime/JSArray.h 2012-03-04 05:04:16 UTC (rev 109672)
+++ trunk/Source/_javascript_Core/runtime/JSArray.h 2012-03-04 05:56:24 UTC (rev 109673)
@@ -86,6 +86,7 @@
// These methods may mutate the contents of the map
void put(ExecState*, JSArray*, unsigned, JSValue);
+ bool putDirect(ExecState*, JSArray*, unsigned, JSValue, bool shouldThrow);
std::pair<iterator, bool> add(JSArray*, unsigned);
iterator find(unsigned i) { return m_map.find(i); }
// This should ASSERT the remove is valid (check the result of the find).
@@ -160,6 +161,18 @@
JS_EXPORT_PRIVATE static bool getOwnPropertySlotByIndex(JSCell*, ExecState*, unsigned propertyName, PropertySlot&);
static bool getOwnPropertyDescriptor(JSObject*, ExecState*, const Identifier&, PropertyDescriptor&);
static void putByIndex(JSCell*, ExecState*, unsigned propertyName, JSValue);
+ // This is similar to the JSObject::putDirect* methods:
+ // - the prototype chain is not consulted
+ // - accessors are not called.
+ // This method creates a property with attributes writable, enumerable and configurable all set to true.
+ bool putDirectIndex(ExecState* exec, unsigned propertyName, JSValue value, bool shouldThrow = true)
+ {
+ if (canSetIndex(propertyName)) {
+ setIndex(exec->globalData(), propertyName, value);
+ return true;
+ }
+ return putDirectIndexBeyondVectorLength(exec, propertyName, value, shouldThrow);
+ }
static JS_EXPORTDATA const ClassInfo s_info;
@@ -283,6 +296,7 @@
bool getOwnPropertySlotSlowCase(ExecState*, unsigned propertyName, PropertySlot&);
void putByIndexBeyondVectorLength(ExecState*, unsigned propertyName, JSValue);
+ bool putDirectIndexBeyondVectorLength(ExecState*, unsigned propertyName, JSValue, bool shouldThrow);
unsigned getNewVectorLength(unsigned desiredLength);
bool increaseVectorLength(JSGlobalData&, unsigned newLength);
