Don't store the current line number in the ExecutionContext

Instead modify our StackFrame struct to hold the
QV4::Function and have a linked list of those for
the frames.

Change-Id: I8676e16bc51a5ba6cf25a5b3423576d44e8a926a
Reviewed-by: Erik Verbruggen <erik.verbruggen@qt.io>

10 files changed, 57 insertions, 78 deletions

diff --git a/src/qml/jsruntime/qv4context_p.h b/src/qml/jsruntime/qv4context_p.h
index f565f57be8..9408f85d66 100644
--- a/src/qml/jsruntime/qv4context_p.h
+++ b/src/qml/jsruntime/qv4context_p.h
@@ -107,8 +107,6 @@ struct QmlContext;
     Member(class, Pointer, ExecutionContext *, outer) \
     Member(class, Pointer, Object *, activation) \
     Member(class, NoMark, QV4::Function *, v4Function) \
-    Member(class, NoMark, int, lineNumber) // as member of non-pointer size this has to come last to preserve the ability to
-                                           // translate offsetof of it between 64-bit and 32-bit.
 
 DECLARE_HEAP_OBJECT(ExecutionContext, Base) {
     DECLARE_MARK_TABLE(ExecutionContext);
@@ -127,7 +125,6 @@ DECLARE_HEAP_OBJECT(ExecutionContext, Base) {
         Base::init();
 
         type = t;
-        lineNumber = -1;
     }
 
     quint8 type;
@@ -146,7 +143,6 @@ Q_STATIC_ASSERT(offsetof(ExecutionContextData, callData) == 0);
 Q_STATIC_ASSERT(offsetof(ExecutionContextData, outer) == offsetof(ExecutionContextData, callData) + QT_POINTER_SIZE);
 Q_STATIC_ASSERT(offsetof(ExecutionContextData, activation) == offsetof(ExecutionContextData, outer) + QT_POINTER_SIZE);
 Q_STATIC_ASSERT(offsetof(ExecutionContextData, v4Function) == offsetof(ExecutionContextData, activation) + QT_POINTER_SIZE);
-Q_STATIC_ASSERT(offsetof(ExecutionContextData, lineNumber) == offsetof(ExecutionContextData, v4Function) + QT_POINTER_SIZE);
 
 #define CallContextMembers(class, Member) \
     Member(class, Pointer, FunctionObject *, function) \
diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index 61b8d7e68d..167e38f04a 100644
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -804,54 +804,32 @@ QQmlContextData *ExecutionEngine::callingQmlContext() const
     return ctx->qml()->context->contextData();
 }
 
+QString StackFrame::source() const
+{
+    return v4Function->sourceFile();
+}
+
+QString StackFrame::function() const
+{
+    return v4Function->name()->toQString();
+}
+
 QVector<StackFrame> ExecutionEngine::stackTrace(int frameLimit) const
 {
     Scope scope(const_cast<ExecutionEngine *>(this));
     ScopedString name(scope);
     QVector<StackFrame> stack;
 
-    ExecutionContext *c = currentContext;
-    while (c && frameLimit) {
-        QV4::Function *function = c->getFunction();
-        if (function) {
-            StackFrame frame;
-            frame.source = function->sourceFile();
-            name = function->name();
-            frame.function = name->toQString();
-
-            // line numbers can be negative for places where you can't set a real breakpoint
-            frame.line = qAbs(c->d()->lineNumber);
-            frame.column = -1;
-
-            stack.append(frame);
-            --frameLimit;
-        }
-        c = parentContext(c);
-    }
-
-    if (frameLimit && globalCode) {
-        StackFrame frame;
-        frame.source = globalCode->sourceFile();
-        frame.function = globalCode->name()->toQString();
-        frame.line = rootContext()->d()->lineNumber;
-        frame.column = -1;
-
+    StackFrame *f = currentStackFrame;
+    while (f && frameLimit) {
+        StackFrame frame = *f;
+        frame.parent = 0;
         stack.append(frame);
+        --frameLimit;
+        f = f->parent;
     }
-    return stack;
-}
 
-StackFrame ExecutionEngine::currentStackFrame() const
-{
-    StackFrame frame;
-    frame.line = -1;
-    frame.column = -1;
-
-    QVector<StackFrame> trace = stackTrace(/*limit*/ 1);
-    if (!trace.isEmpty())
-        frame = trace.first();
-
-    return frame;
+    return stack;
 }
 
 /* Helper and "C" linkage exported function to format a GDBMI stacktrace for
@@ -871,9 +849,9 @@ static inline char *v4StackTrace(const ExecutionContext *context)
         for (int i = 0; i < stackTrace.size(); ++i) {
             if (i)
                 str << ',';
-            const QUrl url(stackTrace.at(i).source);
+            const QUrl url(stackTrace.at(i).source());
             const QString fileName = url.isLocalFile() ? url.toLocalFile() : url.toString();
-            str << "frame={level=\"" << i << "\",func=\"" << stackTrace.at(i).function
+            str << "frame={level=\"" << i << "\",func=\"" << stackTrace.at(i).function()
                 << "\",file=\"" << fileName << "\",fullname=\"" << fileName
                 << "\",line=\"" << stackTrace.at(i).line << "\",language=\"js\"}";
         }
@@ -1088,7 +1066,7 @@ QQmlError ExecutionEngine::catchExceptionAsQmlError()
     QQmlError error;
     if (!trace.isEmpty()) {
         QV4::StackFrame frame = trace.constFirst();
-        error.setUrl(QUrl(frame.source));
+        error.setUrl(QUrl(frame.source()));
         error.setLine(frame.line);
         error.setColumn(frame.column);
     }
diff --git a/src/qml/jsruntime/qv4engine_p.h b/src/qml/jsruntime/qv4engine_p.h
index 0bb460243c..d9678636c7 100644
--- a/src/qml/jsruntime/qv4engine_p.h
+++ b/src/qml/jsruntime/qv4engine_p.h
@@ -408,7 +408,6 @@ public:
 
 
     StackTrace stackTrace(int frameLimit = -1) const;
-    StackFrame currentStackFrame() const;
     QUrl resolvedUrl(const QString &file);
 
     void requireArgumentsAccessors(int n);
diff --git a/src/qml/jsruntime/qv4enginebase_p.h b/src/qml/jsruntime/qv4enginebase_p.h
index b6ab0b7b68..55c44f33f1 100644
--- a/src/qml/jsruntime/qv4enginebase_p.h
+++ b/src/qml/jsruntime/qv4enginebase_p.h
@@ -64,6 +64,7 @@ namespace QV4 {
 #endif
 struct EngineBase {
     Heap::ExecutionContext *current = 0;
+    StackFrame *currentStackFrame = 0;
 
     Value *jsStackTop = 0;
     quint8 hasException = false;
@@ -116,7 +117,8 @@ struct EngineBase {
 
 Q_STATIC_ASSERT(std::is_standard_layout<EngineBase>::value);
 Q_STATIC_ASSERT(offsetof(EngineBase, current) == 0);
-Q_STATIC_ASSERT(offsetof(EngineBase, jsStackTop) == offsetof(EngineBase, current) + QT_POINTER_SIZE);
+Q_STATIC_ASSERT(offsetof(EngineBase, currentStackFrame) == offsetof(EngineBase, current) + QT_POINTER_SIZE);
+Q_STATIC_ASSERT(offsetof(EngineBase, jsStackTop) == offsetof(EngineBase, currentStackFrame) + QT_POINTER_SIZE);
 Q_STATIC_ASSERT(offsetof(EngineBase, hasException) == offsetof(EngineBase, jsStackTop) + QT_POINTER_SIZE);
 Q_STATIC_ASSERT(offsetof(EngineBase, memoryManager) == offsetof(EngineBase, hasException) + QT_POINTER_SIZE);
 Q_STATIC_ASSERT(offsetof(EngineBase, runtime) == offsetof(EngineBase, memoryManager) + QT_POINTER_SIZE);
diff --git a/src/qml/jsruntime/qv4errorobject.cpp b/src/qml/jsruntime/qv4errorobject.cpp
index d483db7a8e..f7c34e7550 100644
--- a/src/qml/jsruntime/qv4errorobject.cpp
+++ b/src/qml/jsruntime/qv4errorobject.cpp
@@ -96,7 +96,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_FileName, scope.engine->newString(e->d()->stackTrace->at(0).source()));
         setProperty(scope.engine, QV4::ErrorObject::Index_LineNumber, Primitive::fromInt32(e->d()->stackTrace->at(0).line));
     }
 
@@ -106,6 +106,7 @@ void Heap::ErrorObject::init(const Value &message, ErrorType t)
 
 void Heap::ErrorObject::init(const Value &message, const QString &fileName, int line, int column, ErrorObject::ErrorType t)
 {
+    Q_UNUSED(fileName); // ####
     Object::init();
     errorType = t;
 
@@ -116,16 +117,14 @@ void Heap::ErrorObject::init(const Value &message, const QString &fileName, int
     setProperty(scope.engine, QV4::ErrorObject::Index_Stack + QV4::Object::SetterOffset, Primitive::undefinedValue());
 
     e->d()->stackTrace = new StackTrace(scope.engine->stackTrace());
-    StackFrame frame;
-    frame.source = fileName;
+    StackFrame frame = *scope.engine->currentStackFrame;
     frame.line = line;
     frame.column = column;
     e->d()->stackTrace->prepend(frame);
 
-    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, Primitive::fromInt32(e->d()->stackTrace->at(0).line));
-    }
+    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, Primitive::fromInt32(e->d()->stackTrace->at(0).line));
 
     if (!message.isUndefined())
         setProperty(scope.engine, QV4::ErrorObject::Index_Message, message);
@@ -163,7 +162,7 @@ void ErrorObject::method_get_stack(const BuiltinFunction *, Scope &scope, CallDa
             if (i > 0)
                 trace += QLatin1Char('\n');
             const StackFrame &frame = This->d()->stackTrace->at(i);
-            trace += frame.function + QLatin1Char('@') + frame.source;
+            trace += frame.function() + QLatin1Char('@') + frame.source();
             if (frame.line >= 0)
                 trace += QLatin1Char(':') + QString::number(frame.line);
         }
diff --git a/src/qml/jsruntime/qv4global_p.h b/src/qml/jsruntime/qv4global_p.h
index 8e091fd80e..d06ab86593 100644
--- a/src/qml/jsruntime/qv4global_p.h
+++ b/src/qml/jsruntime/qv4global_p.h
@@ -357,11 +357,14 @@ struct PropertyAttributes
     }
 };
 
-struct StackFrame {
-    QString source;
-    QString function;
-    int line;
-    int column;
+struct Q_QML_EXPORT StackFrame {
+    StackFrame *parent;
+    Function *v4Function;
+    int line = -1;
+    int column = -1;
+
+    QString source() const;
+    QString function() const;
 };
 typedef QVector<StackFrame> StackTrace;
 
diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index 91650f16e2..fbb7f9729b 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -468,11 +468,11 @@ void QObjectWrapper::setProperty(ExecutionEngine *engine, QObject *object, QQmlP
             if (auto binding = QQmlPropertyPrivate::binding(object, QQmlPropertyIndex(property->coreIndex()))) {
                 Q_ASSERT(!binding->isValueTypeProxy());
                 const auto qmlBinding = static_cast<const QQmlBinding*>(binding);
-                const auto stackFrame = engine->currentStackFrame();
+                const auto stackFrame = engine->currentStackFrame;
                 qCInfo(lcBindingRemoval,
                        "Overwriting binding on %s::%s at %s:%d that was initially bound at %s",
                        object->metaObject()->className(), qPrintable(property->name(object)),
-                       qPrintable(stackFrame.source), stackFrame.line,
+                       qPrintable(stackFrame->source()), stackFrame->line,
                        qPrintable(qmlBinding->expressionIdentifier()));
             }
         }
diff --git a/src/qml/jsruntime/qv4script_p.h b/src/qml/jsruntime/qv4script_p.h
index ef48dde6db..62e1e566bf 100644
--- a/src/qml/jsruntime/qv4script_p.h
+++ b/src/qml/jsruntime/qv4script_p.h
@@ -67,13 +67,11 @@ struct ContextStateSaver {
     Value *savedContext;
     bool strictMode;
     QV4::Function *v4Function;
-    int lineNumber;
 
     ContextStateSaver(const Scope &scope, ExecutionContext *context)
         : savedContext(scope.alloc(1))
         , strictMode(context->d()->strictMode)
         , v4Function(context->d()->v4Function)
-        , lineNumber(context->d()->lineNumber)
     {
         savedContext->setM(context->d());
     }
@@ -81,7 +79,6 @@ struct ContextStateSaver {
         : savedContext(scope.alloc(1))
         , strictMode(context->strictMode)
         , v4Function(context->v4Function)
-        , lineNumber(context->lineNumber)
     {
         savedContext->setM(context);
     }
@@ -91,7 +88,6 @@ struct ContextStateSaver {
         Heap::ExecutionContext *ctx = static_cast<Heap::ExecutionContext *>(savedContext->m());
         ctx->strictMode = strictMode;
         ctx->v4Function = v4Function;
-        ctx->lineNumber = lineNumber;
     }
 };
 
diff --git a/src/qml/jsruntime/qv4sequenceobject.cpp b/src/qml/jsruntime/qv4sequenceobject.cpp
index 8afc672aa2..4d208ad574 100644
--- a/src/qml/jsruntime/qv4sequenceobject.cpp
+++ b/src/qml/jsruntime/qv4sequenceobject.cpp
@@ -66,10 +66,10 @@ static void generateWarning(QV4::ExecutionEngine *v4, const QString& description
     QQmlError retn;
     retn.setDescription(description);
 
-    QV4::StackFrame frame = v4->currentStackFrame();
+    QV4::StackFrame *stackFrame = v4->currentStackFrame;
 
-    retn.setLine(frame.line);
-    retn.setUrl(QUrl(frame.source));
+    retn.setLine(stackFrame->line);
+    retn.setUrl(QUrl(stackFrame->source()));
     QQmlEnginePrivate::warning(engine, retn);
 }
 
diff --git a/src/qml/jsruntime/qv4vme_moth.cpp b/src/qml/jsruntime/qv4vme_moth.cpp
index 348751f0cb..5d3f10b18d 100644
--- a/src/qml/jsruntime/qv4vme_moth.cpp
+++ b/src/qml/jsruntime/qv4vme_moth.cpp
@@ -243,13 +243,13 @@ int qt_v4DebuggerHook(const char *json)
     return -NoSuchCommand; // Failure.
 }
 
-Q_NEVER_INLINE static void qt_v4CheckForBreak(QV4::ExecutionContext *context)
+Q_NEVER_INLINE static void qt_v4CheckForBreak(QV4::StackFrame *frame)
 {
     if (!qt_v4IsStepping && !qt_v4Breakpoints.size())
         return;
 
-    const int lineNumber = context->d()->lineNumber;
-    QV4::Function *function = qt_v4ExtractFunction(context);
+    const int lineNumber = frame->line;
+    QV4::Function *function = frame->v4Function;
     QString engineName = function->sourceFile();
 
     if (engineName.isEmpty())
@@ -282,12 +282,12 @@ Q_NEVER_INLINE static void qt_v4CheckForBreak(QV4::ExecutionContext *context)
 Q_NEVER_INLINE static void debug_slowPath(const QV4::Moth::Instr::instr_debug &instr,
                                           QV4::ExecutionEngine *engine)
 {
-    engine->current->lineNumber = instr.lineNumber;
+    engine->currentStackFrame->line = instr.lineNumber;
     QV4::Debugging::Debugger *debugger = engine->debugger();
     if (debugger && debugger->pauseAtNextOpportunity())
         debugger->maybeBreakAtInstruction();
     if (qt_v4IsDebugging)
-        qt_v4CheckForBreak(engine->currentContext);
+        qt_v4CheckForBreak(engine->currentStackFrame);
 }
 
 #endif // QT_NO_QML_DEBUGGER
@@ -453,6 +453,12 @@ QV4::ReturnedValue VME::exec(Function *function)
 #endif
 
     ExecutionEngine *engine = function->internalClass->engine;
+
+    StackFrame frame;
+    frame.parent = engine->currentStackFrame;
+    frame.v4Function = function;
+    engine->currentStackFrame = &frame;
+
     QV4::Value accumulator = Primitive::undefinedValue();
     QV4::Value *stack = nullptr;
     const uchar *exceptionHandler = 0;
@@ -466,7 +472,6 @@ QV4::ReturnedValue VME::exec(Function *function)
             stack[-i-1] = cc->callData->args[i];
     }
 
-    engine->current->lineNumber = -1;
     if (QV4::Debugging::Debugger *debugger = engine->debugger())
         debugger->enteringFunction();
 
@@ -985,9 +990,9 @@ QV4::ReturnedValue VME::exec(Function *function)
     MOTH_END_INSTR(Debug)
 
     MOTH_BEGIN_INSTR(Line)
-        engine->current->lineNumber = instr.lineNumber;
+        frame.line = instr.lineNumber;
         if (Q_UNLIKELY(qt_v4IsDebugging))
-            qt_v4CheckForBreak(engine->currentContext);
+            qt_v4CheckForBreak(&frame);
     MOTH_END_INSTR(Line)
 #endif // QT_NO_QML_DEBUGGER
 
@@ -1029,5 +1034,6 @@ QV4::ReturnedValue VME::exec(Function *function)
 functionExit:
     if (QV4::Debugging::Debugger *debugger = engine->debugger())
         debugger->leavingFunction(accumulator.asReturnedValue());
+    engine->currentStackFrame = frame.parent;
     return accumulator.asReturnedValue();
 }