Fix calling Qt.binding() on bound functions

Calling Qt.binding() on a bound function object is a valid use
case and used to work until Qt 5.8.

The problem was that we optimized the code in QQmlBinding and
QQmlJavascriptExpression to directly work on a QV4::Function,
so this wouldn't work anymore.

To fix this make sure recursive calls to Function.bind() are
unrolled (so that the BoundFunction's target is never a bound
function itself), then add the bound function as an optional
member to the QQmlBinding and use it's bound arguments if
present.

Task-number: QTBUG-61927
Change-Id: I472214ddd82fc2a1212efd9b769861fc43d2ddaf
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

4 files changed, 41 insertions, 17 deletions

diff --git a/src/qml/jsruntime/qv4functionobject.cpp b/src/qml/jsruntime/qv4functionobject.cpp
index a8c1640767..dc8ee550d5 100644
--- a/src/qml/jsruntime/qv4functionobject.cpp
+++ b/src/qml/jsruntime/qv4functionobject.cpp
@@ -75,7 +75,6 @@ void Heap::FunctionObject::init(QV4::ExecutionContext *scope, QV4::String *name,
     jsConstruct = QV4::FunctionObject::callAsConstructor;
 
     Object::init();
-    function = nullptr;
     this->scope.set(scope->engine(), scope->d());
     Scope s(scope->engine());
     ScopedFunctionObject f(s, this);
@@ -88,7 +87,6 @@ void Heap::FunctionObject::init(QV4::ExecutionContext *scope, QV4::String *name,
     jsConstruct = reinterpret_cast<const ObjectVTable *>(vtable())->callAsConstructor;
 
     Object::init();
-    function = nullptr;
     this->scope.set(scope->engine(), scope->d());
     Scope s(scope->engine());
     ScopedFunctionObject f(s, this);
@@ -101,8 +99,7 @@ void Heap::FunctionObject::init(QV4::ExecutionContext *scope, Function *function
     jsConstruct = reinterpret_cast<const ObjectVTable *>(vtable())->callAsConstructor;
 
     Object::init();
-    this->function = function;
-    function->compilationUnit->addref();
+    setFunction(function);
     this->scope.set(scope->engine(), scope->d());
     Scope s(scope->engine());
     ScopedString name(s, function->name());
@@ -123,13 +120,18 @@ void Heap::FunctionObject::init()
     jsConstruct = reinterpret_cast<const ObjectVTable *>(vtable())->callAsConstructor;
 
     Object::init();
-    function = nullptr;
     this->scope.set(internalClass->engine, internalClass->engine->rootContext()->d());
     Q_ASSERT(internalClass && internalClass->find(internalClass->engine->id_prototype()) == Index_Prototype);
     setProperty(internalClass->engine, Index_Prototype, Primitive::undefinedValue());
 }
 
-
+void Heap::FunctionObject::setFunction(Function *f)
+{
+    if (f) {
+        function = f;
+        function->compilationUnit->addref();
+    }
+}
 void Heap::FunctionObject::destroy()
 {
     if (function)
@@ -347,20 +349,36 @@ ReturnedValue FunctionPrototype::method_bind(const FunctionObject *b, const Valu
 {
     QV4::Scope scope(b);
     ScopedFunctionObject target(scope, thisObject);
-    if (!target)
+    if (!target || target->isBinding())
         return scope.engine->throwTypeError();
 
     ScopedValue boundThis(scope, argc ? argv[0] : Primitive::undefinedValue());
     Scoped<MemberData> boundArgs(scope, (Heap::MemberData *)nullptr);
-    if (argc > 1) {
-        boundArgs = MemberData::allocate(scope.engine, argc - 1);
-        boundArgs->d()->values.size = argc - 1;
-        for (uint i = 0, ei = static_cast<uint>(argc - 1); i < ei; ++i)
+
+    int nArgs = (argc - 1 >= 0) ? argc - 1 : 0;
+    if (target->isBoundFunction()) {
+        BoundFunction *bound = static_cast<BoundFunction *>(target.getPointer());
+        Scoped<MemberData> oldArgs(scope, bound->boundArgs());
+        boundThis = bound->boundThis();
+        int oldSize = oldArgs->size();
+        boundArgs = MemberData::allocate(scope.engine, oldSize + nArgs);
+        boundArgs->d()->values.size = oldSize + nArgs;
+        for (uint i = 0; i < static_cast<uint>(oldSize); ++i)
+            boundArgs->set(scope.engine, i, oldArgs->data()[i]);
+        for (uint i = 0; i < static_cast<uint>(nArgs); ++i)
+            boundArgs->set(scope.engine, oldSize + i, argv[i + 1]);
+        target = bound->target();
+    } else if (nArgs) {
+        boundArgs = MemberData::allocate(scope.engine, nArgs);
+        boundArgs->d()->values.size = nArgs;
+        for (uint i = 0, ei = static_cast<uint>(nArgs); i < ei; ++i)
             boundArgs->set(scope.engine, i, argv[i + 1]);
     }
 
-    ExecutionContext *global = scope.engine->rootContext();
-    return BoundFunction::create(global, target, boundThis, boundArgs)->asReturnedValue();
+    ScopedContext ctx(scope, target->scope());
+    Heap::BoundFunction *bound = BoundFunction::create(ctx, target, boundThis, boundArgs);
+    bound->setFunction(target->function());
+    return bound->asReturnedValue();
 }
 
 DEFINE_OBJECT_VTABLE(ScriptFunction);
@@ -393,8 +411,7 @@ void Heap::ScriptFunction::init(QV4::ExecutionContext *scope, Function *function
     FunctionObject::init();
     this->scope.set(scope->engine(), scope->d());
 
-    this->function = function;
-    function->compilationUnit->addref();
+    setFunction(function);
     Q_ASSERT(function);
     Q_ASSERT(function->code);
 
diff --git a/src/qml/jsruntime/qv4functionobject_p.h b/src/qml/jsruntime/qv4functionobject_p.h
index d6066ec648..32e71a175b 100644
--- a/src/qml/jsruntime/qv4functionobject_p.h
+++ b/src/qml/jsruntime/qv4functionobject_p.h
@@ -90,6 +90,8 @@ DECLARE_HEAP_OBJECT(FunctionObject, Object) {
     void init();
     void destroy();
 
+    void setFunction(Function *f);
+
     unsigned int formalParameterCount() { return function ? function->nFormals : 0; }
     unsigned int varCount() { return function ? function->compiledFunction->nLocals : 0; }
 
diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index c1bbe2a330..816c259b9b 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -460,9 +460,12 @@ void QObjectWrapper::setProperty(ExecutionEngine *engine, QObject *object, QQmlP
 
             QV4::Scoped<QQmlBindingFunction> bindingFunction(scope, (const Value &)f);
 
+            QV4::ScopedFunctionObject f(scope, bindingFunction->bindingFunction());
             QV4::ScopedContext ctx(scope, bindingFunction->scope());
-            newBinding = QQmlBinding::create(property, bindingFunction->function(), object, callingQmlContext, ctx);
+            newBinding = QQmlBinding::create(property, f->function(), object, callingQmlContext, ctx);
             newBinding->setSourceLocation(bindingFunction->currentLocation());
+            if (f->isBoundFunction())
+                newBinding->setBoundFunction(static_cast<QV4::BoundFunction *>(f.getPointer()));
             newBinding->setTarget(object, *property, nullptr);
         }
     }
diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp
index 2506777e76..0211ad1011 100644
--- a/src/qml/jsruntime/qv4runtime.cpp
+++ b/src/qml/jsruntime/qv4runtime.cpp
@@ -1271,7 +1271,9 @@ QV4::ReturnedValue Runtime::method_createUnmappedArgumentsObject(ExecutionEngine
 
 ReturnedValue Runtime::method_loadQmlContext(NoThrowEngine *engine)
 {
-    return engine->qmlContext()->asReturnedValue();
+    Heap::QmlContext *ctx = engine->qmlContext();
+    Q_ASSERT(ctx);
+    return ctx->asReturnedValue();
 }
 
 ReturnedValue Runtime::method_regexpLiteral(ExecutionEngine *engine, int id)