From 564103fb916c2d9cd2ee7160e8f53b7f538e6ee9 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Tue, 10 Oct 2023 14:48:57 +0200 Subject: [PATCH] add setting to disable branches in disassembler sometimes there are too many branches, this patch allows the user to disable them fixes: #516 --- src/models/disassemblymodel.cpp | 4 + src/models/disassemblymodel.h | 1 + src/models/disassemblyoutput.cpp | 20 +++- src/models/disassemblyoutput.h | 1 + src/resultsdisassemblypage.cpp | 9 +- src/settings.cpp | 22 ++++- src/settings.h | 8 ++ src/settingsdialog.cpp | 11 ++- tests/modeltests/tst_disassemblyoutput.cpp | 105 +++++++++++++++++++++ 9 files changed, 171 insertions(+), 10 deletions(-) diff --git a/src/models/disassemblymodel.cpp b/src/models/disassemblymodel.cpp index 1120e4b7..ba3fac82 100644 --- a/src/models/disassemblymodel.cpp +++ b/src/models/disassemblymodel.cpp @@ -80,6 +80,8 @@ QVariant DisassemblyModel::headerData(int section, Qt::Orientation orientation, if (section == AddrColumn) return tr("Address"); + else if (section == BranchColumn) + return tr("Branches"); else if (section == DisassemblyColumn) return tr("Assembly / Disassembly"); @@ -116,6 +118,8 @@ QVariant DisassemblyModel::data(const QModelIndex& index, int role) const if (!data.addr) return {}; return QString::number(data.addr, 16); + } else if (index.column() == BranchColumn) { + return data.branchVisualisation; } else if (index.column() == DisassemblyColumn) { const auto block = m_document->findBlockByLineNumber(index.row()); if (role == SyntaxHighlightRole) diff --git a/src/models/disassemblymodel.h b/src/models/disassemblymodel.h index 1da6f1db..47993d7f 100644 --- a/src/models/disassemblymodel.h +++ b/src/models/disassemblymodel.h @@ -54,6 +54,7 @@ class DisassemblyModel : public QAbstractTableModel enum Columns { AddrColumn, + BranchColumn, DisassemblyColumn, COLUMN_COUNT }; diff --git a/src/models/disassemblyoutput.cpp b/src/models/disassemblyoutput.cpp index 505bc25b..1f36dd5a 100644 --- a/src/models/disassemblyoutput.cpp +++ b/src/models/disassemblyoutput.cpp @@ -120,6 +120,11 @@ QString findBinaryForSymbol(const QStringList& debugPaths, const QStringList& ex return {}; } +bool isHexCharacter(QChar c) +{ + return (c >= QLatin1Char('0') && c <= QLatin1Char('9')) || (c >= QLatin1Char('a') && c <= QLatin1Char('f')); +} + ObjectdumpOutput objdumpParse(const QByteArray& output) { QVector disassemblyLines; @@ -199,8 +204,19 @@ ObjectdumpOutput objdumpParse(const QByteArray& output) assembly = asmLine; } - disassemblyLines.push_back( - {addr, assembly, extractLinkedFunction(asmLine), {currentSourceFileName, sourceCodeLine}}); + // format is the following: + // /- a5 54 12 ... + // | 64 a3 .... + // \-> 65 23 .... + // so we can simply skip all characters until we meet a letter or a number + const auto branchVisualisationRange = + std::distance(assembly.cbegin(), std::find_if(assembly.cbegin(), assembly.cend(), isHexCharacter)); + + disassemblyLines.push_back({addr, + assembly.mid(branchVisualisationRange), + branchVisualisationRange ? assembly.left(branchVisualisationRange) : QString {}, + extractLinkedFunction(asmLine), + {currentSourceFileName, sourceCodeLine}}); } return {disassemblyLines, sourceFileName}; } diff --git a/src/models/disassemblyoutput.h b/src/models/disassemblyoutput.h index d44261f9..d3682f87 100644 --- a/src/models/disassemblyoutput.h +++ b/src/models/disassemblyoutput.h @@ -24,6 +24,7 @@ struct DisassemblyOutput { quint64 addr = 0; QString disassembly; + QString branchVisualisation; LinkedFunction linkedFunction; Data::FileLine fileLine; }; diff --git a/src/resultsdisassemblypage.cpp b/src/resultsdisassemblypage.cpp index a533dbd0..831e1200 100644 --- a/src/resultsdisassemblypage.cpp +++ b/src/resultsdisassemblypage.cpp @@ -101,7 +101,6 @@ ResultsDisassemblyPage::ResultsDisassemblyPage(CostContextMenu* costContextMenu, connect(settings, &Settings::sourceCodePathsChanged, this, [this](const QString&) { showDisassembly(); }); connect(ui->assemblyView, &QTreeView::entered, this, updateFromDisassembly); - connect(ui->sourceCodeView, &QTreeView::entered, this, updateFromSource); ui->sourceCodeView->setContextMenuPolicy(Qt::CustomContextMenu); @@ -253,6 +252,12 @@ ResultsDisassemblyPage::ResultsDisassemblyPage(CostContextMenu* costContextMenu, ui->disasmSearchWidget, ui->disasmSearchEdit, ui->assemblyView, ui->disasmEndReachedWidget, m_disassemblyModel, &m_currentDisasmSearchIndex, 0); + ui->assemblyView->setColumnHidden(DisassemblyModel::BranchColumn, !settings->showBranches()); + + connect(settings, &Settings::showBranchesChanged, this, [this](bool showBranches) { + ui->assemblyView->setColumnHidden(DisassemblyModel::BranchColumn, !showBranches); + }); + #if KFSyntaxHighlighting_FOUND QStringList schemes; @@ -308,7 +313,9 @@ void ResultsDisassemblyPage::setupAsmViewModel() ui->assemblyView->header()->setStretchLastSection(false); ui->assemblyView->header()->setSectionResizeMode(DisassemblyModel::AddrColumn, QHeaderView::ResizeToContents); + ui->assemblyView->header()->setSectionResizeMode(DisassemblyModel::BranchColumn, QHeaderView::ResizeToContents); ui->assemblyView->header()->setSectionResizeMode(DisassemblyModel::DisassemblyColumn, QHeaderView::Stretch); + ui->assemblyView->setItemDelegateForColumn(DisassemblyModel::BranchColumn, m_disassemblyDelegate); ui->assemblyView->setItemDelegateForColumn(DisassemblyModel::DisassemblyColumn, m_disassemblyDelegate); for (int col = DisassemblyModel::COLUMN_COUNT; col < m_disassemblyModel->columnCount(); col++) { diff --git a/src/settings.cpp b/src/settings.cpp index dc2adcc6..03c33a9c 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -191,10 +191,6 @@ void Settings::loadFromFile() sharedConfig->group("PathSettings").writeEntry("userPaths", this->userPaths()); sharedConfig->group("PathSettings").writeEntry("systemPaths", this->systemPaths()); }); - setSourceCodePaths(sharedConfig->group("PathSettings").readEntry("sourceCodePaths", QString())); - connect(this, &Settings::sourceCodePathsChanged, this, [sharedConfig](const QString& paths) { - sharedConfig->group("PathSettings").writeEntry("sourceCodePaths", paths); - }); // fix build error in app image build const auto colorScheme = KColorScheme(QPalette::Normal, KColorScheme::View, sharedConfig); @@ -237,6 +233,16 @@ void Settings::loadFromFile() connect(this, &Settings::lastUsedEnvironmentChanged, this, [sharedConfig](const QString& envName) { sharedConfig->group("PerfPaths").writeEntry("lastUsed", envName); }); + + setSourceCodePaths(sharedConfig->group("Disassembly").readEntry("sourceCodePaths", QString())); + connect(this, &Settings::sourceCodePathsChanged, this, [sharedConfig](const QString& paths) { + sharedConfig->group("Disassembly").writeEntry("sourceCodePaths", paths); + }); + + setShowBranches(sharedConfig->group("Disassembly").readEntry("showBranches", true)); + connect(this, &Settings::showBranchesChanged, [sharedConfig](bool showBranches) { + sharedConfig->group("Disassembly").writeEntry("showBranches", showBranches); + }); } void Settings::setSourceCodePaths(const QString& paths) @@ -254,3 +260,11 @@ void Settings::setPerfPath(const QString& path) emit perfPathChanged(m_perfPath); } } + +void Settings::setShowBranches(bool showBranches) +{ + if (m_showBranches != showBranches) { + m_showBranches = showBranches; + emit showBranchesChanged(m_showBranches); + } +} diff --git a/src/settings.h b/src/settings.h index 065a7fa7..b038a5af 100644 --- a/src/settings.h +++ b/src/settings.h @@ -154,6 +154,11 @@ class Settings : public QObject return m_perfPath; } + bool showBranches() const + { + return m_showBranches; + } + void loadFromFile(); signals: @@ -176,6 +181,7 @@ class Settings : public QObject void lastUsedEnvironmentChanged(const QString& envName); void sourceCodePathsChanged(const QString& paths); void perfPathChanged(const QString& perfPath); + void showBranchesChanged(bool showBranches); public slots: void setPrettifySymbols(bool prettifySymbols); @@ -199,6 +205,7 @@ public slots: void setLastUsedEnvironment(const QString& envName); void setSourceCodePaths(const QString& paths); void setPerfPath(const QString& path); + void setShowBranches(bool showBranches); private: Settings() = default; @@ -222,6 +229,7 @@ public slots: QString m_objdump; QString m_sourceCodePaths; QString m_perfMapPath; + bool m_showBranches = true; QString m_lastUsedEnvironment; diff --git a/src/settingsdialog.cpp b/src/settingsdialog.cpp index 16d7ee18..92b1a73a 100644 --- a/src/settingsdialog.cpp +++ b/src/settingsdialog.cpp @@ -327,13 +327,18 @@ void SettingsDialog::addSourcePathPage() disassemblyPage->setupUi(page); + auto settings = Settings::instance(); + const auto colon = QLatin1Char(':'); - connect(Settings::instance(), &Settings::sourceCodePathsChanged, this, + connect(settings, &Settings::sourceCodePathsChanged, this, [this, colon](const QString& paths) { disassemblyPage->sourcePaths->setItems(paths.split(colon)); }); setupMultiPath(disassemblyPage->sourcePaths, disassemblyPage->label, buttonBox()->button(QDialogButtonBox::Ok)); - connect(buttonBox(), &QDialogButtonBox::accepted, this, [this, colon] { - Settings::instance()->setSourceCodePaths(disassemblyPage->sourcePaths->items().join(colon)); + disassemblyPage->showBranches->setChecked(settings->showBranches()); + + connect(buttonBox(), &QDialogButtonBox::accepted, this, [this, colon, settings] { + settings->setSourceCodePaths(disassemblyPage->sourcePaths->items().join(colon)); + settings->setShowBranches(disassemblyPage->showBranches->isChecked()); }); } diff --git a/tests/modeltests/tst_disassemblyoutput.cpp b/tests/modeltests/tst_disassemblyoutput.cpp index 45a58afd..00228b88 100644 --- a/tests/modeltests/tst_disassemblyoutput.cpp +++ b/tests/modeltests/tst_disassemblyoutput.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -19,6 +20,8 @@ #include #include +#include "../testutils.h" + inline QString findLib(const QString& name) { QFileInfo lib(QCoreApplication::applicationDirPath() + QLatin1String("/../tests/modeltests/%1").arg(name)); @@ -189,6 +192,108 @@ private slots: QCOMPARE(findSourceCodeFile(QStringLiteral("./liba/lib.c"), {tempDir.path()}, QString()), tempDir.path() + QDir::separator() + QStringLiteral("liba/lib.c")); } + + void testDetectBranches() + { + if (!supportsVisualizeJumps()) { + QSKIP("--visualize-jumps is not supported"); + } + + const auto lib = QFileInfo(findLib(QStringLiteral("libfib.so"))); + auto [address, size] = findAddressAndSizeOfFunc(lib.absoluteFilePath(), QStringLiteral("_Z3fibi")); + + const auto symbol = Data::Symbol {QStringLiteral("fib(int)"), address, size, QStringLiteral("libfib.so")}; + + const QString objdump = QStandardPaths::findExecutable(QStringLiteral("objdump")); + QVERIFY(!objdump.isEmpty()); + const auto result = + DisassemblyOutput::disassemble(objdump, {}, QStringList {lib.absolutePath()}, {}, {}, {}, symbol); + QVERIFY(result.errorMessage.isEmpty()); + + auto isValidVisualisationCharacter = [](QChar character) { + const static auto validCharacters = + std::initializer_list {QLatin1Char(' '), QLatin1Char('\t'), QLatin1Char('|'), QLatin1Char('/'), + QLatin1Char('\\'), QLatin1Char('-'), QLatin1Char('>')}; + return std::any_of(validCharacters.begin(), validCharacters.end(), + [character](auto validCharacter) { return character == validCharacter; }); + }; + + for (const auto& line : result.disassemblyLines) { + QVERIFY(!line.branchVisualisation.isEmpty()); + + // check that we only captures valid visualisation characters + QVERIFY(std::all_of(line.branchVisualisation.cbegin(), line.branchVisualisation.cend(), + isValidVisualisationCharacter)); + + QVERIFY(!line.disassembly.isEmpty()); + // check that we removed everyting before the hexdump + bool ok = false; +#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0) + line.disassembly.leftRef(2).toInt(&ok, 16); +#else + line.disassembly.left(2).toInt(&ok, 16); +#endif + QVERIFY(ok); + + // Check that address is valid + QVERIFY(line.addr >= address && line.addr < address + size); + } + } + +private: + struct FunctionData + { + quint64 address; + quint64 size; + }; + static FunctionData findAddressAndSizeOfFunc(const QString& library, const QString& name) + { + QRegularExpression regex(QStringLiteral("[ ]+[0-9]+: ([0-9a-f]+)[ ]+([0-9]+)[0-9 a-zA-Z]+%1\\n").arg(name)); + + auto readelfBinary = QStandardPaths::findExecutable(QStringLiteral("readelf")); + VERIFY_OR_THROW(!readelfBinary.isEmpty()); + + QProcess readelf; + readelf.setProgram(QStringLiteral("readelf")); + readelf.setArguments({QStringLiteral("-s"), library}); + + readelf.start(); + VERIFY_OR_THROW(readelf.waitForFinished()); + + const auto output = readelf.readAllStandardOutput(); + VERIFY_OR_THROW(!output.isEmpty()); + + auto match = regex.match(QString::fromUtf8(output)); + VERIFY_OR_THROW(match.hasMatch()); + + bool ok = false; + const quint64 address = match.captured(1).toULongLong(&ok, 16); + VERIFY_OR_THROW(ok); + const quint64 size = match.captured(2).toULongLong(&ok, 10); + VERIFY_OR_THROW(ok); + return {address, size}; + } + + static bool supportsVisualizeJumps() + { + const QString objdump = QStandardPaths::findExecutable(QStringLiteral("objdump")); + VERIFY_OR_THROW(!objdump.isEmpty()); + + if (objdump.isEmpty()) { + qWarning() << "objdump not found"; + return false; + } + + QProcess process; + process.setProcessChannelMode(QProcess::ForwardedErrorChannel); + process.start(objdump, {QStringLiteral("-H")}); + if (!process.waitForFinished(1000)) { + qWarning() << "failed to query objdump output"; + return false; + } + const auto help = process.readAllStandardOutput(); + return help.contains("--visualize-jumps"); + } }; QTEST_GUILESS_MAIN(TestDisassemblyOutput)