Debugger: Sanitize Datatypes, part 2: Stackframes.

Make address a quint64. Enable DisassemblerViewAgent
to match the disassembly-addresses by converting the numbers,
making it more robust. Remove the complicated formatting needed
for CDB.

src/plugins/debugger/cdb/cdbassembler.cpp

@@ -36,28 +36,6 @@
 
 #include <QtCore/QVector>
 
-// Format a hex address with a given field width if possible. Convert
-// to number to ensure it is not truncated should it be larger than the
-// field width. Check the 64 bit address format '00000001`40002c84'
-static inline void formatAddress(QTextStream &str, QString hexAddressS, int fieldWidth)
-{
-    if (hexAddressS.size() > 9) {
-        const int sepPos = hexAddressS.size() - 9;
-        if (hexAddressS.at(sepPos) == QLatin1Char('`'))
-            hexAddressS.remove(sepPos, 1);
-    }
-    const QChar oldPadChar = str.padChar();
-    const int oldFieldWidth = str.fieldWidth();
-    const int oldIntegerBase = str.integerBase();
-    str.setFieldWidth(fieldWidth);
-    str.setPadChar(QLatin1Char('0'));
-    str.setIntegerBase(16);
-    str << hexAddressS.toULongLong(0, 16);
-    str.setFieldWidth(oldFieldWidth);
-    str.setPadChar(oldPadChar);
-    str.setIntegerBase(oldIntegerBase);
-}
-
 namespace Debugger {
 namespace Internal {
 
@@ -101,10 +79,25 @@ Registers getRegisters(CIDebugControl *ctl,
     return registers;
 }
 
-// Output parser for disassembler lines.
-// It uses the source file lines as symbol until it encounters
-// a C++ symbol (function entered), from which then on
-// it uses that symbol.
+/* Output parser for disassembler lines: Parse a disassembler line:
+ * \code
+module!class::foo:
+                        004017cf cc int 3
+77 mainwindow.cpp       004018ff 8d4da8           lea     ecx,[ebp-0x58]
+\endcode
+ * and reformat to something like:
+ * \code
+00000001400043c9 mainwindow.cpp+296 90 nop
+00000001400043ca mainwindow.cpp+296 488d8c24d8020000 lea rcx,[rsp+2D8h]
+00000001400043d2 mainwindow.cpp+296 ff1500640300 call qword ptr [gitgui!_imp_??1QStringQEAAXZ (00000001`4003a7d8)]
+\endcode
+ * Reformatting brings address to the front for disassembler agent's extracting
+ * the address for location marker to work.
+ * Moves symbol information to the 2nd column, using the source file lines as
+ * symbol until it encounters a C++ symbol (function entered), from which then on
+ * it uses that symbol, indicating the offset.
+ */
+
 class DisassemblerOutputParser
 {
     Q_DISABLE_COPY(DisassemblerOutputParser)
@@ -130,13 +123,6 @@ DisassemblerOutputParser::DisassemblerOutputParser(QTextStream &str, int address
 {
 }
 
-/* Parse a disassembler line:
- * \code
-module!class::foo:
-                        004017cf cc int 3
-77 mainwindow.cpp       004018ff 8d4da8           lea     ecx,[ebp-0x58]
-\endcode */
-
 DisassemblerOutputParser::ParseResult
     DisassemblerOutputParser::parseDisassembled(const QString &in)
 {
@@ -167,16 +153,13 @@ DisassemblerOutputParser::ParseResult
         return ParseIgnore;
     if (tokenCount < 3)
         return ParseFailed;
-    // Format line. Start with address with the field width given,
-    // which is important for setting the marker.
-    const int addressToken = hasSourceFile ? 2 : 0;
-    m_str << "0x";
-    if (m_str.fieldWidth() == m_addressFieldWidth) {
-        m_str << tokens.at(addressToken);
-    } else {
-        formatAddress(m_str, tokens.at(addressToken), m_addressFieldWidth);
-    }
-    m_str << ' ';
+    // Format line. Start with address which is important for setting the marker.
+    // Fix CDB word separator for location marker hex conversion to work.
+    const int addressTokenPos = hasSourceFile ? 2 : 0;
+    QString addressToken = tokens.at(addressTokenPos);
+    if (addressToken.size() > 9 && addressToken.at(8) == QLatin1Char('`'))
+        addressToken.remove(8, 1);
+    m_str << addressToken << ' ';
     // Symbol display: Do we know a symbol? -> Display with offset.
     // Else default to source file information.
     if (m_sourceSymbol.isEmpty()) {
@@ -189,7 +172,7 @@ DisassemblerOutputParser::ParseResult
         m_str << '>';
         m_sourceSymbolOffset++;
     }
-    for (int i = addressToken + 1; i < tokenCount; i++)
+    for (int i = addressTokenPos + 1; i < tokenCount; i++)
         m_str << ' ' << tokens.at(i);
     m_str << '\n';
     return ParseOk;
@@ -213,7 +196,6 @@ bool dissassemble(CdbCore::CoreEngine *engine,
                   ULONG64 offset,
                   unsigned long beforeLines,
                   unsigned long afterLines,
-                  int addressFieldWidth,
                   QTextStream &str,
                   QString *errorMessage)
 {
@@ -222,7 +204,7 @@ bool dissassemble(CdbCore::CoreEngine *engine,
     QString lines;
     if (!engine->dissassemble(offset, beforeLines, afterLines, &lines, errorMessage))
         return false;
-    DisassemblerOutputParser parser(str, addressFieldWidth);
+    DisassemblerOutputParser parser(str);
     parser.parse(lines.split(QLatin1Char('\n')));
     return true;
 }

src/plugins/debugger/cdb/cdbassembler.h

@@ -58,7 +58,6 @@ bool dissassemble(CdbCore::CoreEngine *engine,
                   ULONG64 offset,
                   unsigned long beforeLines,
                   unsigned long afterLines,
-                  int addressFieldWidth /* = 0*/,
                   QTextStream &str,
                   QString *errorMessage);
 

src/plugins/debugger/cdb/cdbengine.cpp

@@ -1169,45 +1169,23 @@ bool CdbEnginePrivate::attemptBreakpointSynchronization(QString *errorMessage)
 
 void CdbEngine::fetchDisassembler(DisassemblerViewAgent *agent)
 {
-    StackFrame frame = agent->frame();
     enum { ContextLines = 40 };
-    bool ok = false;
     QString errorMessage;
-    do {
-        // get address
-        QString address;
-        if (!frame.file.isEmpty())
-            address = frame.address;
-        if (address.isEmpty())
-            address = agent->address();
-        if (debugCDB)
-            qDebug() << "fetchDisassembler" << address << " Agent: " << agent->address()
-            << " Frame" << frame.file << frame.line << frame.address;
-        if (address.isEmpty()) { // Clear window
-            agent->setContents(QString());
-            ok = true;
-            break;
-        }
-        if (address.startsWith(QLatin1String("0x")))
-            address.remove(0, 2);
-        const int addressFieldWith = address.size(); // For the Marker
+    const quint64 address = agent->address();
+    if (debugCDB)
+        qDebug() << "fetchDisassembler" << address << " Agent: " << agent->address();
 
-        const ULONG64 offset = address.toULongLong(&ok, 16);
-        if (!ok) {
-            errorMessage = QString::fromLatin1("Internal error: Invalid address for disassembly: '%1'.").arg(agent->address());
-            break;
-        }
-        QString disassembly;
-        QApplication::setOverrideCursor(Qt::WaitCursor);
-        ok = dissassemble(m_d, offset, ContextLines, ContextLines, addressFieldWith, QTextStream(&disassembly), &errorMessage);
-        QApplication::restoreOverrideCursor();
-        if (!ok)
-            break;
+    if (address == 0) { // Clear window
+        agent->setContents(QString());
+        return;
+    }
+    QString disassembly;
+    QApplication::setOverrideCursor(Qt::WaitCursor);
+    const bool ok = dissassemble(m_d, address, ContextLines, ContextLines, QTextStream(&disassembly), &errorMessage);
+    QApplication::restoreOverrideCursor();
+    if (ok) {
         agent->setContents(disassembly);
-
-    } while (false);
-
-    if (!ok) {
+    } else {
         agent->setContents(QString());
         warning(errorMessage);
     }

src/plugins/debugger/cdb/cdbstacktracecontext.cpp

@@ -108,7 +108,7 @@ QList<StackFrame> CdbStackTraceContext::stackFrames() const
         frame.line = coreFrame.line;
         frame.function =coreFrame.function;
         frame.from = coreFrame.module;
-        frame.address = hexPrefix + QString::number(coreFrame.address, 16);
+        frame.address = coreFrame.address;
         rc.push_back(frame);
     }
     return rc;

src/plugins/debugger/debuggeragents.cpp

@@ -328,6 +328,29 @@ void DisassemblerViewAgent::setMimeType(const QString &mt)
        d->configureMimeType();
 }
 
+// Return a pair of <linenumber [1..n], character position> of an address
+// in assembly code, assuming lines start with a sane hex address.
+static QPair<int, int> lineNumberOfAddress(const QString &disassembly, quint64 address)
+{
+    if (disassembly.isEmpty())
+        return QPair<int, int>(-1, -1);
+
+    int pos = 0;
+    const QChar newLine = QLatin1Char('\n');
+
+    const int size = disassembly.size();
+    for (int lineNumber = 1; pos < size; lineNumber++) {
+        int endOfLinePos = disassembly.indexOf(newLine, pos + 1);
+        if (endOfLinePos == -1)
+            endOfLinePos = size;
+        const QString line = disassembly.mid(pos, endOfLinePos - pos);
+        if (DisassemblerViewAgent::addressFromDisassemblyLine(line) == address)
+            return QPair<int, int>(lineNumber, pos);
+        pos = endOfLinePos;
+    }
+    return QPair<int, int>(-1, -1);;
+}
+
 void DisassemblerViewAgent::setContents(const QString &contents)
 {
     QTC_ASSERT(d, return);
@@ -360,36 +383,25 @@ void DisassemblerViewAgent::setContents(const QString &contents)
     d->editor->markableInterface()->removeMark(d->locationMark);
     d->editor->setDisplayName(_("Disassembler (%1)").arg(d->frame.function));
 
-    for (int pos = 0, line = 0; ; ++line, ++pos) {
-        if (contents.midRef(pos, d->frame.address.size()) == d->frame.address) {
-            d->editor->markableInterface()->addMark(d->locationMark, line + 1);
-            if (plainTextEdit) {
-                QTextCursor tc = plainTextEdit->textCursor();
-                tc.setPosition(pos);
-                plainTextEdit->setTextCursor(tc);
-            }
-            break;
+    const QPair<int, int> lineNumberPos = lineNumberOfAddress(contents, d->frame.address);
+    if (lineNumberPos.first > 0) {
+        d->editor->markableInterface()->addMark(d->locationMark, lineNumberPos.first);
+        if (plainTextEdit) {
+            QTextCursor tc = plainTextEdit->textCursor();
+            tc.setPosition(lineNumberPos.second);
+            plainTextEdit->setTextCursor(tc);
         }
-        pos = contents.indexOf('\n', pos + 1);
-        if (pos == -1)
-            break;
     }
 }
 
 bool DisassemblerViewAgent::contentsCoversAddress(const QString &contents) const
 {
     QTC_ASSERT(d, return false);
-    for (int pos = 0, line = 0; ; ++line, ++pos) {
-        if (contents.midRef(pos, d->frame.address.size()) == d->frame.address)
-            return true;
-        pos = contents.indexOf('\n', pos + 1);
-        if (pos == -1)
-            break;
-    }
-    return false;
+
+    return lineNumberOfAddress(contents, d->frame.address).first > 0;
 }
 
-QString DisassemblerViewAgent::address() const
+quint64 DisassemblerViewAgent::address() const
 {
     return d->frame.address;
 }

src/plugins/debugger/debuggeragents.h

@@ -92,7 +92,7 @@ public:
     QString mimeType() const;
     Q_SLOT void setMimeType(const QString &mt);
 
-    QString address() const;
+    quint64 address() const;
     bool contentsCoversAddress(const QString &contents) const;
     void cleanup();
     bool isMixed() const;

src/plugins/debugger/gdb/gdbengine.cpp

@@ -1830,7 +1830,7 @@ void GdbEngine::executeStep()
     notifyInferiorRunRequested();
     showStatusMessage(tr("Step requested..."), 5000);
     if (m_gdbAdapter->isTrkAdapter() && stackHandler()->stackSize() > 0)
-        postCommand("sal step," + stackHandler()->topAddress().toLatin1());
+        postCommand("sal step,0x" + QByteArray::number(stackHandler()->topAddress(), 16));
     if (isReverseDebugging())
         postCommand("reverse-step", RunRequest, CB(handleExecuteStep));
     else
@@ -1887,7 +1887,7 @@ void GdbEngine::executeNext()
     notifyInferiorRunRequested();
     showStatusMessage(tr("Step next requested..."), 5000);
     if (m_gdbAdapter->isTrkAdapter() && stackHandler()->stackSize() > 0)
-        postCommand("sal next," + stackHandler()->topAddress().toLatin1());
+        postCommand("sal next,0x" + QByteArray::number(stackHandler()->topAddress(), 16));
     if (isReverseDebugging())
         postCommand("reverse-next", RunRequest, CB(handleExecuteNext));
     else
@@ -2829,7 +2829,7 @@ StackFrame GdbEngine::parseStackFrame(const GdbMi &frameMi, int level)
     frame.function = _(frameMi.findChild("func").data());
     frame.from = _(frameMi.findChild("from").data());
     frame.line = frameMi.findChild("line").data().toInt();
-    frame.address = _(frameMi.findChild("addr").data());
+    frame.address = frameMi.findChild("addr").data().toULongLong(0, 16);
     return frame;
 }
 
@@ -3760,9 +3760,7 @@ void GdbEngine::fetchDisassemblerByAddress(const DisassemblerAgentCookie &ac0,
 {
     DisassemblerAgentCookie ac = ac0;
     QTC_ASSERT(ac.agent, return);
-    bool ok = true;
-    quint64 address = ac.agent->address().toULongLong(&ok, 0);
-    QTC_ASSERT(ok, qDebug() << "ADDRESS: " << ac.agent->address() << address; return);
+    const quint64 address = ac.agent->address();
     QByteArray start = QByteArray::number(address - 20, 16);
     QByteArray end = QByteArray::number(address + 100, 16);
     // -data-disassemble [ -s start-addr -e end-addr ]
@@ -3783,8 +3781,7 @@ void GdbEngine::fetchDisassemblerByCli(const DisassemblerAgentCookie &ac0,
 {
     DisassemblerAgentCookie ac = ac0;
     QTC_ASSERT(ac.agent, return);
-    bool ok = false;
-    quint64 address = ac.agent->address().toULongLong(&ok, 0);
+    const quint64 address = ac.agent->address();
     QByteArray cmd = "disassemble ";
     if (useMixedMode && m_gdbVersion >= 60850)
         cmd += "/m ";
@@ -3799,8 +3796,7 @@ void GdbEngine::fetchDisassemblerByAddressCli(const DisassemblerAgentCookie &ac0
 {
     DisassemblerAgentCookie ac = ac0;
     QTC_ASSERT(ac.agent, return);
-    bool ok = false;
-    quint64 address = ac.agent->address().toULongLong(&ok, 0);
+    const quint64 address = ac.agent->address();
     QByteArray start = QByteArray::number(address - 20, 16);
     QByteArray end = QByteArray::number(address + 100, 16);
     // There have been changes to the syntax sometime between 7.0 and 7.1.

src/plugins/debugger/stackframe.cpp

@@ -48,7 +48,7 @@ namespace Internal {
 ////////////////////////////////////////////////////////////////////////
 
 StackFrame::StackFrame()
-  : level(0), line(0)
+  : level(0), line(0), address(0)
 {}
 
 void StackFrame::clear()
@@ -58,7 +58,7 @@ void StackFrame::clear()
     file.clear();
     from.clear();
     to.clear();
-    address.clear();
+    address = 0;
 }
 
 bool StackFrame::isUsable() const
@@ -70,7 +70,11 @@ QString StackFrame::toString() const
 {
     QString res;
     QTextStream str(&res);
-    str << StackHandler::tr("Address:") << ' ' << address << ' '
+    str << StackHandler::tr("Address:") << ' ';
+    str.setIntegerBase(16);
+    str << address;
+    str.setIntegerBase(10);
+    str << ' '
         << StackHandler::tr("Function:") << ' ' << function << ' '
         << StackHandler::tr("File:") << ' ' << file << ' '
         << StackHandler::tr("Line:") << ' ' << line << ' '
@@ -85,7 +89,11 @@ QString StackFrame::toToolTip() const
     QString res;
     QTextStream str(&res);
     str << "<html><body><table>"
-        << "<tr><td>" << StackHandler::tr("Address:") << "</td><td>" <<  address << "</td></tr>"
+        << "<tr><td>" << StackHandler::tr("Address:") << "</td><td>0x";
+    str.setIntegerBase(16);
+    str <<  address;
+    str.setIntegerBase(10);
+    str << "</td></tr>"
         << "<tr><td>" << StackHandler::tr("Function:") << "</td><td>" << function << "</td></tr>"
         << "<tr><td>" << StackHandler::tr("File:") << "</td><td width="
         << QFontMetrics(QToolTip::font()).width(filePath) << ">" << filePath << "</td></tr>"

src/plugins/debugger/stackframe.h

@@ -57,7 +57,7 @@ public:
     QString from;  // Sometimes something like "/usr/lib/libstdc++.so.6"
     QString to;    // Used in ScriptEngine only.
     int line;
-    QString address;
+    quint64 address;
 };
 
 QDebug operator<<(QDebug d, const StackFrame &frame);

src/plugins/debugger/stackhandler.cpp

@@ -117,7 +117,9 @@ QVariant StackHandler::data(const QModelIndex &index, int role) const
         case 3: // Line number
             return frame.line;
         case 4: // Address
-            return frame.address;
+            if (frame.address)
+                return QString::fromAscii("0x%1").arg(frame.address, 0, 16);
+            return QString();
         }
         return QVariant();
     }
@@ -225,7 +227,7 @@ void StackHandler::setFrames(const StackFrames &frames, bool canExpand)
     reset();
 }
 
-StackFrames StackHandler::frames() const
+const StackFrames &StackHandler::frames() const
 {
     return m_stackFrames;
 }

src/plugins/debugger/stackhandler.h

@@ -75,12 +75,12 @@ public:
     ~StackHandler();
 
     void setFrames(const StackFrames &frames, bool canExpand = false);
-    StackFrames frames() const;
+    const StackFrames &frames() const;
     void setCurrentIndex(int index);
     int currentIndex() const { return m_currentIndex; }
     StackFrame currentFrame() const;
     int stackSize() const { return m_stackFrames.size(); }
-    QString topAddress() const { return m_stackFrames.at(0).address; }
+    quint64 topAddress() const { return m_stackFrames.at(0).address; }
 
     // Called from StackHandler after a new stack list has been received
     void removeAll();

src/plugins/debugger/stackwindow.cpp

@@ -98,7 +98,7 @@ void StackWindow::rowActivated(const QModelIndex &index)
 void StackWindow::contextMenuEvent(QContextMenuEvent *ev)
 {
     const QModelIndex index = indexAt(ev->pos());
-    const QString address = modelData(StackFrameAddressRole, index).toString();
+    const quint64 address = modelData(StackFrameAddressRole, index).toULongLong();
     const unsigned engineCapabilities = modelData(EngineCapabilitiesRole).toUInt();
 
     QMenu menu;
@@ -111,20 +111,20 @@ void StackWindow::contextMenuEvent(QContextMenuEvent *ev)
         menu.addAction(theDebuggerAction(CreateFullBacktrace));
 
     QAction *actShowMemory = menu.addAction(QString());
-    if (address.isEmpty()) {
+    if (address == 0) {
         actShowMemory->setText(tr("Open Memory Editor"));
         actShowMemory->setEnabled(false);
     } else {
-        actShowMemory->setText(tr("Open Memory Editor at %1").arg(address));
+        actShowMemory->setText(tr("Open Memory Editor at 0x%1").arg(address, 0, 16));
         actShowMemory->setEnabled(engineCapabilities & ShowMemoryCapability);
     }
 
     QAction *actShowDisassembler = menu.addAction(QString());
-    if (address.isEmpty()) {
+    if (address == 0) {
         actShowDisassembler->setText(tr("Open Disassembler"));
         actShowDisassembler->setEnabled(false);
     } else {
-        actShowDisassembler->setText(tr("Open Disassembler at %1").arg(address));
+        actShowDisassembler->setText(tr("Open Disassembler at 0x%1").arg(address, 0, 16));
         actShowDisassembler->setEnabled(engineCapabilities & DisassemblerCapability);
     }