Commit a90e3b38 authored by Harald Sitter's avatar Harald Sitter 🏳🌈
Browse files

fix incorrect parsing of "at foo.so" and empty function name

Summary:
for unknown reasons gdb seems to behave inconsistently here. usually
'at' denotes a file, but there are backtraces where it doesn't
e.g. https://bugs.kde.org/show_bug.cgi?id=416923

I've add a trivial suffix check to ensure parsing of these lines works
correctly even when 'at' is used in combination with a library name.

this is a bit hackish, but in reality this entire parsing tech should
probably be replaced by a python plugin for gdb so we can get interactive
access to the frames and serialize them in a well defined format instead of
having to parse "random" text

on top of that our regex assumed we'd always have a function name, which is
also not true as that bug report shows. to mitigate this the matching group
has been made optional.

this commit also adds a test for the gdb line parsing unit with some
obvious line samples I could find just now

Test Plan: all tests pass

Reviewers: ngraham

Reviewed By: ngraham

Subscribers: ngraham, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D27041
parent 807d9dcc
......@@ -19,19 +19,10 @@
#include "backtraceparser_p.h"
#include "drkonqi_parser_debug.h"
#include <QRegExp>
#include <QFileInfo>
//BEGIN BacktraceLineGdb
class BacktraceLineGdb : public BacktraceLine
{
public:
BacktraceLineGdb(const QString & line);
private:
void parse();
void rate();
};
BacktraceLineGdb::BacktraceLineGdb(const QString & lineStr)
: BacktraceLine()
{
......@@ -60,7 +51,7 @@ void BacktraceLineGdb::parse()
regExp.setPattern(QStringLiteral("^#([0-9]+)" //matches the stack frame number, ex. "#0"
"[\\s]+(0x[0-9a-f]+[\\s]+in[\\s]+)?" // matches " 0x0000dead in " (optionally)
"((\\(anonymous namespace\\)::)?[^\\(]+)" //matches the function name
"((\\(anonymous namespace\\)::)?[^\\(]+)?" //matches the function name
//(anything except left parenthesis, which is the start of the arguments section)
//and optionally the prefix "(anonymous namespace)::"
"(\\(.*\\))?" //matches the function arguments
......@@ -81,7 +72,13 @@ void BacktraceLineGdb::parse()
d->m_functionName = regExp.cap(3).trimmed();
if (!regExp.cap(7).isEmpty()) { //we have file information (stuff after from|at)
if (regExp.cap(8) == QLatin1String("at")) { //'at' means we have a source file
bool file = regExp.cap(8) == QLatin1String("at"); //'at' means we have a source file (likely)
// Gdb isn't entirely consistent here, when it uses 'from' it always refers to a library, but
// sometimes the stack can resolve to a library even when it uses the 'at' key word.
// This specifically seems to happen when a frame has no function name.
const QString path = regExp.cap(9);
file = file && !QFileInfo(path).completeSuffix().contains(QLatin1String(".so"));
if (file) {
d->m_file = regExp.cap(9);
} else { //'from' means we have a library
d->m_library = regExp.cap(9);
......
......@@ -21,6 +21,16 @@
#include "backtraceparser.h"
class BacktraceParserGdbPrivate;
class BacktraceLineGdb : public BacktraceLine
{
public:
BacktraceLineGdb(const QString & line);
private:
void parse();
void rate();
};
class BacktraceParserGdb : public BacktraceParser
{
Q_OBJECT
......
......@@ -24,6 +24,14 @@ add_subdirectory(crashtest)
add_subdirectory(backtraceparsertest)
add_subdirectory(bugzillalibtest)
ecm_add_tests(
gdbbacktracelinetest
LINK_LIBRARIES
Qt5::Core
Qt5::Test
drkonqi_backtrace_parser
)
if(NOT APPLE)
if(NOT RUBY_EXECTUABLE)
find_program(RUBY_EXECUTABLE ruby)
......
/*
Copyright 2020 Harald Sitter <sitter@kde.org>
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
License as published by the Free Software Foundation; either
version 2.1 of the License, or (at your option) version 3, or any
later version accepted by the membership of KDE e.V. (or its
successor approved by the membership of KDE e.V.), which shall
act as a proxy defined in Section 6 of version 3 of the license.
This library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Lesser General Public License for more details.
You should have received a copy of the GNU Lesser General Public
License along with this library. If not, see <http://www.gnu.org/licenses/>.
*/
#include <QTest>
#include "../parser/backtraceparsergdb.h"
class GdbBacktraceLineTest : public QObject
{
Q_OBJECT
private Q_SLOTS:
// rating() is often times somewhat misleading because it is an exclusive state
// but in practise a frame may have multiple issues. for purposes of rating all
// issues are considered equal. it's not ideal though, a frame that is missing
// multiple elements is objectively worse than a frame that is just missing one.
void testComplete()
{
BacktraceLineGdb line(
"#7 0x00007f468b177bfa in KMime::DateFormatterPrivate::localized (t=t@entry=1579263464, shortFormat=shortFormat@entry=true, lang=...) at /usr/src/debug/kmime-19.12.1-lp151.150.1.x86_64/src/kmime_dateformatter.cpp:310\n"
);
QCOMPARE(line.type(), BacktraceLine::StackFrame);
QCOMPARE(line.frameNumber(), 7);
QCOMPARE(line.functionName(), "KMime::DateFormatterPrivate::localized");
QCOMPARE(line.fileName(), "/usr/src/debug/kmime-19.12.1-lp151.150.1.x86_64/src/kmime_dateformatter.cpp:310");
QCOMPARE(line.libraryName(), "");
QCOMPARE(line.rating(), BacktraceLine::Good);
}
void testPoorFile()
{
BacktraceLineGdb line("#41 0x00007f4684ae4e87 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0\n");
QCOMPARE(line.type(), BacktraceLine::StackFrame);
QCOMPARE(line.frameNumber(), 41);
QCOMPARE(line.functionName(), "g_main_context_dispatch");
QCOMPARE(line.fileName(), "");
QCOMPARE(line.libraryName(), "/usr/lib64/libglib-2.0.so.0");
QCOMPARE(line.rating(), BacktraceLine::MissingSourceFile);
}
void testNoFunctionPoorFile()
{
// Also uses 'at' keyword while referring to a library, this used to trip up
// the parser and make it think there's a source file, when in reality there
// is not.
// Lacking a function name further tripped up the parsing because originally
// couldn't deal with the function name being missing entirely.
// As a result this line used to rate as 'Good' -.-
// https://bugs.kde.org/show_bug.cgi?id=416923
BacktraceLineGdb line("#13 0x00007fe6059971b1 in () at /usr/lib/libglib-2.0.so.0\n");
QCOMPARE(line.type(), BacktraceLine::StackFrame);
QCOMPARE(line.frameNumber(), 13);
QCOMPARE(line.functionName(), "");
QCOMPARE(line.fileName(), "");
QCOMPARE(line.libraryName(), "/usr/lib/libglib-2.0.so.0");
QCOMPARE(line.rating(), BacktraceLine::MissingSourceFile);
}
void testOnlyFunctionNofile()
{
BacktraceLineGdb line("#20 0x0000557e978c1b7e in _start ()\n");
QCOMPARE(line.type(), BacktraceLine::StackFrame);
QCOMPARE(line.frameNumber(), 20);
QCOMPARE(line.functionName(), "_start");
QCOMPARE(line.fileName(), "");
QCOMPARE(line.libraryName(), "");
QCOMPARE(line.rating(), BacktraceLine::MissingLibrary);
}
void testInferiorMarker()
{
BacktraceLineGdb line("[Inferior 1 (process 72692) detached]\n");
QCOMPARE(line.type(), BacktraceLine::Unknown);
QCOMPARE(line.rating(), BacktraceLine::InvalidRating);
}
};
QTEST_GUILESS_MAIN(GdbBacktraceLineTest)
#include "gdbbacktracelinetest.moc"
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment