Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable disassembly of inline functions #568

Merged
merged 4 commits into from
Jan 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions src/flamegraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,29 @@ void FrameGraphicsItem::paint(QPainter* painter, const QStyleOptionGraphicsItem*
auto noMatchColor = brush().color();
noMatchColor.setAlpha(50);
painter->fillRect(rect(), noMatchColor);
} else { // default, when no search is running, or a sub-item is matched
painter->fillRect(rect(), brush());
} else {
// default, when no search is running, or a sub-item is matched
auto background = brush();

// give inline frames a slightly different background color
if (m_symbol.isInline) {
auto color = background.color();
if (qGray(pen().color().rgb()) < 128) {
color = color.lighter();
} else {
color = color.darker();
}
background.setColor(color);
}
painter->fillRect(rect(), background);

// give inline frames a border with the normal background color
if (m_symbol.isInline) {
const auto oldPen = painter->pen();
painter->setPen(QPen(brush().color(), 0.));
painter->drawRect(rect().adjusted(-1, -1, -1, -1));
painter->setPen(oldPen);
}
}

const QPen oldPen = painter->pen();
Expand Down Expand Up @@ -240,7 +261,7 @@ QString FrameGraphicsItem::description() const
root = static_cast<const FrameGraphicsRootItem*>(item);
}

auto symbol = Util::formatSymbol(m_symbol);
auto symbol = Util::formatSymbolExtended(m_symbol);
if (root == this) {
return symbol;
}
Expand Down
6 changes: 4 additions & 2 deletions src/models/data.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ QString prettifySymbol(const QString& symbol);
struct Symbol
{
Symbol(const QString& symbol = {}, quint64 relAddr = 0, quint64 size = 0, const QString& binary = {},
const QString& path = {}, const QString& actualPath = {}, bool isKernel = false)
const QString& path = {}, const QString& actualPath = {}, bool isKernel = false, bool isInline = false)
: symbol(symbol)
, prettySymbol(Data::prettifySymbol(symbol))
, relAddr(relAddr)
Expand All @@ -35,6 +35,7 @@ struct Symbol
, path(path)
, actualPath(actualPath)
, isKernel(isKernel)
, isInline(isInline)
{
}

Expand All @@ -53,6 +54,7 @@ struct Symbol
// actual file path
QString actualPath;
bool isKernel = false;
bool isInline = false;

bool operator<(const Symbol& rhs) const
{
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -66,7 +68,7 @@ struct Symbol

bool canDisassemble() const
{
return !symbol.isEmpty() && !path.isEmpty() && relAddr > 0 && size > 0;
return !symbol.isEmpty() && !path.isEmpty() && relAddr > 0 && size > 0 && !isInline;
}
};

Expand Down
11 changes: 7 additions & 4 deletions src/parsers/perf/perfparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,13 @@ struct Symbol
StringId path;
StringId actualPath;
bool isKernel = false;
bool isInline = false;
};

QDataStream& operator>>(QDataStream& stream, Symbol& symbol)
{
return stream >> symbol.name >> symbol.binary >> symbol.path >> symbol.isKernel >> symbol.relAddr >> symbol.size
>> symbol.actualPath;
>> symbol.actualPath >> symbol.isInline;
}

QDebug operator<<(QDebug stream, const Symbol& symbol)
Expand All @@ -245,7 +246,8 @@ QDebug operator<<(QDebug stream, const Symbol& symbol)
<< "binary=" << symbol.binary << ", "
<< "path=" << symbol.path << ", "
<< "actualPath=" << symbol.actualPath << ", "
<< "isKernel=" << symbol.isKernel << "}";
<< "isKernel=" << symbol.isKernel << ", "
<< "isInline=" << symbol.isInline << "}";
return stream;
}

Expand Down Expand Up @@ -1017,8 +1019,9 @@ class PerfParserPrivate : public QObject
const auto pathString = strings.value(symbol.symbol.path.id);
const auto actualPathString = strings.value(symbol.symbol.actualPath.id);
const auto isKernel = symbol.symbol.isKernel;
bottomUpResult.symbols[symbol.id] = {symbolString, relAddr, size, binaryString,
pathString, actualPathString, isKernel};
const auto isInline = symbol.symbol.isInline;
bottomUpResult.symbols[symbol.id] = {symbolString, relAddr, size, binaryString,
pathString, actualPathString, isKernel, isInline};

// Count total and missing symbols per module for error report
auto& numSymbols = numSymbolsByModule[symbol.symbol.binary.id];
Expand Down
11 changes: 10 additions & 1 deletion src/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ std::pair<QString, QString> elideArguments(const QString& symbolText)
QString formatForTooltip(const Data::Symbol& symbol)
{
return QCoreApplication::translate("Util", "symbol: <tt>%1</tt><br/>binary: <tt>%2</tt>")
.arg(Util::formatSymbol(symbol).toHtmlEscaped(), Util::formatString(symbol.binary));
.arg(Util::formatSymbolExtended(symbol).toHtmlEscaped(), Util::formatString(symbol.binary));
}

QString formatTooltipImpl(int id, const QString& text, const Data::Costs* selfCosts, const Data::Costs* inclusiveCosts)
Expand Down Expand Up @@ -266,6 +266,15 @@ QString Util::formatSymbol(const Data::Symbol& symbol, bool replaceEmptyString)
return formatString(symbolString, replaceEmptyString);
}

QString Util::formatSymbolExtended(const Data::Symbol& symbol)
{
auto ret = formatSymbol(symbol);
if (symbol.isInline) {
ret = QCoreApplication::translate("Util", "%1 (inlined)").arg(ret);
}
return ret;
}

QString Util::formatCost(quint64 cost)
{
// resulting format: 1.234E56
Expand Down
1 change: 1 addition & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ struct HashCombine

QString formatString(const QString& input, bool replaceEmptyString = true);
QString formatSymbol(const Data::Symbol& symbol, bool replaceEmptyString = true);
QString formatSymbolExtended(const Data::Symbol& symbol);
QString formatCost(quint64 cost);
QString formatCostRelative(quint64 selfCost, quint64 totalCost, bool addPercentSign = false);
QString formatTimeString(quint64 nanoseconds, bool shortForm = false);
Expand Down
26 changes: 26 additions & 0 deletions tests/modeltests/tst_disassemblyoutput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,32 @@ private slots:
}
}

void testCanDisassemble_data()
{
QTest::addColumn<Data::Symbol>("symbol");
QTest::addColumn<bool>("canDisassemble");

QTest::newRow("normal symbol") << Data::Symbol(QStringLiteral("main"), 0x1159, 0x32805, {},
QStringLiteral("/some/path"), {}, false, false)
<< true;
QTest::newRow("relocated symbol")
<< Data::Symbol(QStringLiteral("printf"), 0, 0, {}, QStringLiteral("/some/path"), {}, false, false)
<< false;
QTest::newRow("inlined symbol") << Data::Symbol(QStringLiteral("main::memcpy"), 0x1159, 0x32805, {},
QStringLiteral("/some/path"), {}, false, true)
<< false;
QTest::newRow("unkown binary") << Data::Symbol(QStringLiteral("main"), 0x1159, 0x32805, {}, {}, {}, false,
false)
<< false;
}

void testCanDisassemble()
{
QFETCH(Data::Symbol, symbol);
QFETCH(bool, canDisassemble);
QCOMPARE(symbol.canDisassemble(), canDisassemble);
}

private:
struct FunctionData
{
Expand Down