QML: Encode "missing" line number as negated address of stack frame

This way we can identify which entry in a stack frame to amend when
processing an exception in generated code. However, negative line
numbers are also used to signal the position of "Ret" instructions.
Since you cannot throw an exception from a "Ret" instruction, those
cannot collide, but we cannot qAbs() the line number anymore when saving
it in the stack trace. We have to qAbs() it in all the places where it's
read.

Pick-to: 6.5
Fixes: QTBUG-112946
Change-Id: I24dc4008fb7eab38e4d24e70211c22e46f1b72a7
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.cpp

@@ -220,7 +220,7 @@ QJsonObject QV4DataCollector::buildFrame(const QV4::StackFrame &stackFrame, int
     frame[QLatin1String("debuggerFrame")] = false;
     frame[QLatin1String("func")] = stackFrame.function;
     frame[QLatin1String("script")] = stackFrame.source;
-    frame[QLatin1String("line")] = stackFrame.line - 1;
+    frame[QLatin1String("line")] = qAbs(stackFrame.line) - 1;
     if (stackFrame.column >= 0)
         frame[QLatin1String("column")] = stackFrame.column;
 

src/qml/jsapi/qjsengine.cpp

@@ -530,7 +530,7 @@ QJSValue QJSEngine::evaluate(const QString& program, const QString& fileName, in
             for (auto &&frame: trace)
                 exceptionStackTrace->push_back(QString::fromLatin1("%1:%2:%3:%4").arg(
                                           frame.function,
-                                          QString::number(frame.line),
+                                          QString::number(qAbs(frame.line)),
                                           QString::number(frame.column),
                                           frame.source)
                                       );

src/qml/jsruntime/qv4engine.cpp

@@ -1234,7 +1234,7 @@ StackTrace ExecutionEngine::stackTrace(int frameLimit) const
         QV4::StackFrame frame;
         frame.source = f->source();
         frame.function = f->function();
-        frame.line = qAbs(f->lineNumber());
+        frame.line = f->lineNumber();
         frame.column = -1;
         stack.append(frame);
         if (f->isJSTypesFrame()) {
@@ -1272,7 +1272,7 @@ static inline char *v4StackTrace(const ExecutionContext *context)
             const QString fileName = url.isLocalFile() ? url.toLocalFile() : url.toString();
             str << "frame={level=\"" << i << "\",func=\"" << stackTrace.at(i).function
                 << "\",file=\"" << fileName << "\",fullname=\"" << fileName
-                << "\",line=\"" << stackTrace.at(i).line << "\",language=\"js\"}";
+                << "\",line=\"" << qAbs(stackTrace.at(i).line) << "\",language=\"js\"}";
         }
     }
     str << ']';
@@ -1467,7 +1467,7 @@ QQmlError ExecutionEngine::catchExceptionAsQmlError()
     if (!trace.isEmpty()) {
         QV4::StackFrame frame = trace.constFirst();
         error.setUrl(QUrl(frame.source));
-        error.setLine(frame.line);
+        error.setLine(qAbs(frame.line));
         error.setColumn(frame.column);
     }
     QV4::Scoped<QV4::ErrorObject> errorObj(scope, exception);

src/qml/jsruntime/qv4errorobject.cpp

@@ -56,7 +56,7 @@ void Heap::ErrorObject::init(const Value &message, ErrorType t)
     e->d()->stackTrace = new StackTrace(scope.engine->stackTrace());
     if (!e->d()->stackTrace->isEmpty()) {
         setProperty(scope.engine, QV4::ErrorObject::Index_FileName, scope.engine->newString(e->d()->stackTrace->at(0).source));
-        setProperty(scope.engine, QV4::ErrorObject::Index_LineNumber, Value::fromInt32(e->d()->stackTrace->at(0).line));
+        setProperty(scope.engine, QV4::ErrorObject::Index_LineNumber, Value::fromInt32(qAbs(e->d()->stackTrace->at(0).line)));
     }
 
     if (!message.isUndefined())
@@ -84,7 +84,7 @@ void Heap::ErrorObject::init(const Value &message, const QString &fileName, int
 
     Q_ASSERT(!e->d()->stackTrace->isEmpty());
     setProperty(scope.engine, QV4::ErrorObject::Index_FileName, scope.engine->newString(e->d()->stackTrace->at(0).source));
-    setProperty(scope.engine, QV4::ErrorObject::Index_LineNumber, Value::fromInt32(e->d()->stackTrace->at(0).line));
+    setProperty(scope.engine, QV4::ErrorObject::Index_LineNumber, Value::fromInt32(qAbs(e->d()->stackTrace->at(0).line)));
 
     if (!message.isUndefined())
         setProperty(scope.engine, QV4::ErrorObject::Index_Message, message);

src/qml/jsruntime/qv4stackframe.cpp

@@ -40,7 +40,7 @@ int CppStackFrame::lineNumber() const
 {
     if (auto *line = lineAndStatement(this))
         return line->line;
-    return -1;
+    return missingLineNumber();
 }
 
 int CppStackFrame::statementNumber() const
@@ -50,6 +50,15 @@ int CppStackFrame::statementNumber() const
     return -1;
 }
 
+int CppStackFrame::missingLineNumber() const
+{
+    // Remove the first bit so that we can cast to positive int and negate.
+    // Remove the last bit so that it can't be -1.
+    const int result = -int(quintptr(this) & 0x7ffffffe);
+    Q_ASSERT(result < -1);
+    return result;
+}
+
 ReturnedValue QV4::CppStackFrame::thisObject() const
 {
     if (isJSTypesFrame())

src/qml/jsruntime/qv4stackframe_p.h

@@ -83,6 +83,8 @@ struct Q_QML_PRIVATE_EXPORT CppStackFrame : protected CppStackFrameBase
     int lineNumber() const;
     int statementNumber() const;
 
+    int missingLineNumber() const;
+
     CppStackFrame *parentFrame() const { return parent; }
     void setParentFrame(CppStackFrame *parentFrame) { parent = parentFrame; }
 

src/qml/qml/qqml.cpp

@@ -1234,13 +1234,25 @@ static bool initValueLookup(QV4::Lookup *l, QV4::ExecutableCompilationUnit *comp
 
 static void amendException(QV4::ExecutionEngine *engine)
 {
+    const int missingLineNumber = engine->currentStackFrame->missingLineNumber();
     const int lineNumber = engine->currentStackFrame->lineNumber();
-    engine->exceptionStackTrace.front().line = lineNumber;
+    Q_ASSERT(missingLineNumber != lineNumber);
+
+    auto amendStackTrace = [&](QV4::StackTrace *stackTrace) {
+        for (auto it = stackTrace->begin(), end = stackTrace->end(); it != end; ++it) {
+            if (it->line == missingLineNumber) {
+                it->line = lineNumber;
+                break;
+            }
+        }
+    };
+
+    amendStackTrace(&engine->exceptionStackTrace);
 
     QV4::Scope scope(engine);
     QV4::Scoped<QV4::ErrorObject> error(scope, *engine->exceptionValue);
     if (error) // else some other value was thrown
-        error->d()->stackTrace->front().line = lineNumber;
+        amendStackTrace(error->d()->stackTrace);
 }
 
 

src/qml/qml/qqmlbuiltinfunctions.cpp

@@ -1755,15 +1755,14 @@ static QString jsStack(QV4::ExecutionEngine *engine) {
         const QV4::StackFrame &frame = stackTrace.at(i);
 
         QString stackFrame;
-        if (frame.column >= 0)
-            stackFrame = QStringLiteral("%1 (%2:%3:%4)").arg(frame.function,
-                                                             frame.source,
-                                                             QString::number(frame.line),
-                                                             QString::number(frame.column));
-        else
-            stackFrame = QStringLiteral("%1 (%2:%3)").arg(frame.function,
-                                                             frame.source,
-                                                             QString::number(frame.line));
+        if (frame.column >= 0) {
+            stackFrame = QStringLiteral("%1 (%2:%3:%4)").arg(
+                frame.function, frame.source,
+                QString::number(qAbs(frame.line)), QString::number(frame.column));
+        } else {
+            stackFrame = QStringLiteral("%1 (%2:%3)").arg(
+                frame.function, frame.source, QString::number(qAbs(frame.line)));
+        }
 
         if (i)
             stack += QLatin1Char('\n');

src/qmltest/quicktestutil.cpp

@@ -87,7 +87,7 @@ int QuickTestUtil::callerLine(int frameIndex) const
 
     QVector<QV4::StackFrame> stack = v4->stackTrace(frameIndex + 2);
     if (stack.size() > frameIndex + 1)
-        return stack.at(frameIndex + 1).line;
+        return qAbs(stack.at(frameIndex + 1).line);
     return -1;
 }
 

tests/auto/qml/debugger/qv4debugger/tst_qv4debugger.cpp

@@ -274,9 +274,10 @@ public:
     void dumpStackTrace() const
     {
         qDebug() << "Stack depth:" << m_stackTrace.size();
-        foreach (const QV4::StackFrame &frame, m_stackTrace)
+        for (const QV4::StackFrame &frame : m_stackTrace) {
             qDebug("\t%s (%s:%d:%d)", qPrintable(frame.function), qPrintable(frame.source),
-                   frame.line, frame.column);
+                   qAbs(frame.line), frame.column);
+        }
     }
 };
 

tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt

@@ -105,6 +105,7 @@ set(qml_files
     equalityQUrl.qml
     equalityVarAndNonStorable.qml
     equalsUndefined.qml
+    exceptionFromInner.qml
     excessiveParameters.qml
     extendedTypes.qml
     failures.qml

tests/auto/qml/qmlcppcodegen/data/exceptionFromInner.qml

@@ -0,0 +1,10 @@
+pragma Strict
+import QtQml
+
+QtObject {
+    property QtObject theNull: null
+
+    function doFail() : string { return theNull.objectName }
+    function delegateFail() : string { doFail() }
+    function disbelieveFail() : string { delegateFail() }
+}

tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp

@@ -74,6 +74,7 @@ private slots:
     void equalityVarAndNonStorable();
     void equalsUndefined();
     void evadingAmbiguity();
+    void exceptionFromInner();
     void excessiveParameters();
     void extendedTypes();
     void failures();
@@ -1415,6 +1416,21 @@ void tst_QmlCppCodegen::evadingAmbiguity()
     QCOMPARE(o2->property("i").toString(), QStringLiteral("Ambiguous2"));
 }
 
+void tst_QmlCppCodegen::exceptionFromInner()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine, QUrl(u"qrc:/qt/qml/TestTypes/exceptionFromInner.qml"_s));
+    QVERIFY2(!component.isError(), component.errorString().toUtf8());
+    QScopedPointer<QObject> object(component.create());
+    QVERIFY(!object.isNull());
+
+    QTest::ignoreMessage(
+        QtWarningMsg,
+        "qrc:/qt/qml/TestTypes/exceptionFromInner.qml:7: TypeError: "
+        "Cannot read property 'objectName' of null");
+    QMetaObject::invokeMethod(object.data(), "disbelieveFail");
+}
+
 void tst_QmlCppCodegen::excessiveParameters()
 {
     QQmlEngine engine;