JS bridge does not transmit QVariants anymore in Qt5 https://bugs.webkit.org/show_bug.cgi?id=104540

Patch by Simon Hausmann  <simon.hausmann@digia.com>, Jedrzej Nowacki <jedrzej.nowacki@digia.com> on 2012-12-19
Reviewed by Kenneth Rohde Christiansen.

A data corruption exists in the QObject bridge when calling slots that
take a QVariant.

The calling convention for slots is that the void* parameter array must
contain pointers to the actually required destination argument type. If
a function takes an int for example, the corresponding entry in the
void* parameter array must be a pointer to an int that the moc
generated code then can "safely" cast to an int* and dereference.
Similarly if the function takes a QVariant it must be a pointer to a
QVariant.

We implement this calling convention by constructing QVariants of the
requested parameter types and passing the value of data() into the
void* parameter array. This works fine for all types except if the
requested type is a QVariant. In that case data() will _not_ return a
pointer that can later be safely casted to a QVariant pointer and
dereferenced. Instead we must use the address of our variant to ensure
a working cast.

Our auto tests cover this case, but they worked by accident because the
provided pointer when casted to a QVariant happens to have the correct
type id that doesn't produce the warning seen in the test case of the
provided example and the unit test just copies the QVariant and thus
pointer.

* bridge/qt/qt_runtime.cpp:
(JSC::Bindings::QtMethodMatchType::typeId): Replace string based meta
type id determination of QVariant with a quicker table lookup.
(JSC::Bindings::findMethodIndex): Remember the chosen (requested) types
and pass the pointer to the QVariant instead of its data() pointer if
requested.
(JSC::Bindings::QtRuntimeMethod::call): Fixed determination of whether
we need to convert a return value or not solely based on the return
type _specified_ in the meta method instead of the variant value
returned. The latter is not sufficient because a slot can return an
invalid variant, which is not the same as returning void. This was
triggered by an unit test that accidentally passed due to this memory
corruption in the first place.

Change-Id: Ie8ed983a7b7f530c038956c9f32eef3738a3cc9d
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@138247 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@digia.com>

1 files changed, 10 insertions, 4 deletions

diff --git a/Source/WebCore/bridge/qt/qt_runtime.cpp b/Source/WebCore/bridge/qt/qt_runtime.cpp
index 951fa1cde..131239533 100644
--- a/Source/WebCore/bridge/qt/qt_runtime.cpp
+++ b/Source/WebCore/bridge/qt/qt_runtime.cpp
@@ -893,7 +893,7 @@ private:
 QMetaType::Type QtMethodMatchType::typeId() const
 {
     if (isVariant())
-        return (QMetaType::Type) QMetaType::type("QVariant");
+        return (QMetaType::Type) qMetaTypeId<QVariant>();
     return (QMetaType::Type) (isMetaEnum() ? QMetaType::Int : m_typeId);
 }
 
@@ -1086,6 +1086,7 @@ static int findMethodIndex(JSContextRef context,
                 && (matchDistance == 0)) {
                 // perfect match, use this one
                 chosenIndex = index;
+                chosenTypes = types;
                 break;
             }
             QtMethodMatchData currentMatch(matchDistance, index, types, args);
@@ -1163,6 +1164,7 @@ static int findMethodIndex(JSContextRef context,
             setException(context, exception, message);
         } else {
             chosenIndex = bestMatch.index;
+            chosenTypes = bestMatch.types;
             args = bestMatch.args;
         }
     }
@@ -1173,7 +1175,10 @@ static int findMethodIndex(JSContextRef context,
         vars.resize(args.count());
         for (i=0; i < args.count(); i++) {
             vars[i] = args[i];
-            vvars[i] = vars[i].data();
+            if (chosenTypes[i].isVariant())
+                vvars[i] = &vars[i];
+            else
+                vvars[i] = vars[i].data();
         }
     }
 
@@ -1238,14 +1243,15 @@ JSValueRef QtRuntimeMethod::call(JSContextRef context, JSObjectRef function, JSO
 
     QVarLengthArray<QVariant, 10> vargs;
     void* qargs[11];
+    const QMetaObject* metaObject = obj->metaObject();
 
-    int methodIndex = findMethodIndex(context, obj->metaObject(), d->m_identifier,  argumentCount, arguments,
+    int methodIndex = findMethodIndex(context, metaObject, d->m_identifier,  argumentCount, arguments,
                                       (d->m_flags & AllowPrivate), vargs, (void **)qargs, exception);
 
     if (QMetaObject::metacall(obj, QMetaObject::InvokeMetaMethod, methodIndex, qargs) >= 0)
         return JSValueMakeUndefined(context);
 
-    if (vargs.size() > 0 && vargs[0].isValid())
+    if (vargs.size() > 0 && metaObject->method(methodIndex).returnType() != QMetaType::Void)
         return convertQVariantToValue(context, d->m_instance->rootObject(), vargs[0], exception);
 
     return JSValueMakeUndefined(context);