Re-enable Debug instructions and locations for QML functions

Debug instructions are used to trigger break points and are added for
every source line.

We also need to insert Debug instructions before Ret, so that we can
step out. We also need to assign line numbers to the entry and return
points of "abbreviated" QML functions (by simulating lbrace and
rbrace) so that we can set break points on them. The line numbers on Ret
need to be negative, so that you cannot (accidentally) set break points
on them. A typical signal handler or binding in QML consists of only one
line and if you set a break point on that line, you want it to hit only
once, when entering the function. If the line numbers on Ret were
positive, it would be hit again on exit. Some of the tests in
tst_qqmldebugjs implicitly check for that.

Also the new interpreter does something on the left brace, so a
function actually starts there, not on the first statement.

Change-Id: Id9dfb20e35696b420d0950deab988f7cc5197bfc
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

src/plugins/qmltooling/qmldbg_debugger/qv4debugger.cpp

@@ -203,6 +203,7 @@ void QV4Debugger::maybeBreakAtInstruction()
         pauseAndWait(PauseRequest);
     } else if (m_haveBreakPoints) {
         if (QV4::Function *f = getFunction()) {
+            // lineNumber will be negative for Ret instructions, so those won't match
             const int lineNumber = engine()->currentStackFrame->lineNumber();
             if (reallyHitTheBreakPoint(f->sourceFile(), lineNumber))
                 pauseAndWait(BreakPointHit);

src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.cpp

@@ -84,7 +84,7 @@ void QV4DebuggerAgent::debuggerPaused(QV4Debugger *debugger, QV4Debugger::PauseR
             break;
 
         body.insert(QStringLiteral("invocationText"), frame->function());
-        body.insert(QStringLiteral("sourceLine"), frame->lineNumber() - 1);
+        body.insert(QStringLiteral("sourceLine"), qAbs(frame->lineNumber()) - 1);
 //        if (frame->column > 0)
 //            body.insert(QStringLiteral("sourceColumn"), frame->column);
         QJsonArray breakPoints;

src/plugins/qmltooling/qmldbg_nativedebugger/qqmlnativedebugservice.cpp

@@ -607,6 +607,7 @@ void NativeDebugger::maybeBreakAtInstruction()
 
     if (m_service->m_breakHandler->m_haveBreakPoints) {
         if (QV4::Function *function = getFunction()) {
+            // lineNumber will be negative for Ret instructions, so those won't match
             const int lineNumber = m_engine->currentStackFrame->lineNumber();
             if (reallyHitTheBreakPoint(function, lineNumber))
                 pauseAndWait();

src/qml/compiler/qqmltypecompiler.cpp

@@ -491,7 +491,9 @@ bool SignalHandlerConverter::convertSignalHandlerExpressionsToFunctionDeclaratio
             QQmlJS::AST::FunctionBody *body = new (pool) QQmlJS::AST::FunctionBody(elements);
 
             functionDeclaration = new (pool) QQmlJS::AST::FunctionDeclaration(compiler->newStringRef(stringAt(binding->propertyNameIndex)), paramList, body);
-            functionDeclaration->functionToken = foe->node->firstSourceLocation();
+            functionDeclaration->lbraceToken = functionDeclaration->functionToken
+                    = foe->node->firstSourceLocation();
+            functionDeclaration->rbraceToken = foe->node->lastSourceLocation();
         }
         foe->node = functionDeclaration;
         binding->propertyNameIndex = compiler->registerString(propertyName);

src/qml/compiler/qv4bytecodegenerator.cpp

@@ -181,7 +181,21 @@ void BytecodeGenerator::finalize(Compiler::Context *context)
 }
 
 int BytecodeGenerator::addInstructionHelper(Instr::Type type, const Instr &i, int offsetOfOffset) {
-    int pos = instructions.size();
+    if (debugMode && type != Instr::Type::Debug) {
+QT_WARNING_PUSH
+QT_WARNING_DISABLE_GCC("-Wmaybe-uninitialized") // broken gcc warns about Instruction::Debug()
+        if (instructions.isEmpty() || currentLine != instructions.constLast().line) {
+            addInstruction(Instruction::Debug());
+        } else if (type == Instr::Type::Ret) {
+            currentLine = -currentLine;
+            addInstruction(Instruction::Debug());
+            currentLine = -currentLine;
+        }
+QT_WARNING_POP
+    }
+
+    const int pos = instructions.size();
+
     int s = Moth::InstrInfo::argumentCount[static_cast<int>(type)]*sizeof(int);
     if (offsetOfOffset != -1)
         offsetOfOffset += 1;

src/qml/compiler/qv4bytecodegenerator_p.h

@@ -65,8 +65,8 @@ namespace Moth {
 
 class BytecodeGenerator {
 public:
-    BytecodeGenerator(int line)
-        : startLine(line) {}
+    BytecodeGenerator(int line, bool debug)
+        : startLine(line), debugMode(debug) {}
 
     struct Label {
         enum LinkMode {
@@ -271,6 +271,7 @@ public:
 private:
     int startLine = 0;
     int currentLine = 0;
+    bool debugMode = false;
 };
 
 }

src/qml/compiler/qv4codegen.cpp

@@ -2027,7 +2027,7 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
     // ### still needed?
     _context->maxNumberOfArguments = qMax(_context->maxNumberOfArguments, (int)QV4::Global::ReservedArgumentCount);
 
-    BytecodeGenerator bytecode(_context->line);
+    BytecodeGenerator bytecode(_context->line, _module->debugMode);
     BytecodeGenerator *savedBytecodeGenerator;
     savedBytecodeGenerator = bytecodeGenerator;
     bytecodeGenerator = &bytecode;

src/qml/jsruntime/qv4engine.cpp

@@ -809,7 +809,7 @@ StackTrace ExecutionEngine::stackTrace(int frameLimit) const
         QV4::StackFrame frame;
         frame.source = f->source();
         frame.function = f->function();
-        frame.line = f->lineNumber();
+        frame.line = qAbs(f->lineNumber());
         frame.column = -1;
         stack.append(frame);
         --frameLimit;

src/qml/jsruntime/qv4vme_moth.cpp

@@ -1294,6 +1294,7 @@ QV4::ReturnedValue VME::exec(CallData *callData, QV4::Function *function)
 
 #ifndef QT_NO_QML_DEBUGGER
     MOTH_BEGIN_INSTR(Debug)
+        STORE_IP();
         debug_slowPath(engine);
     MOTH_END_INSTR(Debug)
 #endif // QT_NO_QML_DEBUGGER

tests/auto/qml/debugger/qqmldebugjs/qqmldebugjs/tst_qqmldebugjs.cpp

@@ -855,7 +855,6 @@ void tst_QQmlDebugJS::disconnect()
 
 void tst_QQmlDebugJS::setBreakpointInScriptOnCompleted()
 {
-    QSKIP("fixme");
     //void setBreakpoint(QString type, QString target, int line = -1, int column = -1, bool enabled = false, QString condition = QString(), int ignoreCount = -1)
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
@@ -879,7 +878,6 @@ void tst_QQmlDebugJS::setBreakpointInScriptOnCompleted()
 
 void tst_QQmlDebugJS::setBreakpointInScriptOnComponentCreated()
 {
-    QSKIP("fixme");
     //void setBreakpoint(QString type, QString target, int line = -1, int column = -1, bool enabled = false, QString condition = QString(), int ignoreCount = -1)
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
@@ -903,7 +901,6 @@ void tst_QQmlDebugJS::setBreakpointInScriptOnComponentCreated()
 
 void tst_QQmlDebugJS::setBreakpointInScriptOnTimerCallback()
 {
-    QSKIP("fixme");
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
     QFETCH(bool, namesAsObjects);
@@ -1001,7 +998,6 @@ void tst_QQmlDebugJS::setBreakpointInScriptOnEmptyLine()
 
 void tst_QQmlDebugJS::setBreakpointInScriptOnOptimizedBinding()
 {
-    QSKIP("fixme");
     //void setBreakpoint(QString type, QString target, int line = -1, int column = -1, bool enabled = false, QString condition = QString(), int ignoreCount = -1)
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
@@ -1065,7 +1061,6 @@ void tst_QQmlDebugJS::setBreakpointInScriptWithCondition()
 
 void tst_QQmlDebugJS::setBreakpointInScriptThatQuits()
 {
-    QSKIP("fixme");
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
     QFETCH(bool, namesAsObjects);
@@ -1109,7 +1104,6 @@ void tst_QQmlDebugJS::setBreakpointWhenAttaching()
 
 void tst_QQmlDebugJS::clearBreakpoint()
 {
-    QSKIP("fixme");
     //void clearBreakpoint(int breakpoint);
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
@@ -1173,7 +1167,6 @@ void tst_QQmlDebugJS::setExceptionBreak()
 
 void tst_QQmlDebugJS::stepNext()
 {
-    QSKIP("fixme");
     //void continueDebugging(StepAction stepAction, int stepCount = 1);
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
@@ -1198,37 +1191,40 @@ void tst_QQmlDebugJS::stepNext()
     QCOMPARE(QFileInfo(body.value("script").toMap().value("name").toString()).fileName(), QLatin1String(STEPACTION_QMLFILE));
 }
 
+static QVariantMap responseBody(QJSDebugClient *client)
+{
+    const QString jsonString(client->response);
+    const QVariantMap value = client->parser.call(QJSValueList() << QJSValue(jsonString))
+            .toVariant().toMap();
+    return value.value("body").toMap();
+}
+
 void tst_QQmlDebugJS::stepIn()
 {
-    QSKIP("fixme");
     //void continueDebugging(StepAction stepAction, int stepCount = 1);
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
     QFETCH(bool, namesAsObjects);
 
     int sourceLine = 41;
-    int actualLine = 37;
+    int actualLine = 36;
     QCOMPARE(init(qmlscene, STEPACTION_QMLFILE), ConnectSuccess);
 
     m_client->setBreakpoint(QLatin1String(STEPACTION_QMLFILE), sourceLine, 1, true);
     m_client->connect(redundantRefs, namesAsObjects);
     QVERIFY(QQmlDebugTest::waitForSignal(m_client, SIGNAL(stopped())));
+    QCOMPARE(responseBody(m_client).value("sourceLine").toInt(), sourceLine);
 
     m_client->continueDebugging(QJSDebugClient::In);
     QVERIFY(QQmlDebugTest::waitForSignal(m_client, SIGNAL(stopped())));
 
-    QString jsonString(m_client->response);
-    QVariantMap value = m_client->parser.call(QJSValueList() << QJSValue(jsonString)).toVariant().toMap();
-
-    QVariantMap body = value.value("body").toMap();
-
+    const QVariantMap body = responseBody(m_client);
     QCOMPARE(body.value("sourceLine").toInt(), actualLine);
     QCOMPARE(QFileInfo(body.value("script").toMap().value("name").toString()).fileName(), QLatin1String(STEPACTION_QMLFILE));
 }
 
 void tst_QQmlDebugJS::stepOut()
 {
-    QSKIP("fixme");
     //void continueDebugging(StepAction stepAction, int stepCount = 1);
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
@@ -1241,22 +1237,18 @@ void tst_QQmlDebugJS::stepOut()
     m_client->setBreakpoint(QLatin1String(STEPACTION_QMLFILE), sourceLine, -1, true);
     m_client->connect(redundantRefs, namesAsObjects);
     QVERIFY(QQmlDebugTest::waitForSignal(m_client, SIGNAL(stopped())));
+    QCOMPARE(responseBody(m_client).value("sourceLine").toInt(), sourceLine);
 
     m_client->continueDebugging(QJSDebugClient::Out);
     QVERIFY(QQmlDebugTest::waitForSignal(m_client, SIGNAL(stopped())));
 
-    QString jsonString(m_client->response);
-    QVariantMap value = m_client->parser.call(QJSValueList() << QJSValue(jsonString)).toVariant().toMap();
-
-    QVariantMap body = value.value("body").toMap();
-
+    const QVariantMap body = responseBody(m_client);
     QCOMPARE(body.value("sourceLine").toInt(), actualLine);
     QCOMPARE(QFileInfo(body.value("script").toMap().value("name").toString()).fileName(), QLatin1String(STEPACTION_QMLFILE));
 }
 
 void tst_QQmlDebugJS::continueDebugging()
 {
-    QSKIP("fixme");
     //void continueDebugging(StepAction stepAction, int stepCount = 1);
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
@@ -1285,7 +1277,6 @@ void tst_QQmlDebugJS::continueDebugging()
 
 void tst_QQmlDebugJS::backtrace()
 {
-    QSKIP("fixme");
     //void backtrace(int fromFrame = -1, int toFrame = -1, bool bottom = false);
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
@@ -1304,7 +1295,6 @@ void tst_QQmlDebugJS::backtrace()
 
 void tst_QQmlDebugJS::getFrameDetails()
 {
-    QSKIP("fixme");
     //void frame(int number = -1);
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
@@ -1323,7 +1313,6 @@ void tst_QQmlDebugJS::getFrameDetails()
 
 void tst_QQmlDebugJS::getScopeDetails()
 {
-    QSKIP("fixme");
     //void scope(int number = -1, int frameNumber = -1);
     QFETCH(bool, qmlscene);
     QFETCH(bool, redundantRefs);
@@ -1362,7 +1351,6 @@ void tst_QQmlDebugJS::evaluateInGlobalScope()
 
 void tst_QQmlDebugJS::evaluateInLocalScope()
 {
-    QSKIP("fixme");
     //void evaluate(QString expr, bool global = false, bool disableBreak = false, int frame = -1, const QVariantMap &addContext = QVariantMap());
 
     QFETCH(bool, qmlscene);
@@ -1453,7 +1441,6 @@ void tst_QQmlDebugJS::evaluateInContext()
 
 void tst_QQmlDebugJS::getScripts()
 {
-    QSKIP("fixme");
     //void scripts(int types = -1, QList<int> ids = QList<int>(), bool includeSource = false, QVariant filter = QVariant());
 
     QFETCH(bool, qmlscene);

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

@@ -363,7 +363,6 @@ void tst_qv4debugger::cleanup()
 
 void tst_qv4debugger::breakAnywhere()
 {
-    QSKIP("fixme");
     QString script =
             "var i = 42;\n"
             "var j = i + 1\n"
@@ -375,7 +374,6 @@ void tst_qv4debugger::breakAnywhere()
 
 void tst_qv4debugger::pendingBreakpoint()
 {
-    QSKIP("fixme");
     QString script =
             "var i = 42;\n"
             "var j = i + 1\n"
@@ -391,7 +389,6 @@ void tst_qv4debugger::pendingBreakpoint()
 
 void tst_qv4debugger::liveBreakPoint()
 {
-    QSKIP("fixme");
     QString script =
             "var i = 42;\n"
             "var j = i + 1\n"
@@ -420,7 +417,6 @@ void tst_qv4debugger::removePendingBreakPoint()
 
 void tst_qv4debugger::addBreakPointWhilePaused()
 {
-    QSKIP("fixme");
     QString script =
             "var i = 42;\n"
             "var j = i + 1\n"
@@ -464,7 +460,6 @@ void tst_qv4debugger::removeBreakPointForNextInstruction()
 
 void tst_qv4debugger::conditionalBreakPoint()
 {
-    QSKIP("fixme");
     m_debuggerAgent->m_captureContextInfo = true;
     QString script =
             "function test() {\n"
@@ -484,6 +479,7 @@ void tst_qv4debugger::conditionalBreakPoint()
 
     QVERIFY(m_debuggerAgent->m_capturedScope.size() > 1);
     const TestAgent::NamedRefs &frame0 = m_debuggerAgent->m_capturedScope.at(0);
+    QSKIP("fixme: function 'test' ends up in frame0 but shouldn't.");
     QCOMPARE(frame0.size(), 2);
     QVERIFY(frame0.contains("i"));
     QCOMPARE(frame0.value("i").toInt(), 11);
@@ -491,7 +487,6 @@ void tst_qv4debugger::conditionalBreakPoint()
 
 void tst_qv4debugger::conditionalBreakPointInQml()
 {
-    QSKIP("fixme");
     QQmlEngine engine;
     QV4::ExecutionEngine *v4 = QV8Engine::getV4(&engine);
     QV4Debugger *v4Debugger = new QV4Debugger(v4);
@@ -722,7 +717,6 @@ void tst_qv4debugger::breakInCatch()
 
 void tst_qv4debugger::breakInWith()
 {
-    QSKIP("fixme");
     QString script =
             "with (42) {\n"
             "    console.log('give the answer');\n"
@@ -740,7 +734,6 @@ void tst_qv4debugger::breakInWith()
 
 void tst_qv4debugger::evaluateExpression()
 {
-    QSKIP("fixme");
     QFETCH(bool, redundantRefs);
     m_debuggerAgent->collector.setRedundantRefs(redundantRefs);