From d7aa952e143accc18d54707d956d019272197078 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Fri, 3 Feb 2017 20:58:22 +0100 Subject: Make writes to ArrayData write-barrier safe Change-Id: I2e46100fe72fd83b36b3195130eefce5289d1627 Reviewed-by: Simon Hausmann --- src/qml/jsruntime/qv4arraydata.cpp | 65 +++++++++++++++++++------------------ src/qml/jsruntime/qv4arraydata_p.h | 19 +++++++---- src/qml/jsruntime/qv4engine.cpp | 4 ++- src/qml/jsruntime/qv4lookup.cpp | 4 +-- src/qml/jsruntime/qv4memberdata.cpp | 1 + src/qml/jsruntime/qv4object.cpp | 7 ++-- src/qml/jsruntime/qv4runtime.cpp | 2 +- 7 files changed, 58 insertions(+), 44 deletions(-) diff --git a/src/qml/jsruntime/qv4arraydata.cpp b/src/qml/jsruntime/qv4arraydata.cpp index ef1a7bee3c..f8c2d3f82b 100644 --- a/src/qml/jsruntime/qv4arraydata.cpp +++ b/src/qml/jsruntime/qv4arraydata.cpp @@ -159,7 +159,7 @@ void ArrayData::realloc(Object *o, Type newType, uint requested, bool enforceAtt } newData->setAlloc(alloc); newData->setType(newType); - newData->setAttrs(enforceAttributes ? reinterpret_cast(newData->d()->values.v + alloc) : 0); + newData->setAttrs(enforceAttributes ? reinterpret_cast(newData->d()->values.values + alloc) : 0); o->setArrayData(newData); if (d) { @@ -173,10 +173,12 @@ void ArrayData::realloc(Object *o, Type newType, uint requested, bool enforceAtt if (toCopy > d->d()->values.alloc - offset) { uint copyFromStart = toCopy - (d->d()->values.alloc - offset); - memcpy(newData->d()->values.v + toCopy - copyFromStart, d->d()->values.v, sizeof(Value)*copyFromStart); + // no write barrier required here + memcpy(newData->d()->values.values + toCopy - copyFromStart, d->d()->values.values, sizeof(Value)*copyFromStart); toCopy -= copyFromStart; } - memcpy(newData->d()->values.v, d->d()->values.v + offset, sizeof(Value)*toCopy); + // no write barrier required here + memcpy(newData->d()->values.values, d->d()->values.values + offset, sizeof(Value)*toCopy); } if (newType != Heap::ArrayData::Sparse) @@ -201,8 +203,8 @@ void ArrayData::realloc(Object *o, Type newType, uint requested, bool enforceAtt n->value = i; } else { storeValue(lastFree, i); - sparse->values[i].setEmpty(); - lastFree = &sparse->values[i].rawValueRef(); + sparse->values.values[i].setEmpty(); + lastFree = &sparse->values.values[i].rawValueRef(); } } } @@ -210,8 +212,8 @@ void ArrayData::realloc(Object *o, Type newType, uint requested, bool enforceAtt if (toCopy < sparse->values.alloc) { for (uint i = toCopy; i < sparse->values.alloc; ++i) { storeValue(lastFree, i); - sparse->values[i].setEmpty(); - lastFree = &sparse->values[i].rawValueRef(); + sparse->values.values[i].setEmpty(); + lastFree = &sparse->values.values[i].rawValueRef(); } storeValue(lastFree, UINT_MAX); } @@ -247,7 +249,7 @@ bool SimpleArrayData::put(Object *o, uint index, const Value &value) Heap::SimpleArrayData *dd = o->d()->arrayData.cast(); Q_ASSERT(index >= dd->values.size || !dd->attrs || !dd->attrs[index].isAccessor()); // ### honour attributes - dd->data(index) = value; + dd->setData(o->engine(), index, value); if (index >= dd->values.size) { if (dd->attrs) dd->attrs[index] = Attr_Data; @@ -263,7 +265,7 @@ bool SimpleArrayData::del(Object *o, uint index) return true; if (!dd->attrs || dd->attrs[index].isConfigurable()) { - dd->data(index) = Primitive::emptyValue(); + dd->setData(o->engine(), index, Primitive::emptyValue()); if (dd->attrs) dd->attrs[index] = Attr_Data; return true; @@ -296,7 +298,7 @@ void SimpleArrayData::push_front(Object *o, const Value *values, uint n) } dd->values.size += n; for (uint i = 0; i < n; ++i) - dd->data(i) = values[i].asReturnedValue(); + dd->setData(o->engine(), i, values[i]); } ReturnedValue SimpleArrayData::pop_front(Object *o) @@ -343,10 +345,11 @@ bool SimpleArrayData::putArray(Object *o, uint index, const Value *values, uint reallocate(o, index + n + 1, false); dd = o->d()->arrayData.cast(); } + QV4::ExecutionEngine *e = o->engine(); for (uint i = dd->values.size; i < index; ++i) - dd->data(i) = Primitive::emptyValue(); + dd->setData(e, i, Primitive::emptyValue()); for (uint i = 0; i < n; ++i) - dd->data(index + i) = values[i]; + dd->setData(e, index + i, values[i]); dd->values.size = qMax(dd->values.size, index + n); return true; } @@ -354,7 +357,7 @@ bool SimpleArrayData::putArray(Object *o, uint index, const Value *values, uint void SparseArrayData::free(Heap::ArrayData *d, uint idx) { Q_ASSERT(d && d->type == Heap::ArrayData::Sparse); - Value *v = d->values.v + idx; + Value *v = d->values.values + idx; if (d->attrs && d->attrs[idx].isAccessor()) { // double slot, free both. Order is important, so we have a double slot for allocation again afterwards. v[1].setEmpty(Value::fromReturnedValue(d->freeList).emptyValue()); @@ -398,7 +401,7 @@ uint SparseArrayData::allocate(Object *o, bool doubleSlot) dd->attrs[idx] = Attr_Accessor; return idx; } - last = &dd->values[Value::fromReturnedValue(*last).value()].rawValueRef(); + last = &dd->values.values[Value::fromReturnedValue(*last).value()].rawValueRef(); } } else { if (Value::fromReturnedValue(dd->freeList).value() == UINT_MAX) { @@ -435,7 +438,7 @@ bool SparseArrayData::put(Object *o, uint index, const Value &value) if (n->value == UINT_MAX) n->value = allocate(o); s = o->d()->arrayData.cast(); - s->values[n->value] = value; + s->setArrayData(o->engine(), n->value, value); if (s->attrs) s->attrs[n->value] = Attr_Data; return true; @@ -463,11 +466,11 @@ bool SparseArrayData::del(Object *o, uint index) if (isAccessor) { // free up both indices - dd->values[pidx + 1].setEmpty(Value::fromReturnedValue(dd->freeList).emptyValue()); - dd->values[pidx].setEmpty(pidx + 1); + dd->values.values[pidx + 1].setEmpty(Value::fromReturnedValue(dd->freeList).emptyValue()); + dd->values.values[pidx].setEmpty(pidx + 1); } else { Q_ASSERT(dd->type == Heap::ArrayData::Sparse); - dd->values[pidx].setEmpty(Value::fromReturnedValue(dd->freeList).emptyValue()); + dd->values.values[pidx].setEmpty(Value::fromReturnedValue(dd->freeList).emptyValue()); } dd->freeList = Primitive::emptyValue(pidx).asReturnedValue(); @@ -499,7 +502,7 @@ void SparseArrayData::push_front(Object *o, const Value *values, uint n) for (int i = static_cast(n) - 1; i >= 0; --i) { uint idx = allocate(o); d = o->d()->arrayData.cast(); - d->values[idx] = values[i]; + d->setArrayData(o->engine(), idx, values[i]); d->sparse->push_front(idx); } } @@ -603,10 +606,10 @@ uint ArrayData::append(Object *obj, ArrayObject *otherObj, uint n) uint chunk = toCopy; if (chunk > os->values.alloc - os->offset) chunk -= os->values.alloc - os->offset; - obj->arrayPut(oldSize, os->values.v + os->offset, chunk); + obj->arrayPut(oldSize, os->values.data() + os->offset, chunk); toCopy -= chunk; if (toCopy) - obj->arrayPut(oldSize + chunk, os->values.v, toCopy); + obj->arrayPut(oldSize + chunk, os->values.data(), toCopy); } return oldSize + n; @@ -624,10 +627,10 @@ void ArrayData::insert(Object *o, uint index, const Value *v, bool isAccessor) if (index >= d->values.size) { // mark possible hole in the array for (uint i = d->values.size; i < index; ++i) - d->data(i) = Primitive::emptyValue(); + d->setData(o->engine(), i, Primitive::emptyValue()); d->values.size = index + 1; } - d->values[d->mappedIndex(index)] = *v; + d->setData(o->engine(), index, *v); return; } } @@ -638,9 +641,9 @@ void ArrayData::insert(Object *o, uint index, const Value *v, bool isAccessor) if (n->value == UINT_MAX) n->value = SparseArrayData::allocate(o, isAccessor); s = o->d()->arrayData.cast(); - s->values[n->value] = *v; + s->setArrayData(o->engine(), n->value, *v); if (isAccessor) - s->values[n->value + Object::SetterOffset] = v[Object::SetterOffset]; + s->setArrayData(o->engine(), n->value + Object::SetterOffset, v[Object::SetterOffset]); } @@ -777,7 +780,7 @@ void ArrayData::sort(ExecutionEngine *engine, Object *thisObject, const Value &c break; PropertyAttributes a = sparse->attrs() ? sparse->attrs()[n->value] : Attr_Data; - d->data(i) = thisObject->getValue(sparse->arrayData()[n->value], a); + d->setData(engine, i, Value::fromReturnedValue(thisObject->getValue(sparse->arrayData()[n->value], a))); d->attrs[i] = a.isAccessor() ? Attr_Data : a; n = n->nextNode(); @@ -787,7 +790,7 @@ void ArrayData::sort(ExecutionEngine *engine, Object *thisObject, const Value &c while (n != sparse->sparse()->end()) { if (n->value >= len) break; - d->data(i) = sparse->arrayData()[n->value]; + d->setData(engine, i, sparse->arrayData()[n->value]); n = n->nextNode(); ++i; } @@ -800,7 +803,7 @@ void ArrayData::sort(ExecutionEngine *engine, Object *thisObject, const Value &c thisObject->initSparseArray(); while (n != sparse->sparse()->end()) { PropertyAttributes a = sparse->attrs() ? sparse->attrs()[n->value] : Attr_Data; - thisObject->arraySet(n->value, reinterpret_cast(sparse->arrayData() + n->value), a); + thisObject->arraySet(n->value, reinterpret_cast(sparse->arrayData() + n->value), a); n = n->nextNode(); } @@ -818,8 +821,8 @@ void ArrayData::sort(ExecutionEngine *engine, Object *thisObject, const Value &c if (!d->data(len).isEmpty()) break; Q_ASSERT(!d->attrs || !d->attrs[len].isAccessor()); - d->data(i) = d->data(len); - d->data(len) = Primitive::emptyValue(); + d->setData(engine, i, d->data(len)); + d->setData(engine, len, Primitive::emptyValue()); } } @@ -830,7 +833,7 @@ void ArrayData::sort(ExecutionEngine *engine, Object *thisObject, const Value &c ArrayElementLessThan lessThan(engine, thisObject, comparefn); - Value *begin = thisObject->arrayData()->values.v; + Value *begin = thisObject->arrayData()->values.values; sortHelper(begin, begin + len, *begin, lessThan); #ifdef CHECK_SPARSE_ARRAYS diff --git a/src/qml/jsruntime/qv4arraydata_p.h b/src/qml/jsruntime/qv4arraydata_p.h index 65cf69f6cd..f7f007d128 100644 --- a/src/qml/jsruntime/qv4arraydata_p.h +++ b/src/qml/jsruntime/qv4arraydata_p.h @@ -96,7 +96,7 @@ namespace Heap { Member(class, NoMark, PropertyAttributes *, attrs) \ Member(class, NoMark, ReturnedValue, freeList) \ Member(class, NoMark, SparseArray *, sparse) \ - Member(class, ValueArray, ValueArray, values) + Member(class, ValueArray, HeapValueArray, values) DECLARE_HEAP_OBJECT(ArrayData, Base) { DECLARE_MARK_TABLE(ArrayData); @@ -135,14 +135,20 @@ DECLARE_HEAP_OBJECT(ArrayData, Base) { return vtable()->length(this); } + void setArrayData(ExecutionEngine *e, uint index, Value newVal) { + values.set(e, index, newVal); + } + uint mappedIndex(uint index) const; }; V4_ASSERT_IS_TRIVIAL(ArrayData) struct SimpleArrayData : public ArrayData { uint mappedIndex(uint index) const { return (index + offset) % values.alloc; } - Value data(uint index) const { return values[mappedIndex(index)]; } - Value &data(uint index) { return values[mappedIndex(index)]; } + const Value &data(uint index) const { return values[mappedIndex(index)]; } + void setData(ExecutionEngine *e, uint index, Value newVal) { + values.set(e, mappedIndex(index), newVal); + } PropertyAttributes attributes(uint i) const { return attrs ? attrs[i] : Attr_Data; @@ -190,8 +196,10 @@ struct Q_QML_EXPORT ArrayData : public Managed void setType(Type t) { d()->type = t; } PropertyAttributes *attrs() const { return d()->attrs; } void setAttrs(PropertyAttributes *a) { d()->attrs = a; } - const Value *arrayData() const { return d()->values.v; } - Value *arrayData() { return d()->values.v; } + const Value *arrayData() const { return d()->values.data(); } + void setArrayData(ExecutionEngine *e, uint index, Value newVal) { + d()->setArrayData(e, index, newVal); + } const ArrayVTable *vtable() const { return d()->vtable(); } bool isSparse() const { return type() == Heap::ArrayData::Sparse; } @@ -229,7 +237,6 @@ struct Q_QML_EXPORT SimpleArrayData : public ArrayData uint mappedIndex(uint index) const { return d()->mappedIndex(index); } Value data(uint index) const { return d()->data(index); } - Value &data(uint index) { return d()->data(index); } uint &len() { return d()->values.size; } uint len() const { return d()->values.size; } diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp index f38580e5f8..02cd19008a 100644 --- a/src/qml/jsruntime/qv4engine.cpp +++ b/src/qml/jsruntime/qv4engine.cpp @@ -604,7 +604,9 @@ Heap::ArrayObject *ExecutionEngine::newArrayObject(const Value *values, int leng d->offset = 0; d->values.alloc = length; d->values.size = length; - memcpy(&d->values.v, values, length*sizeof(Value)); + // this doesn't require a write barrier, things will be ok, when the new array data gets inserted into + // the parent object + memcpy(&d->values.values, values, length*sizeof(Value)); a->d()->arrayData.set(this, d); a->setArrayLengthUnchecked(length); } diff --git a/src/qml/jsruntime/qv4lookup.cpp b/src/qml/jsruntime/qv4lookup.cpp index f322d5a001..9fc11a2d2d 100644 --- a/src/qml/jsruntime/qv4lookup.cpp +++ b/src/qml/jsruntime/qv4lookup.cpp @@ -218,7 +218,7 @@ void Lookup::indexedSetterFallback(Lookup *, ExecutionEngine *engine, const Valu if (o->d()->arrayData && o->d()->arrayData->type == Heap::ArrayData::Simple) { Heap::SimpleArrayData *s = o->d()->arrayData.cast(); if (idx < s->values.size) { - s->data(idx) = value; + s->setData(engine, idx, value); return; } } @@ -240,7 +240,7 @@ void Lookup::indexedSetterObjectInt(Lookup *l, ExecutionEngine *engine, const Va if (o->arrayData && o->arrayData->type == Heap::ArrayData::Simple) { Heap::SimpleArrayData *s = o->arrayData.cast(); if (idx < s->values.size) { - s->data(idx) = v; + s->setData(engine, idx, v); return; } } diff --git a/src/qml/jsruntime/qv4memberdata.cpp b/src/qml/jsruntime/qv4memberdata.cpp index ce1c8b614e..8f862d63e9 100644 --- a/src/qml/jsruntime/qv4memberdata.cpp +++ b/src/qml/jsruntime/qv4memberdata.cpp @@ -53,6 +53,7 @@ Heap::MemberData *MemberData::allocate(ExecutionEngine *e, uint n, Heap::MemberD size_t alloc = MemoryManager::align(sizeof(Heap::MemberData) + (n - 1)*sizeof(Value)); Heap::MemberData *m = e->memoryManager->allocManaged(alloc); if (old) + // no write barrier required here memcpy(m, old, sizeof(Heap::MemberData) + (old->values.size - 1) * sizeof(Value)); else m->init(); diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp index ce8fdc6d6d..d400c2ae64 100644 --- a/src/qml/jsruntime/qv4object.cpp +++ b/src/qml/jsruntime/qv4object.cpp @@ -581,7 +581,7 @@ void Object::advanceIterator(Managed *m, ObjectIterator *it, Value *name, uint * int k = it->arrayNode->key(); uint pidx = it->arrayNode->value; Heap::SparseArrayData *sa = o->d()->arrayData.cast(); - Property *p = reinterpret_cast(sa->values.v + pidx); + const Property *p = reinterpret_cast(sa->values.data() + pidx); it->arrayNode = it->arrayNode->nextNode(); PropertyAttributes a = sa->attrs ? sa->attrs[pidx] : Attr_Data; if (!(it->flags & ObjectIterator::EnumerableOnly) || a.isEnumerable()) { @@ -598,7 +598,7 @@ void Object::advanceIterator(Managed *m, ObjectIterator *it, Value *name, uint * // dense arrays while (it->arrayIndex < o->d()->arrayData->values.size) { Heap::SimpleArrayData *sa = o->d()->arrayData.cast(); - Value &val = sa->data(it->arrayIndex); + const Value &val = sa->data(it->arrayIndex); PropertyAttributes a = o->arrayData()->attributes(it->arrayIndex); ++it->arrayIndex; if (!val.isEmpty() @@ -1146,7 +1146,8 @@ void Object::copyArrayData(Object *other) dd->values.size = other->d()->arrayData->values.size; dd->offset = other->d()->arrayData->offset; } - memcpy(d()->arrayData->values.v, other->d()->arrayData->values.v, other->d()->arrayData->values.alloc*sizeof(Value)); + // ### need a write barrier + memcpy(d()->arrayData->values.values, other->d()->arrayData->values.values, other->d()->arrayData->values.alloc*sizeof(Value)); } setArrayLengthUnchecked(other->getLength()); } diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp index 7708b81d8a..2e833eda7b 100644 --- a/src/qml/jsruntime/qv4runtime.cpp +++ b/src/qml/jsruntime/qv4runtime.cpp @@ -644,7 +644,7 @@ void Runtime::method_setElement(ExecutionEngine *engine, const Value &object, co if (o->arrayType() == Heap::ArrayData::Simple) { Heap::SimpleArrayData *s = static_cast(o->arrayData()); if (s && idx < s->values.size && !s->data(idx).isEmpty()) { - s->data(idx) = value; + s->setData(engine, idx, value); return; } } -- cgit v1.2.3