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>

9 files changed, 27 insertions, 7 deletions

diff --git a/src/plugins/qmltooling/qmldbg_debugger/qv4debugger.cpp b/src/plugins/qmltooling/qmldbg_debugger/qv4debugger.cpp
index 4ef377922b..1581cf637d 100644
--- a/src/plugins/qmltooling/qmldbg_debugger/qv4debugger.cpp
+++ b/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);
diff --git a/src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.cpp b/src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.cpp
index 2a46dc60a6..87e75c49b5 100644
--- a/src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.cpp
+++ b/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;
diff --git a/src/plugins/qmltooling/qmldbg_nativedebugger/qqmlnativedebugservice.cpp b/src/plugins/qmltooling/qmldbg_nativedebugger/qqmlnativedebugservice.cpp
index cb5d2d4c1b..7bf419a1d9 100644
--- a/src/plugins/qmltooling/qmldbg_nativedebugger/qqmlnativedebugservice.cpp
+++ b/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();
diff --git a/src/qml/compiler/qqmltypecompiler.cpp b/src/qml/compiler/qqmltypecompiler.cpp
index 1e17312fe1..2bee00552f 100644
--- a/src/qml/compiler/qqmltypecompiler.cpp
+++ b/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);
diff --git a/src/qml/compiler/qv4bytecodegenerator.cpp b/src/qml/compiler/qv4bytecodegenerator.cpp
index 78f9639ddd..05bbf25292 100644
--- a/src/qml/compiler/qv4bytecodegenerator.cpp
+++ b/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;
diff --git a/src/qml/compiler/qv4bytecodegenerator_p.h b/src/qml/compiler/qv4bytecodegenerator_p.h
index ece05b41ac..3d516da922 100644
--- a/src/qml/compiler/qv4bytecodegenerator_p.h
+++ b/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;
 };
 
 }
diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 0bf043d3fc..8f6863b5d8 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/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;
diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index d63320b595..5db7da668b 100644
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/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;
diff --git a/src/qml/jsruntime/qv4vme_moth.cpp b/src/qml/jsruntime/qv4vme_moth.cpp
index 8efd684290..b3b9842ba9 100644
--- a/src/qml/jsruntime/qv4vme_moth.cpp
+++ b/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