Debugger: Make disassembler caching smarter.

Check if address is contained in range of lines.

Task-number: QTCREATORBUG-5205
Change-Id: I12a4f2f2f3837e164fd093e80fb427e9234136af
Reviewed-on: http://codereview.qt.nokia.com/1017
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@nokia.com>

src/plugins/debugger/disassembleragent.cpp

@@ -56,6 +56,8 @@
 #include <QtGui/QTextBlock>
 #include <QtGui/QIcon>
 #include <QtCore/QPointer>
+#include <QtCore/QPair>
+#include <QtCore/QDir>
 
 using namespace Core;
 using namespace TextEditor;
@@ -69,6 +71,27 @@ namespace Internal {
 //
 ///////////////////////////////////////////////////////////////////////
 
+class FrameKey
+{
+public:
+    FrameKey() : startAddress(0), endAddress(0) {}
+    inline bool matches(const Location &loc) const;
+
+    QString functionName;
+    QString fileName;
+    quint64 startAddress;
+    quint64 endAddress;
+};
+
+bool FrameKey::matches(const Location &loc) const
+{
+    return loc.address() >= startAddress
+            && loc.address() < endAddress
+            && loc.fileName() == fileName && loc.functionName() == functionName;
+}
+
+typedef QPair<FrameKey, DisassemblerLines> CacheEntry;
+
 class DisassemblerAgentPrivate
 {
 public:
@@ -83,8 +106,9 @@ public:
     QPointer<DebuggerEngine> engine;
     ITextMark *locationMark;
     QList<ITextMark *> breakpointMarks;
-    
-    QHash<QString, DisassemblerLines> cache;
+
+    QList<CacheEntry> cache;
+
     QString mimeType;
     bool m_resetLocationScheduled;
 };
@@ -130,6 +154,14 @@ DisassemblerAgent::~DisassemblerAgent()
     d = 0;
 }
 
+int DisassemblerAgent::indexOf(const Location &loc) const
+{
+    for (int i = 0; i < d->cache.size(); i++)
+        if (d->cache.at(i).first.matches(loc))
+            return i;
+    return -1;
+}
+
 void DisassemblerAgent::cleanup()
 {
     d->cache.clear();
@@ -150,12 +182,6 @@ void DisassemblerAgent::resetLocation()
     }
 }
 
-static QString frameKey(const Location &loc)
-{
-    return _("%1:%2:%3").arg(loc.functionName())
-        .arg(loc.fileName()).arg(loc.address());
-}
-
 const Location &DisassemblerAgent::location() const
 {
     return d->location;
@@ -172,20 +198,28 @@ bool DisassemblerAgent::isMixed() const
 void DisassemblerAgent::setLocation(const Location &loc)
 {
     d->location = loc;
-    if (isMixed()) {
-        QHash<QString, DisassemblerLines>::ConstIterator it =
-            d->cache.find(frameKey(loc));
-        if (it != d->cache.end()) {
-            QString msg = _("Use cache disassembler for '%1' in '%2'")
-                .arg(loc.functionName()).arg(loc.fileName());
-            d->engine->showMessage(msg);
-            setContents(*it);
-            updateBreakpointMarkers();
-            updateLocationMarker();
-            return;
+    int index = indexOf(loc);
+    if (index != -1) {
+        // Refresh when not displaying a function and there is not sufficient
+        // context left past the address.
+        if (!isMixed() && d->cache.at(index).first.endAddress - loc.address() < 24) {
+            index = -1;
+            d->cache.removeAt(index);
         }
     }
-    d->engine->fetchDisassembler(this);
+    if (index != -1) {
+        const FrameKey &key = d->cache.at(index).first;
+        const QString msg =
+            _("Using cached disassembly for 0x%1 (0x%2-0x%3) in '%4'/ '%5'")
+                .arg(loc.address(), 0, 16)
+                .arg(key.startAddress, 0, 16).arg(key.endAddress, 0, 16)
+                .arg(loc.functionName(), QDir::toNativeSeparators(loc.fileName()));
+        d->engine->showMessage(msg);
+        setContentsToEditor(d->cache.at(index).second);
+        d->m_resetLocationScheduled = false; // In case reset from previous run still pending.
+    } else {
+        d->engine->fetchDisassembler(this);
+    }
 }
 
 void DisassemblerAgentPrivate::configureMimeType()
@@ -223,6 +257,24 @@ void DisassemblerAgent::setMimeType(const QString &mt)
 }
 
 void DisassemblerAgent::setContents(const DisassemblerLines &contents)
+{
+    QTC_ASSERT(d, return);
+    if (contents.size()) {
+        const quint64 startAddress = contents.startAddress();
+        const quint64 endAddress = contents.endAddress();
+        if (startAddress) {
+            FrameKey key;
+            key.fileName = d->location.fileName();
+            key.functionName = d->location.functionName();
+            key.startAddress = startAddress;
+            key.endAddress = endAddress;
+            d->cache.append(CacheEntry(key, contents));
+        }
+    }
+    setContentsToEditor(contents);
+}
+
+void DisassemblerAgent::setContentsToEditor(const DisassemblerLines &contents)
 {
     QTC_ASSERT(d, return);
     using namespace Core;
@@ -260,7 +312,6 @@ void DisassemblerAgent::setContents(const DisassemblerLines &contents)
     plainTextEdit->setPlainText(str);
     plainTextEdit->setReadOnly(true);
 
-    d->cache.insert(frameKey(d->location), contents);
     d->editor->setDisplayName(_("Disassembler (%1)")
         .arg(d->location.functionName()));
 
@@ -272,9 +323,10 @@ void DisassemblerAgent::updateLocationMarker()
 {
     QTC_ASSERT(d->editor, return);
 
-    const DisassemblerLines &contents = d->cache.value(frameKey(d->location));
+    const int index = indexOf(d->location);
+    const DisassemblerLines contents = index != -1 ?
+                d->cache.at(index).second : DisassemblerLines();
     int lineNumber = contents.lineForAddress(d->location.address());
-
     if (d->location.needsMarker()) {
         d->editor->markableInterface()->removeMark(d->locationMark);
         if (lineNumber)
@@ -300,8 +352,9 @@ void DisassemblerAgent::updateBreakpointMarkers()
     if (ids.isEmpty())
         return;
 
-    const DisassemblerLines &contents = d->cache.value(frameKey(d->location));
-
+    const int index = indexOf(d->location);
+    const DisassemblerLines contents = index != -1 ?
+                                       d->cache.at(index).second : DisassemblerLines();
     foreach (TextEditor::ITextMark *marker, d->breakpointMarks)
         d->editor->markableInterface()->removeMark(marker);
     d->breakpointMarks.clear();

src/plugins/debugger/disassembleragent.h

@@ -72,6 +72,9 @@ public:
     bool isMixed() const;
 
 private:
+    void setContentsToEditor(const DisassemblerLines &contents);
+    int indexOf(const Location &loc) const;
+
     DisassemblerAgentPrivate *d;
 };
 

src/plugins/debugger/disassemblerlines.cpp

@@ -80,6 +80,22 @@ quint64 DisassemblerLine::addressFromDisassemblyLine(const QString &line)
     return l.address;
 }
 
+quint64 DisassemblerLines::startAddress() const
+{
+    for (int i = 0; i < m_data.size(); ++i)
+        if (m_data.at(i).address)
+            return m_data.at(i).address;
+    return 0;
+}
+
+quint64 DisassemblerLines::endAddress() const
+{
+    for (int i = m_data.size()- 1; i >= 0; --i)
+        if (m_data.at(i).address)
+            return m_data.at(i).address;
+    return 0;
+}
+
 int DisassemblerLines::lineForAddress(quint64 address) const
 {
     return m_rowCache.value(address);

src/plugins/debugger/disassemblerlines.h

@@ -83,6 +83,9 @@ public:
     const DisassemblerLine &at(int i) const { return m_data.at(i); }
     int lineForAddress(quint64 address) const;
 
+    quint64 startAddress() const;
+    quint64 endAddress() const;
+
 private:
     QString m_lastFunction;
     QVector<DisassemblerLine> m_data;