Retry unwinding when the symbol cache is dirty

mmap()'d binaries can overlap in the profiled application's address
space. For example ld.so usually reserves a huge chunk of address space
for itself, which subsequently gets filled with other libraries.
However, libdw won't reliably accept binaries reported "on top of"
existing modules. In seemingly random cases it will complain that the
modules overlap and reject the new one.

So, when we fail to report a module to Dwfl, we mark the symbol cache
as dirty and retry unwinding at most once after clearing the cache.
Clearing the cache resets libdw's state and allows us to report the new
module first, and unwind symbols from it.

Change-Id: Idb5d85afb39e05c0439206b8d4938b79b6173b2c
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Reviewed-by: Milian Wolff <milian.wolff@kdab.com>

3 files changed, 45 insertions, 16 deletions

diff --git a/app/perfsymboltable.cpp b/app/perfsymboltable.cpp
index 0004f48..9516283 100644
--- a/app/perfsymboltable.cpp
+++ b/app/perfsymboltable.cpp
@@ -34,7 +34,7 @@
 #include <cxxabi.h>
 
 PerfSymbolTable::PerfSymbolTable(quint32 pid, Dwfl_Callbacks *callbacks, PerfUnwind *parent) :
-    m_perfMapFile(QString::fromLatin1("/tmp/perf-%1.map").arg(pid)),
+    m_perfMapFile(QString::fromLatin1("/tmp/perf-%1.map").arg(pid)), m_cacheIsDirty(false),
     m_unwind(parent), m_lastMmapAddedTime(0),
     m_nextMmapOverwrittenTime(std::numeric_limits<quint64>::max()), m_callbacks(callbacks),
     m_pid(pid)
@@ -357,8 +357,11 @@ Dwfl_Module *PerfSymbolTable::reportElf(PerfElfMap::ConstIterator i)
                 m_dwfl, i.value().file.fileName().toLocal8Bit().constData(),
                 i.value().file.absoluteFilePath().toLocal8Bit().constData(), -1, i.key(),
                 false);
-    if (!ret)
+
+    if (!ret) {
         reportError(i, dwfl_errmsg(dwfl_errno()));
+        m_cacheIsDirty = true;
+    }
 
     if (m_lastMmapAddedTime < i->timeAdded)
         m_lastMmapAddedTime = i->timeAdded;
@@ -597,4 +600,6 @@ void PerfSymbolTable::clearCache()
     // Throw out the dwfl state
     dwfl_end(m_dwfl);
     m_dwfl = dwfl_begin(m_callbacks);
+
+    m_cacheIsDirty = false;
 }
diff --git a/app/perfsymboltable.h b/app/perfsymboltable.h
index 3f12f31..113ecfe 100644
--- a/app/perfsymboltable.h
+++ b/app/perfsymboltable.h
@@ -71,6 +71,7 @@ public:
 
     Dwfl *attachDwfl(quint64 timestamp, void *arg);
     void clearCache();
+    bool cacheIsDirty() const { return m_cacheIsDirty; }
 
 private:
 
@@ -82,6 +83,7 @@ private:
     QFile m_perfMapFile;
     QVector<PerfMapSymbol> m_perfMap;
     QHash<Dwarf_Addr, AddressCacheEntry> m_addressCache;
+    bool m_cacheIsDirty;
 
     PerfUnwind *m_unwind;
     Dwfl *m_dwfl;
diff --git a/app/perfunwind.cpp b/app/perfunwind.cpp
index 8dfcc7f..88885d8 100644
--- a/app/perfunwind.cpp
+++ b/app/perfunwind.cpp
@@ -232,10 +232,15 @@ static int frameCallback(Dwfl_Frame *state, void *arg)
 
     Dwarf_Addr pc_adjusted = pc - (isactivation ? 0 : 1);
 
+    auto* symbolTable = ui->unwind->symbolTable(ui->sample->pid());
     // isKernel = false as unwinding generally only works on user code
     bool isInterworking = false;
-    ui->frames.append(ui->unwind->symbolTable(ui->sample->pid())->lookupFrame(
-                          pc_adjusted, ui->sample->time(), false, &isInterworking));
+    const auto frame = symbolTable->lookupFrame(pc_adjusted, ui->sample->time(), false,
+                                                &isInterworking);
+    if (symbolTable->cacheIsDirty())
+        return DWARF_CB_ABORT;
+
+    ui->frames.append(frame);
     if (isInterworking && ui->frames.length() == 1)
         ui->isInterworking = true;
     return DWARF_CB_OK;
@@ -304,6 +309,9 @@ void PerfUnwind::resolveCallchain()
                                               ip, m_currentUnwind.sample->time(), isKernel,
                                               &m_currentUnwind.isInterworking));
         }
+
+        if (symbols->cacheIsDirty())
+            break;
     }
 }
 
@@ -322,23 +330,37 @@ void PerfUnwind::sample(const PerfRecordSample &sample)
 
 void PerfUnwind::analyze(const PerfRecordSample &sample)
 {
-    m_currentUnwind.isInterworking = false;
-    m_currentUnwind.firstGuessedFrame = -1;
-    m_currentUnwind.sample = &sample;
-    m_currentUnwind.frames.clear();
-
     const bool isKernel = ipIsInKernelSpace(sample.ip());
 
     PerfSymbolTable *userSymbols = symbolTable(sample.pid());
-    userSymbols->updatePerfMap();
 
-    // Do this before any lookupFrame() calls; we want to clear the caches if timestamps reset.
-    Dwfl *userDwfl = userSymbols->attachDwfl(sample.time(), &m_currentUnwind);
-    if (sample.callchain().length() > 0)
-        resolveCallchain();
+    for (int unwindingAttempt = 0; unwindingAttempt < 2; ++unwindingAttempt) {
+        m_currentUnwind.isInterworking = false;
+        m_currentUnwind.firstGuessedFrame = -1;
+        m_currentUnwind.sample = &sample;
+        m_currentUnwind.frames.clear();
+
+        userSymbols->updatePerfMap();
 
-    if (userDwfl && sample.registerAbi() != 0 && sample.userStack().length() > 0)
-        unwindStack(userDwfl);
+        Dwfl *userDwfl = userSymbols->attachDwfl(sample.time(), &m_currentUnwind);
+        if (sample.callchain().length() > 0)
+            resolveCallchain();
+
+        // only try to unwind when resolveCallchain did not dirty the cache
+        if (!userSymbols->cacheIsDirty()) {
+            if (userDwfl && sample.registerAbi() != 0 && sample.userStack().length() > 0)
+                unwindStack(userDwfl);
+            else
+                break;
+        }
+
+        // when the cache is dirty, we clean it up and try again, otherwise we can
+        // stop as unwinding should have succeeded
+        if (userSymbols->cacheIsDirty())
+            userSymbols->clearCache(); // fail, try again
+        else
+            break; // success
+    }
 
     // If nothing was found, at least look up the IP
     if (m_currentUnwind.frames.isEmpty()) {