Debugger: Rework GDB disassembly display

- Use exclusively CLI to retrieve information. MI output misses parts.
- Remove MI output handling
- Re-order lines in ascending address order in mixed output.
  GDB orders by source line, but that's confusing when stepping
- Introduce a new 'hunk' field for source lines to give a hint
  which part of a split source line we are looking at.

Change-Id: I61a547503e9a18f95e99b897e26dbb207cabb8cf
Reviewed-by: David Schulz <david.schulz@digia.com>

src/plugins/debugger/disassemblerlines.cpp

@@ -227,6 +227,8 @@ QString DisassemblerLine::toString() const
         str += _("        ");
         str += data;
     } else if (isCode()) {
+        if (hunk)
+            str += _("[%1]").arg(hunk);
         str += someSpace;
         str += data;
     } else {

src/plugins/debugger/disassemblerlines.h

@@ -44,9 +44,8 @@ namespace Internal {
 
 class DisassemblerLine
 {
-    //DisassemblerLine(const QString &unparsed);
 public:
-    DisassemblerLine() : address(0), offset(0), lineNumber(0) {}
+    DisassemblerLine() : address(0), offset(0), lineNumber(0), hunk(0) {}
     bool isAssembler() const { return address != 0; }
     bool isCode() const { return lineNumber != 0; }
     bool isComment() const { return lineNumber == 0 && address == 0; }
@@ -61,6 +60,7 @@ public:
     QString function; // (ass) Function to which current instruction belongs.
     uint offset;      // (ass) Offset of instruction in relation to current function.
     uint lineNumber;  // (src) Line number in source.
+    uint hunk;        // (src) Number of hunk if source line was split
     QByteArray rawData;  // (ass) Raw bytes of the instruction
     QString data;     // (ass) Instruction text, (src) source text, (cmt) arbitrary.
 };
@@ -80,6 +80,7 @@ public:
     int size() const { return m_data.size(); }
     const DisassemblerLine &at(int i) const { return m_data.at(i); }
     int lineForAddress(quint64 address) const;
+    QVector<DisassemblerLine> data() const { return m_data; }
 
     quint64 startAddress() const;
     quint64 endAddress() const;

src/plugins/debugger/gdb/gdbengine.cpp

@@ -3968,68 +3968,12 @@ public:
 
 void GdbEngine::fetchDisassembler(DisassemblerAgent *agent)
 {
-    // As of 7.2 the MI output is often less informative then the CLI version.
+    // Doing that unconditionally seems to be the most robust
-    // So globally fall back to CLI.
+    // solution given the richest output. Looks like GDB is
-    if (agent->isMixed())
+    // a command line tool after all...
-        fetchDisassemblerByCliPointMixed(agent);
+    fetchDisassemblerByCliPointMixed(agent);
-    else
-        fetchDisassemblerByCliPointPlain(agent);
-#if 0
-    if (agent->isMixed())
-        fetchDisassemblerByMiRangeMixed(agent)
-    else
-        fetchDisassemblerByMiRangePlain(agent);
-#endif
 }
 
-#if 0
-void GdbEngine::fetchDisassemblerByMiRangePlain(const DisassemblerAgentCookie &ac0)
-{
-    // Disassemble full function:
-    const StackFrame &frame = agent->frame();
-    DisassemblerAgentCookie ac = ac0;
-    QTC_ASSERT(ac.agent, return);
-    const quint64 address = ac.agent->address();
-    QByteArray cmd = "-data-disassemble"
-        " -f " + frame.file.toLocal8Bit() +
-        " -l " + QByteArray::number(frame.line) + " -n -1 -- 1";
-    postCommand(cmd, Discardable, CB(handleFetchDisassemblerByMiPointMixed),
-        QVariant::fromValue(DisassemblerAgentCookie(agent)));
-}
-#endif
-
-#if 0
-void GdbEngine::fetchDisassemblerByMiRangeMixed(const DisassemblerAgentCookie &ac0)
-{
-    DisassemblerAgentCookie ac = ac0;
-    QTC_ASSERT(ac.agent, 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 ]
-    //  | [ -f filename -l linenum [ -n lines ] ] -- mode
-    postCommand("-data-disassemble -s 0x" + start + " -e 0x" + end + " -- 1",
-        Discardable, CB(handleFetchDisassemblerByMiRangeMixed),
-        QVariant::fromValue(ac));
-}
-#endif
-
-#if 0
-void GdbEngine::fetchDisassemblerByMiRangePlain(const DisassemblerAgentCookie &ac0)
-{
-    DisassemblerAgentCookie ac = ac0;
-    QTC_ASSERT(ac.agent, 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 ]
-    //  | [ -f filename -l linenum [ -n lines ] ] -- mode
-    postCommand("-data-disassemble -s 0x" + start + " -e 0x" + end + " -- 0",
-        Discardable, CB(handleFetchDisassemblerByMiRangePlain),
-        QVariant::fromValue(ac));
-}
-#endif
-
 static inline QByteArray disassemblerCommand(const Location &location, bool mixed)
 {
     QByteArray command = "disassemble ";
@@ -4055,19 +3999,6 @@ void GdbEngine::fetchDisassemblerByCliPointMixed(const DisassemblerAgentCookie &
         QVariant::fromValue(ac));
 }
 
-void GdbEngine::fetchDisassemblerByCliPointPlain(const DisassemblerAgentCookie &ac0)
-{
-    // This here
-    //    DisassemblerAgentCookie ac = ac0;
-    //    QTC_ASSERT(ac.agent, return);
-    //    postCommand(disassemblerCommand(ac.agent->location(), false), Discardable,
-    //        CB(handleFetchDisassemblerByCliPointPlain),
-    //        QVariant::fromValue(ac));
-    // takes far too long if function boundaries are not hit.
-    // Skip this feature and immediately fall back to the 'range' version:
-    fetchDisassemblerByCliRangePlain(ac0);
-}
-
 void GdbEngine::fetchDisassemblerByCliRangeMixed(const DisassemblerAgentCookie &ac0)
 {
     DisassemblerAgentCookie ac = ac0;
@@ -4080,6 +4011,7 @@ void GdbEngine::fetchDisassemblerByCliRangeMixed(const DisassemblerAgentCookie &
         CB(handleFetchDisassemblerByCliRangeMixed), QVariant::fromValue(ac));
 }
 
+
 void GdbEngine::fetchDisassemblerByCliRangePlain(const DisassemblerAgentCookie &ac0)
 {
     DisassemblerAgentCookie ac = ac0;
@@ -4092,72 +4024,56 @@ void GdbEngine::fetchDisassemblerByCliRangePlain(const DisassemblerAgentCookie &
         CB(handleFetchDisassemblerByCliRangePlain), QVariant::fromValue(ac));
 }
 
-static DisassemblerLine parseLine(const GdbMi &line)
+struct LineData
 {
-    DisassemblerLine dl;
+    LineData() {}
-    QByteArray address = line["address"].data();
+    LineData(int i, int f) : index(i), function(f) {}
-    dl.address = address.toULongLong(0, 0);
+    int index;
-    dl.data = _(line["inst"].data());
+    int function;
-    dl.function = _(line["func-name"].data());
+};
-    dl.offset = line["offset"].data().toUInt();
-    return dl;
-}
 
-DisassemblerLines GdbEngine::parseMiDisassembler(const GdbMi &lines)
+bool GdbEngine::handleCliDisassemblerResult(const QByteArray &output, DisassemblerAgent *agent)
-{
-    // ^done,data={asm_insns=[src_and_asm_line={line="1243",file=".../app.cpp",
-    // line_asm_insn=[{address="0x08054857",func-name="main",offset="27",
-    // inst="call 0x80545b0 <_Z13testQFileInfov>"}]},
-    // src_and_asm_line={line="1244",file=".../app.cpp",
-    // line_asm_insn=[{address="0x0805485c",func-name="main",offset="32",
-    //inst="call 0x804cba1 <_Z11testObject1v>"}]}]}
-    // - or - (non-Mac)
-    // ^done,asm_insns=[
-    // {address="0x0805acf8",func-name="...",offset="25",inst="and $0xe8,%al"},
-    // {address="0x0805acfa",func-name="...",offset="27",inst="pop %esp"}, ..]
-    // - or - (MAC)
-    // ^done,asm_insns={
-    // {address="0x0d8f69e0",func-name="...",offset="1952",inst="add $0x0,%al"},..}
-
-    DisassemblerLines result;
-
-    // FIXME: Performance?
-    foreach (const GdbMi &child, lines.children()) {
-        if (child.hasName("src_and_asm_line")) {
-            const QString fileName = QFile::decodeName(child["file"].data());
-            const uint line = child["line"].data().toUInt();
-            result.appendSourceLine(fileName, line);
-            GdbMi insn = child["line_asm_insn"];
-            foreach (const GdbMi &item, insn.children())
-                result.appendLine(parseLine(item));
-        } else {
-            // The non-mixed version.
-            result.appendLine(parseLine(child));
-        }
-    }
-    return result;
-}
-
-DisassemblerLines GdbEngine::parseCliDisassembler(const QByteArray &output)
 {
     // First line is something like
     // "Dump of assembler code from 0xb7ff598f to 0xb7ff5a07:"
     DisassemblerLines dlines;
     foreach (const QByteArray &line, output.split('\n'))
         dlines.appendUnparsed(_(line));
-    return dlines;
-}
 
-DisassemblerLines GdbEngine::parseDisassembler(const GdbResponse &response)
+    QVector<DisassemblerLine> lines = dlines.data();
-{
+
-    // Apple's gdb produces MI output even for CLI commands.
+    typedef QMap<quint64, LineData> LineMap;
-    // FIXME: Check whether wrapping this into -interpreter-exec console
+    LineMap lineMap;
-    // (i.e. usgind the 'ConsoleCommand' GdbCommandFlag makes a
+    int currentFunction = -1;
-    // difference.
+    for (int i = 0, n = lines.size(); i != n; ++i) {
-    GdbMi lines = response.data["asm_insns"];
+        const DisassemblerLine &line = lines.at(i);
-    if (lines.isValid())
+        if (line.address)
-        return parseMiDisassembler(lines);
+            lineMap.insert(line.address, LineData(i, currentFunction));
-    return parseCliDisassembler(response.consoleStreamOutput);
+        else
+            currentFunction = i;
+    }
+
+    currentFunction = -1;
+    DisassemblerLines result;
+    for (LineMap::const_iterator it = lineMap.begin(), et = lineMap.end(); it != et; ++it) {
+        LineData d = *it;
+        if (d.function != currentFunction) {
+            if (d.function != -1) {
+                DisassemblerLine &line = lines[d.function];
+                ++line.hunk;
+                result.appendLine(line);
+                currentFunction = d.function;
+            }
+        }
+        result.appendLine(lines.at(d.index));
+    }
+
+    if (result.coversAddress(agent->address())) {
+        agent->setContents(result);
+        return true;
+    }
+
+    return false;
 }
 
 void GdbEngine::reloadDisassembly()
@@ -4171,35 +4087,13 @@ void GdbEngine::handleFetchDisassemblerByCliPointMixed(const GdbResponse &respon
     DisassemblerAgentCookie ac = response.cookie.value<DisassemblerAgentCookie>();
     QTC_ASSERT(ac.agent, return);
 
-    if (response.resultClass == GdbResultDone) {
+    if (response.resultClass == GdbResultDone)
-        DisassemblerLines dlines = parseDisassembler(response);
+        if (handleCliDisassemblerResult(response.consoleStreamOutput, ac.agent))
-        if (dlines.coversAddress(ac.agent->address())) {
-            ac.agent->setContents(dlines);
             return;
-        }
-    }
-    fetchDisassemblerByCliPointPlain(ac);
-}
 
-void GdbEngine::handleFetchDisassemblerByCliPointPlain(const GdbResponse &response)
+    // 'point, plain' can take far too long.
-{
+    // Skip this feature and immediately fall back to the 'range' version:
-    DisassemblerAgentCookie ac = response.cookie.value<DisassemblerAgentCookie>();
+    fetchDisassemblerByCliRangeMixed(ac);
-    QTC_ASSERT(ac.agent, return);
-    // Agent address is 0 when disassembling a function name only
-    const quint64 agentAddress = ac.agent->address();
-    if (response.resultClass == GdbResultDone) {
-        DisassemblerLines dlines = parseDisassembler(response);
-        if (!agentAddress || dlines.coversAddress(agentAddress)) {
-            ac.agent->setContents(dlines);
-            return;
-        }
-    }
-    if (agentAddress) {
-        if (ac.agent->isMixed())
-            fetchDisassemblerByCliRangeMixed(ac);
-        else
-            fetchDisassemblerByCliRangePlain(ac);
-    }
 }
 
 void GdbEngine::handleFetchDisassemblerByCliRangeMixed(const GdbResponse &response)
@@ -4207,13 +4101,10 @@ void GdbEngine::handleFetchDisassemblerByCliRangeMixed(const GdbResponse &respon
     DisassemblerAgentCookie ac = response.cookie.value<DisassemblerAgentCookie>();
     QTC_ASSERT(ac.agent, return);
 
-    if (response.resultClass == GdbResultDone) {
+    if (response.resultClass == GdbResultDone)
-        DisassemblerLines dlines = parseDisassembler(response);
+        if (handleCliDisassemblerResult(response.consoleStreamOutput, ac.agent))
-        if (dlines.coversAddress(ac.agent->address())) {
-            ac.agent->setContents(dlines);
             return;
-        }
+
-    }
     fetchDisassemblerByCliRangePlain(ac);
 }
 
@@ -4222,13 +4113,9 @@ void GdbEngine::handleFetchDisassemblerByCliRangePlain(const GdbResponse &respon
     DisassemblerAgentCookie ac = response.cookie.value<DisassemblerAgentCookie>();
     QTC_ASSERT(ac.agent, return);
 
-    if (response.resultClass == GdbResultDone) {
+    if (response.resultClass == GdbResultDone)
-        DisassemblerLines dlines = parseDisassembler(response);
+        if (handleCliDisassemblerResult(response.consoleStreamOutput, ac.agent))
-        if (dlines.size()) {
-            ac.agent->setContents(dlines);
             return;
-        }
-    }
 
     // Finally, give up.
     //76^error,msg="No function contains program counter for selected..."

src/plugins/debugger/gdb/gdbengine.h

@@ -354,30 +354,18 @@ private: ////////// View & Data Stuff //////////
     // Disassembler specific stuff
     //
     // Chain of fallbacks: PointMixed -> PointPlain -> RangeMixed -> RangePlain.
-    // The Mi versions are not used right now.
     void fetchDisassembler(DisassemblerAgent *agent);
     void fetchDisassemblerByCliPointMixed(const DisassemblerAgentCookie &ac);
-    void fetchDisassemblerByCliPointPlain(const DisassemblerAgentCookie &ac);
     void fetchDisassemblerByCliRangeMixed(const DisassemblerAgentCookie &ac);
     void fetchDisassemblerByCliRangePlain(const DisassemblerAgentCookie &ac);
-    //void fetchDisassemblerByMiPointMixed(const DisassemblerAgentCookie &ac);
-    //void fetchDisassemblerByMiPointPlain(const DisassemblerAgentCookie &ac);
-    //void fetchDisassemblerByMiRangeMixed(const DisassemblerAgentCookie &ac);
-    //void fetchDisassemblerByMiRangePlain(const DisassemblerAgentCookie &ac);
     void handleFetchDisassemblerByCliPointMixed(const GdbResponse &response);
-    void handleFetchDisassemblerByCliPointPlain(const GdbResponse &response);
     void handleFetchDisassemblerByCliRangeMixed(const GdbResponse &response);
     void handleFetchDisassemblerByCliRangePlain(const GdbResponse &response);
-    //void handleFetchDisassemblerByMiPointMixed(const GdbResponse &response);
+    bool handleCliDisassemblerResult(const QByteArray &response, DisassemblerAgent *agent);
-    //void handleFetchDisassemblerByMiPointPlain(const GdbResponse &response);
-    //void handleFetchDisassemblerByMiRangeMixed(const GdbResponse &response);
-    //void handleFetchDisassemblerByMiRangePlain(const GdbResponse &response);
-    void handleBreakOnQFatal(const GdbResponse &response);
-    DisassemblerLines parseDisassembler(const GdbResponse &response);
-    DisassemblerLines parseCliDisassembler(const QByteArray &response);
-    DisassemblerLines parseMiDisassembler(const GdbMi &response);
     Q_SLOT void reloadDisassembly();
 
+    void handleBreakOnQFatal(const GdbResponse &response);
+
     //
     // Source file specific stuff
     //