Return a Heap object from the getter()/setter() methods of Property

We actually need to put the returned value into a ScopedFunctionObject
before calling it, as the Property could get deleted during the call
leading to a dangling pointer. With a GC that moves objects this will
become even more important.

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

5 files changed, 31 insertions, 27 deletions

diff --git a/src/qml/jsruntime/qv4argumentsobject.cpp b/src/qml/jsruntime/qv4argumentsobject.cpp
index f7493353ff..d9e6caf5ab 100644
--- a/src/qml/jsruntime/qv4argumentsobject.cpp
+++ b/src/qml/jsruntime/qv4argumentsobject.cpp
@@ -109,7 +109,7 @@ bool ArgumentsObject::defineOwnProperty(ExecutionEngine *engine, uint index, con
     uint numAccessors = qMin((int)context()->function->formalParameterCount(), context()->realArgumentCount);
     if (pd && index < (uint)numAccessors)
         isMapped = arrayData()->attributes(index).isAccessor() &&
-                pd->getter()->d() == context()->engine->argumentsAccessors[index].getter()->d();
+                pd->getter() == context()->engine->argumentsAccessors[index].getter();
 
     if (isMapped) {
         Q_ASSERT(arrayData());
@@ -127,10 +127,11 @@ bool ArgumentsObject::defineOwnProperty(ExecutionEngine *engine, uint index, con
 
     if (isMapped && attrs.isData()) {
         Q_ASSERT(arrayData());
+        ScopedFunctionObject setter(scope, map.setter());
         ScopedCallData callData(scope, 1);
         callData->thisObject = this->asReturnedValue();
         callData->args[0] = desc.value;
-        map.setter()->call(callData);
+        setter->call(callData);
 
         if (attrs.isWritable()) {
             setArrayAttributes(index, mapAttrs);
diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index 1410d93e61..b8d1a6b45d 100644
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -886,9 +886,9 @@ void ExecutionEngine::markObjects()
 
     for (int i = 0; i < nArgumentsAccessors; ++i) {
         const Property &pd = argumentsAccessors[i];
-        if (FunctionObject *getter = pd.getter())
+        if (Heap::FunctionObject *getter = pd.getter())
             getter->mark(this);
-        if (FunctionObject *setter = pd.setter())
+        if (Heap::FunctionObject *setter = pd.setter())
             setter->mark(this);
     }
 
diff --git a/src/qml/jsruntime/qv4lookup.cpp b/src/qml/jsruntime/qv4lookup.cpp
index dead79a939..abb769b171 100644
--- a/src/qml/jsruntime/qv4lookup.cpp
+++ b/src/qml/jsruntime/qv4lookup.cpp
@@ -434,7 +434,7 @@ ReturnedValue Lookup::getterAccessor0(Lookup *l, ExecutionEngine *engine, const
         Object *o = object->objectValue();
         if (l->classList[0] == o->internalClass()) {
             Scope scope(o->engine());
-            FunctionObject *getter = o->propertyAt(l->index)->getter();
+            ScopedFunctionObject getter(scope, o->propertyAt(l->index)->getter());
             if (!getter)
                 return Encode::undefined();
 
@@ -456,7 +456,7 @@ ReturnedValue Lookup::getterAccessor1(Lookup *l, ExecutionEngine *engine, const
         if (l->classList[0] == o->internalClass &&
             l->classList[1] == o->prototype->internalClass) {
             Scope scope(o->internalClass->engine);
-            FunctionObject *getter = o->prototype->propertyAt(l->index)->getter();
+            ScopedFunctionObject getter(scope, o->prototype->propertyAt(l->index)->getter());
             if (!getter)
                 return Encode::undefined();
 
@@ -481,7 +481,7 @@ ReturnedValue Lookup::getterAccessor2(Lookup *l, ExecutionEngine *engine, const
                 o = o->prototype;
                 if (l->classList[2] == o->internalClass) {
                     Scope scope(o->internalClass->engine);
-                    FunctionObject *getter = o->propertyAt(l->index)->getter();
+                    ScopedFunctionObject getter(scope, o->propertyAt(l->index)->getter());
                     if (!getter)
                         return Encode::undefined();
 
@@ -525,7 +525,7 @@ ReturnedValue Lookup::primitiveGetterAccessor0(Lookup *l, ExecutionEngine *engin
         Object *o = l->proto;
         if (l->classList[0] == o->internalClass()) {
             Scope scope(o->engine());
-            FunctionObject *getter = o->propertyAt(l->index)->getter();
+            ScopedFunctionObject getter(scope, o->propertyAt(l->index)->getter());
             if (!getter)
                 return Encode::undefined();
 
@@ -545,7 +545,7 @@ ReturnedValue Lookup::primitiveGetterAccessor1(Lookup *l, ExecutionEngine *engin
         if (l->classList[0] == o->internalClass() &&
             l->classList[1] == o->prototype()->internalClass) {
             Scope scope(o->engine());
-            FunctionObject *getter = o->prototype()->propertyAt(l->index)->getter();
+            ScopedFunctionObject getter(scope, o->prototype()->propertyAt(l->index)->getter());
             if (!getter)
                 return Encode::undefined();
 
@@ -648,7 +648,7 @@ ReturnedValue Lookup::globalGetterAccessor0(Lookup *l, ExecutionEngine *engine)
     Object *o = engine->globalObject();
     if (l->classList[0] == o->internalClass()) {
         Scope scope(o->engine());
-        FunctionObject *getter = o->propertyAt(l->index)->getter();
+        ScopedFunctionObject getter(scope, o->propertyAt(l->index)->getter());
         if (!getter)
             return Encode::undefined();
 
@@ -666,7 +666,7 @@ ReturnedValue Lookup::globalGetterAccessor1(Lookup *l, ExecutionEngine *engine)
     if (l->classList[0] == o->internalClass() &&
         l->classList[1] == o->prototype()->internalClass) {
         Scope scope(o->engine());
-        FunctionObject *getter = o->prototype()->propertyAt(l->index)->getter();
+        ScopedFunctionObject getter(scope, o->prototype()->propertyAt(l->index)->getter());
         if (!getter)
             return Encode::undefined();
 
@@ -687,7 +687,7 @@ ReturnedValue Lookup::globalGetterAccessor2(Lookup *l, ExecutionEngine *engine)
             o = o->prototype;
             if (l->classList[2] == o->internalClass) {
                 Scope scope(o->internalClass->engine);
-                FunctionObject *getter = o->propertyAt(l->index)->getter();
+                ScopedFunctionObject getter(scope, o->propertyAt(l->index)->getter());
                 if (!getter)
                     return Encode::undefined();
 
diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp
index d129175ed8..cb2ff842d8 100644
--- a/src/qml/jsruntime/qv4object.cpp
+++ b/src/qml/jsruntime/qv4object.cpp
@@ -84,11 +84,11 @@ ReturnedValue Object::getValue(const ValueRef thisObject, const Property *p, Pro
 {
     if (!attrs.isAccessor())
         return p->value.asReturnedValue();
-    FunctionObject *getter = p->getter();
-    if (!getter)
+    if (!p->getter())
         return Encode::undefined();
 
-    Scope scope(getter->engine());
+    Scope scope(p->getter()->internalClass->engine);
+    ScopedFunctionObject getter(scope, p->getter());
     ScopedCallData callData(scope);
     callData->thisObject = *thisObject;
     return getter->call(callData);
@@ -100,12 +100,13 @@ void Object::putValue(Property *pd, PropertyAttributes attrs, const ValueRef val
         return;
 
     if (attrs.isAccessor()) {
-        if (FunctionObject *set = pd->setter()) {
-            Scope scope(set->engine());
+        if (Heap::FunctionObject *set = pd->setter()) {
+            Scope scope(set->internalClass->engine);
+            ScopedFunctionObject setter(scope, set);
             ScopedCallData callData(scope, 1);
             callData->args[0] = *value;
             callData->thisObject = this;
-            set->call(callData);
+            setter->call(callData);
             return;
         }
         goto reject;
@@ -713,10 +714,11 @@ void Object::internalPut(String *name, const ValueRef value)
         assert(pd->setter() != 0);
 
         Scope scope(engine());
+        ScopedFunctionObject setter(scope, pd->setter());
         ScopedCallData callData(scope, 1);
         callData->args[0] = *value;
         callData->thisObject = this;
-        pd->setter()->call(callData);
+        setter->call(callData);
         return;
     }
 
@@ -786,10 +788,11 @@ void Object::internalPutIndexed(uint index, const ValueRef value)
         assert(pd->setter() != 0);
 
         Scope scope(engine());
+        ScopedFunctionObject setter(scope, pd->setter());
         ScopedCallData callData(scope, 1);
         callData->args[0] = *value;
         callData->thisObject = this;
-        pd->setter()->call(callData);
+        setter->call(callData);
         return;
     }
 
diff --git a/src/qml/jsruntime/qv4property_p.h b/src/qml/jsruntime/qv4property_p.h
index ef58b4c0f8..50d3c0e351 100644
--- a/src/qml/jsruntime/qv4property_p.h
+++ b/src/qml/jsruntime/qv4property_p.h
@@ -71,10 +71,10 @@ struct Property {
     inline bool isSubset(const PropertyAttributes &attrs, const Property &other, PropertyAttributes otherAttrs) const;
     inline void merge(PropertyAttributes &attrs, const Property &other, PropertyAttributes otherAttrs);
 
-    inline FunctionObject *getter() const { return reinterpret_cast<FunctionObject *>(value.asManaged()); }
-    inline FunctionObject *setter() const { return reinterpret_cast<FunctionObject *>(set.asManaged()); }
+    inline Heap::FunctionObject *getter() const { return value.isManaged() ? reinterpret_cast<Heap::FunctionObject *>(value.heapObject()) : 0; }
+    inline Heap::FunctionObject *setter() const { return set.isManaged() ? reinterpret_cast<Heap::FunctionObject *>(set.heapObject()) : 0; }
     inline void setGetter(FunctionObject *g) { value = Primitive::fromManaged(reinterpret_cast<Managed *>(g)); }
-    inline void setSetter(FunctionObject *s) { set = Primitive::fromManaged(reinterpret_cast<Managed *>(s)); }
+    inline void setSetter(FunctionObject *s) { set = s ? Primitive::fromManaged(reinterpret_cast<Managed *>(s)) : Value::fromHeapObject(0); }
 
     void copy(const Property &other, PropertyAttributes attrs) {
         value = other.value;
@@ -82,8 +82,8 @@ struct Property {
             set = other.set;
     }
 
-    explicit Property()  { value = Encode::undefined(); set = Encode::undefined(); }
-    explicit Property(Value v) : value(v) { set = Encode::undefined(); }
+    explicit Property()  { value = Encode::undefined(); set = Value::fromHeapObject(0); }
+    explicit Property(Value v) : value(v) { set = Value::fromHeapObject(0); }
     Property(FunctionObject *getter, FunctionObject *setter) {
         value = Primitive::fromManaged(reinterpret_cast<Managed *>(getter));
         set = Primitive::fromManaged(reinterpret_cast<Managed *>(setter));
@@ -111,9 +111,9 @@ inline bool Property::isSubset(const PropertyAttributes &attrs, const Property &
     if (attrs.type() == PropertyAttributes::Data && !value.sameValue(other.value))
         return false;
     if (attrs.type() == PropertyAttributes::Accessor) {
-        if (value.asManaged() != other.value.asManaged())
+        if (value.heapObject() != other.value.heapObject())
             return false;
-        if (set.asManaged() != other.set.asManaged())
+        if (set.heapObject() != other.set.heapObject())
             return false;
     }
     return true;