Fix copying of Property's

Data properties don't contain valid data in the set field
if they are being stored in Objects. Thus we should never
access that field unless we are dealing with accessor
properties.

Change-Id: I19dcbaee7ebd042ae24387f92a93571d75ca578a
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

diff --git a/src/qml/jsapi/qjsvalueiterator.cpp b/src/qml/jsapi/qjsvalueiterator.cpp
index e3358b2..9e02310 100644 (file)
--- a/src/qml/jsapi/qjsvalueiterator.cpp
+++ b/src/qml/jsapi/qjsvalueiterator.cpp
@@ -150,7 +150,7 @@ bool QJSValueIterator::next()
         return false;
     d_ptr->currentName = d_ptr->nextName;
     d_ptr->currentIndex = d_ptr->nextIndex;
-    d_ptr->currentProperty = d_ptr->nextProperty;
+    d_ptr->currentProperty.copy(d_ptr->nextProperty, d_ptr->nextAttributes);
     d_ptr->currentAttributes = d_ptr->nextAttributes;
 
     QV4::ExecutionEngine *v4 = d_ptr->iterator.engine();

diff --git a/src/qml/jsruntime/qv4argumentsobject.cpp b/src/qml/jsruntime/qv4argumentsobject.cpp
index 3a0aad2..c8ba2c4 100644 (file)
--- a/src/qml/jsruntime/qv4argumentsobject.cpp
+++ b/src/qml/jsruntime/qv4argumentsobject.cpp
@@ -58,11 +58,12 @@ ArgumentsObject::ArgumentsObject(CallContext *context)
     setArrayType(ArrayData::Complex);
 
     if (context->strictMode) {
-        Property pd = Property::fromAccessor(v4->thrower, v4->thrower);
         Q_ASSERT(CalleePropertyIndex == internalClass->find(context->engine->id_callee));
         Q_ASSERT(CallerPropertyIndex == internalClass->find(context->engine->id_caller));
-        *propertyAt(CalleePropertyIndex) = pd;
-        *propertyAt(CallerPropertyIndex) = pd;
+        propertyAt(CalleePropertyIndex)->value = v4->thrower;
+        propertyAt(CalleePropertyIndex)->set = v4->thrower;
+        propertyAt(CallerPropertyIndex)->value = v4->thrower;
+        propertyAt(CallerPropertyIndex)->set = v4->thrower;
 
         arrayReserve(context->callData->argc);
         arrayPut(0, context->callData->args, context->callData->argc);
@@ -94,7 +95,7 @@ void ArgumentsObject::fullyCreate()
     context->engine->requireArgumentsAccessors(numAccessors);
     for (uint i = 0; i < (uint)numAccessors; ++i) {
         mappedArguments.append(context->callData->args[i]);
-        arraySet(i, context->engine->argumentsAccessors.at(i), Attr_Accessor);
+        arraySet(i, context->engine->argumentsAccessors[i], Attr_Accessor);
     }
     arrayPut(numAccessors, context->callData->args + numAccessors, argCount - numAccessors);
     for (uint i = numAccessors; i < argCount; ++i)
@@ -113,11 +114,11 @@ bool ArgumentsObject::defineOwnProperty(ExecutionContext *ctx, uint index, const
     PropertyAttributes mapAttrs;
     bool isMapped = false;
     if (pd && index < (uint)mappedArguments.size())
-        isMapped = arrayData->attributes(index).isAccessor() && pd->getter() == context->engine->argumentsAccessors.at(index).getter();
+        isMapped = arrayData->attributes(index).isAccessor() && pd->getter() == context->engine->argumentsAccessors[index].getter();
 
     if (isMapped) {
-        map = *pd;
         mapAttrs = arrayData->attributes(index);
+        map.copy(*pd, mapAttrs);
         setArrayAttributes(index, Attr_Data);
         pd = arrayData->getProperty(index);
         pd->value = mappedArguments.at(index);
@@ -137,7 +138,7 @@ bool ArgumentsObject::defineOwnProperty(ExecutionContext *ctx, uint index, const
         if (attrs.isWritable()) {
             setArrayAttributes(index, mapAttrs);
             pd = arrayData->getProperty(index);
-            *pd = map;
+            pd->copy(map, mapAttrs);
         }
     }
 

diff --git a/src/qml/jsruntime/qv4arraydata.cpp b/src/qml/jsruntime/qv4arraydata.cpp
index 44727cb..f03f5cd 100644 (file)
--- a/src/qml/jsruntime/qv4arraydata.cpp
+++ b/src/qml/jsruntime/qv4arraydata.cpp
@@ -595,7 +595,7 @@ uint ArrayData::append(Object *obj, const ArrayObject *otherObj, uint n)
         } else {
             for (const SparseArrayNode *it = static_cast<const SparseArrayData *>(other)->sparse->begin();
                  it != static_cast<const SparseArrayData *>(other)->sparse->end(); it = it->nextNode())
-                obj->arraySet(oldSize + it->key(), other->data[it->value]);
+                obj->arraySet(oldSize + it->key(), ValueRef(other->data[it->value]));
         }
     } else {
         obj->arrayPut(oldSize, other->data, n);

diff --git a/src/qml/jsruntime/qv4arrayobject.cpp b/src/qml/jsruntime/qv4arrayobject.cpp
index efe1dff..3d710d2 100644 (file)
--- a/src/qml/jsruntime/qv4arrayobject.cpp
+++ b/src/qml/jsruntime/qv4arrayobject.cpp
@@ -179,7 +179,7 @@ ReturnedValue ArrayPrototype::method_concat(CallContext *ctx)
                 result->putIndexed(startIndex + i, entry);
             }
         } else {
-            result->arraySet(result->getLength(), ctx->callData->args[i]);
+            result->arraySet(result->getLength(), ValueRef(ctx->callData->args[i]));
         }
     }
 

diff --git a/src/qml/jsruntime/qv4context.cpp b/src/qml/jsruntime/qv4context.cpp
index fa5dc72..0d8e57b 100644 (file)
--- a/src/qml/jsruntime/qv4context.cpp
+++ b/src/qml/jsruntime/qv4context.cpp
@@ -127,7 +127,7 @@ void ExecutionContext::createMutableBinding(const StringRef name, bool deletable
 
     if (activation->hasProperty(name))
         return;
-    Property desc = Property::fromValue(Primitive::undefinedValue());
+    Property desc(Primitive::undefinedValue());
     PropertyAttributes attrs(Attr_Data);
     attrs.setConfigurable(deletable);
     activation->__defineOwnProperty__(this, name, desc, attrs);

diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index 1807513..5c7caca 100644 (file)
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -171,6 +171,8 @@ ExecutionEngine::ExecutionEngine(EvalISelFactory *factory)
     , globalObject(0)
     , globalCode(0)
     , v8Engine(0)
+    , argumentsAccessors(0)
+    , nArgumentsAccessors(0)
     , m_engineId(engineSerial.fetchAndAddOrdered(1))
     , regExpCache(0)
     , m_multiplyWrappedQObjects(0)
@@ -423,6 +425,7 @@ ExecutionEngine::~ExecutionEngine()
     delete executableAllocator;
     jsStack->deallocate();
     delete jsStack;
+    delete [] argumentsAccessors;
 }
 
 void ExecutionEngine::enableDebugger()
@@ -784,20 +787,26 @@ QUrl ExecutionEngine::resolvedUrl(const QString &file)
 
 void ExecutionEngine::requireArgumentsAccessors(int n)
 {
-    if (n <= argumentsAccessors.size())
+    if (n <= nArgumentsAccessors)
         return;
 
     Scope scope(this);
     ScopedFunctionObject get(scope);
     ScopedFunctionObject set(scope);
 
-    uint oldSize = argumentsAccessors.size();
-    argumentsAccessors.resize(n);
-    for (int i = oldSize; i < n; ++i) {
-        get = new (memoryManager) ArgumentsGetterFunction(rootContext, i);
-        set = new (memoryManager) ArgumentsSetterFunction(rootContext, i);
-        Property pd = Property::fromAccessor(get.getPointer(), set.getPointer());
-        argumentsAccessors[i] = pd;
+    if (n >= nArgumentsAccessors) {
+        Property *oldAccessors = argumentsAccessors;
+        int oldSize = nArgumentsAccessors;
+        nArgumentsAccessors = qMax(8, n);
+        argumentsAccessors = new Property[nArgumentsAccessors];
+        if (oldAccessors) {
+            memcpy(argumentsAccessors, oldAccessors, oldSize*sizeof(Property));
+            delete [] oldAccessors;
+        }
+        for (int i = oldSize; i < nArgumentsAccessors; ++i) {
+            argumentsAccessors[i].value = Value::fromManaged(new (memoryManager) ArgumentsGetterFunction(rootContext, i));
+            argumentsAccessors[i].set = Value::fromManaged(new (memoryManager) ArgumentsSetterFunction(rootContext, i));
+        }
     }
 }
 
@@ -807,8 +816,8 @@ void ExecutionEngine::markObjects()
 
     globalObject->mark(this);
 
-    for (int i = 0; i < argumentsAccessors.size(); ++i) {
-        const Property &pd = argumentsAccessors.at(i);
+    for (int i = 0; i < nArgumentsAccessors; ++i) {
+        const Property &pd = argumentsAccessors[i];
         if (FunctionObject *getter = pd.getter())
             getter->mark(this);
         if (FunctionObject *setter = pd.setter())

diff --git a/src/qml/jsruntime/qv4engine_p.h b/src/qml/jsruntime/qv4engine_p.h
index 14358c1..6bfdf1f 100644 (file)
--- a/src/qml/jsruntime/qv4engine_p.h
+++ b/src/qml/jsruntime/qv4engine_p.h
@@ -231,7 +231,8 @@ public:
     EvalFunction *evalFunction;
     FunctionObject *thrower;
 
-    QVector<Property> argumentsAccessors;
+    Property *argumentsAccessors;
+    int nArgumentsAccessors;
 
     StringValue id_undefined;
     StringValue id_null;

diff --git a/src/qml/jsruntime/qv4functionobject.cpp b/src/qml/jsruntime/qv4functionobject.cpp
index 9b502a0..c53c528 100644 (file)
--- a/src/qml/jsruntime/qv4functionobject.cpp
+++ b/src/qml/jsruntime/qv4functionobject.cpp
@@ -374,7 +374,7 @@ ScriptFunction::ScriptFunction(ExecutionContext *scope, Function *function)
     defineReadonlyProperty(scope->engine->id_length, Primitive::fromInt32(formalParameterCount()));
 
     if (scope->strictMode) {
-        Property pd = Property::fromAccessor(v4->thrower, v4->thrower);
+        Property pd(v4->thrower, v4->thrower);
         insertMember(scope->engine->id_caller, pd, Attr_Accessor|Attr_NotConfigurable|Attr_NotEnumerable);
         insertMember(scope->engine->id_arguments, pd, Attr_Accessor|Attr_NotConfigurable|Attr_NotEnumerable);
     }
@@ -457,7 +457,7 @@ SimpleScriptFunction::SimpleScriptFunction(ExecutionContext *scope, Function *fu
     defineReadonlyProperty(scope->engine->id_length, Primitive::fromInt32(formalParameterCount()));
 
     if (scope->strictMode) {
-        Property pd = Property::fromAccessor(v4->thrower, v4->thrower);
+        Property pd(v4->thrower, v4->thrower);
         insertMember(scope->engine->id_caller, pd, Attr_Accessor|Attr_NotConfigurable|Attr_NotEnumerable);
         insertMember(scope->engine->id_arguments, pd, Attr_Accessor|Attr_NotConfigurable|Attr_NotEnumerable);
     }
@@ -631,7 +631,7 @@ BoundFunction::BoundFunction(ExecutionContext *scope, FunctionObjectRef target,
 
     ExecutionEngine *v4 = scope->engine;
 
-    Property pd = Property::fromAccessor(v4->thrower, v4->thrower);
+    Property pd(v4->thrower, v4->thrower);
     insertMember(scope->engine->id_arguments, pd, Attr_Accessor|Attr_NotConfigurable|Attr_NotEnumerable);
     insertMember(scope->engine->id_caller, pd, Attr_Accessor|Attr_NotConfigurable|Attr_NotEnumerable);
 }

diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp
index 7ad3189..f5c1be7 100644 (file)
--- a/src/qml/jsruntime/qv4object.cpp
+++ b/src/qml/jsruntime/qv4object.cpp
@@ -252,7 +252,9 @@ void Object::insertMember(const StringRef s, const Property &p, PropertyAttribut
 
     if (attributes.isAccessor()) {
         hasAccessorProperty = 1;
-        *propertyAt(idx) = p;
+        Property *pp = propertyAt(idx);
+        pp->value = p.value;
+        pp->set = p.set;
     } else {
         memberData[idx] = p.value;
     }
@@ -568,7 +570,7 @@ void Object::advanceIterator(Managed *m, ObjectIterator *it, StringRef name, uin
                     it->arrayIndex = k + 1;
                     *index = k;
                     *attrs = a;
-                    *pd = *p;
+                    pd->copy(*p, a);
                     return;
                 }
             }
@@ -604,7 +606,7 @@ void Object::advanceIterator(Managed *m, ObjectIterator *it, StringRef name, uin
         if (!(it->flags & ObjectIterator::EnumerableOnly) || a.isEnumerable()) {
             name = n;
             *attrs = a;
-            *pd = *p;
+            pd->copy(*p, a);
             return;
         }
     }
@@ -916,7 +918,8 @@ bool Object::__defineOwnProperty__(ExecutionContext *ctx, const StringRef name,
         if (!extensible)
             goto reject;
         // clause 4
-        Property pd = p;
+        Property pd;
+        pd.copy(p, attrs);
         pd.fullyPopulated(&attrs);
         insertMember(name, pd, attrs);
         return true;
@@ -961,7 +964,8 @@ bool Object::defineOwnProperty2(ExecutionContext *ctx, uint index, const Propert
         if (!extensible)
             goto reject;
         // clause 4
-        Property pp(p);
+        Property pp;
+        pp.copy(p, attrs);
         pp.fullyPopulated(&attrs);
         if (attrs == Attr_Data) {
             Scope scope(ctx);

diff --git a/src/qml/jsruntime/qv4object_p.h b/src/qml/jsruntime/qv4object_p.h
index 5c8590f..89dbde5 100644 (file)
--- a/src/qml/jsruntime/qv4object_p.h
+++ b/src/qml/jsruntime/qv4object_p.h
@@ -173,7 +173,7 @@ struct Q_QML_EXPORT Object: Managed {
     void defineReadonlyProperty(const StringRef name, ValueRef value);
 
     void insertMember(const StringRef s, const ValueRef v, PropertyAttributes attributes = Attr_Data) {
-        insertMember(s, Property::fromValue(*v), attributes);
+        insertMember(s, Property(*v), attributes);
     }
     void insertMember(const StringRef s, const Property &p, PropertyAttributes attributes);
 

diff --git a/src/qml/jsruntime/qv4objectproto.cpp b/src/qml/jsruntime/qv4objectproto.cpp
index 51d6b8d..8f55f3e 100644 (file)
--- a/src/qml/jsruntime/qv4objectproto.cpp
+++ b/src/qml/jsruntime/qv4objectproto.cpp
@@ -134,8 +134,8 @@ void ObjectPrototype::init(ExecutionEngine *v4, ObjectRef ctor)
     defineDefaultProperty(QStringLiteral("__defineSetter__"), method_defineSetter, 2);
 
     Scoped<String> id_proto(scope, v4->id___proto__);
-    Property p = Property::fromAccessor(v4->newBuiltinFunction(v4->rootContext, id_proto, method_get_proto)->getPointer(),
-                                        v4->newBuiltinFunction(v4->rootContext, id_proto, method_set_proto)->getPointer());
+    Property p(v4->newBuiltinFunction(v4->rootContext, id_proto, method_get_proto)->getPointer(),
+               v4->newBuiltinFunction(v4->rootContext, id_proto, method_set_proto)->getPointer());
     insertMember(StringRef(v4->id___proto__), p, Attr_Accessor|Attr_NotEnumerable);
 }
 

diff --git a/src/qml/jsruntime/qv4property_p.h b/src/qml/jsruntime/qv4property_p.h
index aceb602..4d59b6d 100644 (file)
--- a/src/qml/jsruntime/qv4property_p.h
+++ b/src/qml/jsruntime/qv4property_p.h
@@ -70,18 +70,6 @@ struct Property {
         attrs->resolve();
     }
 
-    static inline Property fromValue(Value v) {
-        Property pd;
-        pd.value = v;
-        return pd;
-    }
-    static inline Property fromAccessor(FunctionObject *getter, FunctionObject *setter) {
-        Property pd;
-        pd.value = Primitive::fromManaged(reinterpret_cast<Managed *>(getter));
-        pd.set = Primitive::fromManaged(reinterpret_cast<Managed *>(setter));
-        return pd;
-    }
-
     static Property genericDescriptor() {
         Property pd;
         pd.value = Primitive::emptyValue();
@@ -95,6 +83,23 @@ struct Property {
     inline FunctionObject *setter() const { return reinterpret_cast<FunctionObject *>(set.asManaged()); }
     inline void setGetter(FunctionObject *g) { value = Primitive::fromManaged(reinterpret_cast<Managed *>(g)); }
     inline void setSetter(FunctionObject *s) { set = Primitive::fromManaged(reinterpret_cast<Managed *>(s)); }
+
+    void copy(const Property &other, PropertyAttributes attrs) {
+        value = other.value;
+        if (attrs.isAccessor())
+            set = other.set;
+    }
+
+    explicit Property()  { value = Encode::undefined(); set = Encode::undefined(); }
+    explicit Property(Value v) : value(v) { set = Encode::undefined(); }
+    Property(FunctionObject *getter, FunctionObject *setter) {
+        value = Primitive::fromManaged(reinterpret_cast<Managed *>(getter));
+        set = Primitive::fromManaged(reinterpret_cast<Managed *>(setter));
+    }
+    Property &operator=(Value v) { value = v; return *this; }
+private:
+    Property(const Property &);
+    Property &operator=(const Property &);
 };
 
 inline bool Property::isSubset(const PropertyAttributes &attrs, const Property &other, PropertyAttributes otherAttrs) const
@@ -140,6 +145,8 @@ inline void Property::merge(PropertyAttributes &attrs, const Property &other, Pr
 
 }
 
+Q_DECLARE_TYPEINFO(QV4::Property, Q_MOVABLE_TYPE);
+
 QT_END_NAMESPACE
 
 #endif

diff --git a/src/qml/jsruntime/qv4stringobject.cpp b/src/qml/jsruntime/qv4stringobject.cpp
index 07cdf31..5b0891e 100644 (file)
--- a/src/qml/jsruntime/qv4stringobject.cpp
+++ b/src/qml/jsruntime/qv4stringobject.cpp
@@ -148,7 +148,7 @@ void StringObject::advanceIterator(Managed *m, ObjectIterator *it, StringRef nam
             Property *pd = s->__getOwnProperty__(*index, &a);
             if (!(it->flags & ObjectIterator::EnumerableOnly) || a.isEnumerable()) {
                 *attrs = a;
-                *p = *pd;
+                p->copy(*pd, a);
                 return;
             }
         }