From 8c0ccd5b993af410f5bf7c6f09dd8dce2188fb70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 30 Apr 2026 16:58:02 +0200 Subject: [PATCH 1/5] report: Include source-and-location into Stacktrace and ThreadStacktrace Stacktraces in errors.ubuntu.com and bugs are reported in a way that they provide lines for the specific package version, although sometimes it's not always clear what each call relates to. This is because not all the errors are exposing the actual package version in the main error trace, especially when similar traces are affecting very-different sources versions (and errors.ubuntu.com does not care, since the processes still match). This does not break the current parsing though and we added tests for ensuring this is the case. --- apport/report.py | 11 +++-- doc/data-format.tex | 5 ++- tests/unit/test_apport_retrace.py | 73 +++++++++++++++++++++++++++++++ tests/unit/test_report.py | 50 +++++++++++++++++++++ 4 files changed, 133 insertions(+), 6 deletions(-) diff --git a/apport/report.py b/apport/report.py index c828c8b63..ddf862371 100644 --- a/apport/report.py +++ b/apport/report.py @@ -1009,8 +1009,10 @@ def add_gdb_info( ExecutablePath. This adds the following fields: - Registers: Output of gdb's 'info registers' command - Disassembly: Output of gdb's 'x/16i $pc' command - - Stacktrace: Output of gdb's 'bt full' command - - ThreadStacktrace: Output of gdb's 'thread apply all bt full' command + - Stacktrace: Output of gdb's 'bt -full -frame-info source-and-location' + command + - ThreadStacktrace: Output of gdb's + 'thread apply all bt -full -frame-info source-and-location' command - StacktraceTop: simplified stacktrace (topmost 5 functions) for inline inclusion into bug reports and easier processing - AssertionMessage: Value of __abort_msg or __glib_assert_msg @@ -1031,8 +1033,9 @@ def add_gdb_info( gdb_reports = { "Registers": "info registers", "Disassembly": "x/16i $pc", - "Stacktrace": "bt full", - "ThreadStacktrace": "thread apply all bt full", + "Stacktrace": "bt -full -frame-info source-and-location", + "ThreadStacktrace": "thread apply all bt -full " + + "-frame-info source-and-location", "AssertionMessage": "print __abort_msg->msg", "GLibAssertionMessage": "print (char*) __glib_assert_msg", } diff --git a/doc/data-format.tex b/doc/data-format.tex index 7df319e02..9bbe4894c 100644 --- a/doc/data-format.tex +++ b/doc/data-format.tex @@ -241,10 +241,11 @@ \subsection{Signal crash specific data fields} 'minidump' format or any other useful image of the stack. \item [Stacktrace:] (optional) Stack trace (e. g. produced by gdb's - \verb!bt full! command or minidump processor) + \verb!bt -full -frame-info source-and-location! command or minidump processor) \item [ThreadStacktrace:] (optional) Threaded stack trace (e. g. produced - by the gdb command \verb!thread apply all bt full! or minidump processor) + by the gdb command \verb!thread apply all bt -full + -frame-info source-and-location! or minidump processor) \item [StacktraceTop:] (optional) First five frames of \verb!Stacktrace! with the leading addresses and local variables removed; this is intended to diff --git a/tests/unit/test_apport_retrace.py b/tests/unit/test_apport_retrace.py index 324cbfa3e..35a679c31 100644 --- a/tests/unit/test_apport_retrace.py +++ b/tests/unit/test_apport_retrace.py @@ -1,6 +1,7 @@ """Unit tests for apport-retrace.""" import io +import pathlib import tempfile import unittest import unittest.mock @@ -50,3 +51,75 @@ def test_malformed_kernel_crash_report(get_crashdb_mock: MagicMock) -> None: stderr.getvalue() == "ERROR: report file does not contain the required fields\n" ) get_crashdb_mock.assert_called_once_with(None) + + +@unittest.mock.patch.object(apport_retrace.packaging, "get_source_tree") +def test_gen_source_stacktrace_ignores_frame_info_noise( + get_source_tree_mock: MagicMock, +) -> None: + """Test that gen_source_stacktrace() ignores warnings and "No locals." + lines in the stacktrace output from gdb.""" + with tempfile.TemporaryDirectory() as srcdir: + posix_file = pathlib.Path(srcdir) / "gthread-posix.c" + thread_file = pathlib.Path(srcdir) / "gthread.c" + + posix_file.write_text( + "".join( + ( + f"line {line}\n" + if line != 822 + else "pthread_setname_np (pthread_self (), name_); " + + "/* on Linux and Solaris */\n" + ) + for line in range(1, 831) + ), + encoding="utf-8", + ) + thread_file.write_text( + "".join( + ( + f"line {line}\n" + if line != 889 + else "g_system_thread_set_name (thread->name);\n" + ) + for line in range(1, 896) + ), + encoding="utf-8", + ) + get_source_tree_mock.return_value = srcdir + + report = apport_retrace.Report() + report["SourcePackage"] = "glib2.0" + report["Package"] = "glib2.0 2.78.0" + report["Stacktrace"] = ( + "#2 0x00007ffff7eebd45 in g_system_thread_set_name (" + 'name=0x5555556e2d80 "unref-target2") at ' + "../../glib/glib/gthread-posix.c:822\n" + "822\tpthread_setname_np (pthread_self (), name_); /* on Linux " + "and Solaris */\n" + ' name_ = "unref-target2\\000\\233", \n" + "#3 g_thread_proxy (data=0x5555556e2d60) at " + "../../glib/glib/gthread.c:889\n" + "889\tg_system_thread_set_name (thread->name);\n" + "warning: 889\t../../glib/glib/gthread.c: No such file or " + "directory\n" + "No locals.\n" + ) + + apport_retrace.gen_source_stacktrace(report, sandbox=None) + + stacktrace_source = report["StacktraceSource"] + assert ( + "#2 0x00007ffff7eebd45 in g_system_thread_set_name " + + '(name=0x5555556e2d80 "unref-target2")' + " at ../../glib/glib/gthread-posix.c:822\n" + ) in stacktrace_source + assert ( + "#3 g_thread_proxy (data=0x5555556e2d60) at " + + "../../glib/glib/gthread.c:889\n" + ) in stacktrace_source + assert "822: pthread_setname_np (pthread_self (), name_);" in stacktrace_source + assert "889: g_system_thread_set_name (thread->name);" in stacktrace_source + assert "warning:" not in stacktrace_source + assert "No locals." not in stacktrace_source diff --git a/tests/unit/test_report.py b/tests/unit/test_report.py index bfc6e3b40..53c962844 100644 --- a/tests/unit/test_report.py +++ b/tests/unit/test_report.py @@ -881,6 +881,54 @@ def test_gen_stacktrace_top(self) -> None: d (x=1) at crash.c:29"""), ) + def test_gen_stacktrace_top_with_frame_info_source_lines(self) -> None: + """_gen_stacktrace_top() ignores frame-info source/location noise.""" + r = apport.report.Report() + r["Stacktrace"] = textwrap.dedent( + "#2 0x00007ffff7eebd45 in g_system_thread_set_name (" + 'name=0x5555556e2d80 "unref-target2") at ' + "../../glib/glib/gthread-posix.c:822\n" + "822 pthread_setname_np (pthread_self (), name_); /* on Linux " + "and Solaris */\n" + ' name_ = "unref-target2\\000\\233", \n" + "#3 g_thread_proxy (data=0x5555556e2d60) at " + "../../glib/glib/gthread.c:889\n" + "889 g_system_thread_set_name (thread->name);\n" + " thread = 0x5555556e2d60\n" + ' __func__ = "g_thread_proxy"\n' + "#4 0x00007ffff7aa3be5 in create_thread (pd=pd@entry=0x7ff90da746c0, " + "attr=attr@entry=0x7ffffffdea40, stopped_start=stopped_start@entry=" + "0x7ffffffde946, stackaddr=stackaddr@entry=0x7ff90d274000, " + "stacksize=, thread_ran=thread_ran@entry=" + "0x7ffffffde947) at ./nptl/pthread_create.c:298\n" + "⚠️ warning: 298\t./nptl/pthread_create.c: No such file or directory\n" + "need_setaffinity = false\n" + "clone_flags = 4001536\n" + "tp = 0x7ff90da746c0\n" + "args = {flags = 4001536, pidfd = 140707652651408, child_tid = " + "140707652652264, parent_tid = 140707652651408, exit_signal = 0, " + "stack = 140707644260352, stack_size = 8388480, tls = " + "140707652650688, set_tid = 0, set_tid_size = 0, cgroup = 0}\n" + "ret = \n" + '__PRETTY_FUNCTION__ = "create_thread"\n' + ) + r._gen_stacktrace_top() + self.assertEqual( + r["StacktraceTop"], + textwrap.dedent( + 'g_system_thread_set_name (name=0x5555556e2d80 "unref-target2") ' + "at ../../glib/glib/gthread-posix.c:822\n" + "g_thread_proxy (data=0x5555556e2d60) at " + "../../glib/glib/gthread.c:889\n" + "create_thread (pd=pd@entry=0x7ff90da746c0, " + "attr=attr@entry=0x7ffffffdea40, stopped_start=stopped_start@entry=" + "0x7ffffffde946, stackaddr=stackaddr@entry=0x7ff90d274000, " + "stacksize=, thread_ran=thread_ran@entry=" + "0x7ffffffde947) at ./nptl/pthread_create.c:298" + ), + ) + @unittest.mock.patch("shutil.which", MagicMock(return_value=None)) def test_gdb_add_info_no_gdb(self) -> None: r = apport.report.Report() @@ -1238,9 +1286,11 @@ def test_crash_signature_addresses(self) -> None: # good stack trace pr["Stacktrace"] = """ #0 0x00007f491fac5687 in kill () at ../sysdeps/unix/syscall-template.S:82 +82 return INLINE_SYSCALL_CALL (kill, pid, sig); No locals. #1 0x000000000043fd51 in kill_pid () #2 g_main_context_iterate (context=0x1731680) at gmain.c:3068 +⚠️ warning: 3068\tgmain.c: No such file or directory #3 0x000000000042eb76 in ?? () #4 0x00000000004324d8 in ?? No symbol table info available. From d38910f498e669262c2b98954111874ecb27d911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 6 May 2026 16:05:01 +0200 Subject: [PATCH 2/5] integration/test_report: Split out building a report and adding gdb info It will help in future tests --- tests/integration/test_report.py | 38 +++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/tests/integration/test_report.py b/tests/integration/test_report.py index 95f9014ed..d980edda8 100644 --- a/tests/integration/test_report.py +++ b/tests/integration/test_report.py @@ -638,9 +638,8 @@ def test_check_interpreted_twistd(self) -> None: self.assertIn("UnreportableReason", pr) self.assertEqual(pr["InterpreterPath"], "/usr/bin/twistd") - def _generate_sigsegv_report( + def _build_sigsegv_report( self, - file: IO[bytes] | None = None, signal: str = "11", code: str = """ int f(int x) { @@ -654,7 +653,7 @@ def _generate_sigsegv_report( ) -> apport.report.Report: """Create a test executable which will die with a SIGSEGV, generate a core dump for it, create a problem report with those two arguments - (ExecutablePath and CoreDump) and call add_gdb_info(). + (ExecutablePath and CoreDump). If file is given, the report is written into it. Return the apport.report.Report. @@ -703,16 +702,39 @@ def _generate_sigsegv_report( pr["ExecutablePath"] = os.path.join(workdir, "crash") pr["CoreDump"] = (os.path.join(workdir, "core"),) pr["Signal"] = signal - - pr.add_gdb_info() - if file: - pr.write(file) - file.flush() finally: os.chdir(orig_cwd) return pr + def _generate_sigsegv_report( + self, + file: IO[bytes] | None = None, + signal: str = "11", + code: str = """ +int f(int x) { + int* p = 0; *p = x; + return x+1; +} +int main() { return f(42); } +""", + args: list[str] | None = None, + extra_gcc_args: list[str] | None = None, + ) -> apport.report.Report: + """Create a test executable which will die with a SIGSEGV, generate a + core dump for it, create a problem report with those two arguments + (ExecutablePath and CoreDump) and call add_gdb_info(). + + If file is given, the report is written into it. Return + the apport.report.Report. + """ + pr = self._build_sigsegv_report(signal, code, args, extra_gcc_args) + pr.add_gdb_info() + if file: + pr.write(file) + file.flush() + return pr + @staticmethod def _validate_core(core_path: str) -> None: subprocess.check_call(["sync"]) From 5e0e41ed9f8f963197391518ab91e9d7478dc46b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 6 May 2026 14:29:47 +0200 Subject: [PATCH 3/5] apport/report: Support getting crash report with provided sources --- apport/report.py | 17 ++++++++++++++--- tests/integration/test_report.py | 28 ++++++++++++++++++++++++++++ tests/unit/test_report.py | 26 ++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/apport/report.py b/apport/report.py index ddf862371..46e388f7a 100644 --- a/apport/report.py +++ b/apport/report.py @@ -999,7 +999,10 @@ def add_kernel_crash_info(self) -> bool: return ret def add_gdb_info( - self, rootdir: str | None = None, gdb_sandbox: str | None = None + self, + rootdir: str | None = None, + gdb_sandbox: str | None = None, + gdb_source_dirs: list[str] | None = None, ) -> None: # TODO: Split into smaller functions/methods # pylint: disable=too-complex,too-many-branches,too-many-locals @@ -1023,6 +1026,9 @@ def add_gdb_info( chroot() or root privileges, it just instructs gdb to search for the files there. + The optional gdb_source_dirs can specify source tree roots + that gdb should use for source lookups. + Raises a OSError if the core dump is invalid/truncated, or OSError if calling gdb fails, or FileNotFoundError if gdb or the crashing executable cannot be found. @@ -1039,7 +1045,7 @@ def add_gdb_info( "AssertionMessage": "print __abort_msg->msg", "GLibAssertionMessage": "print (char*) __glib_assert_msg", } - gdb_cmd, environ = self.gdb_command(rootdir, gdb_sandbox) + gdb_cmd, environ = self.gdb_command(rootdir, gdb_sandbox, gdb_source_dirs) environ["HOME"] = "/nonexistent" gdb_cmd += [ "--batch", @@ -1984,7 +1990,10 @@ def get_executable_timestamp(self) -> int | None: return None def gdb_command( - self, sandbox: str | None, gdb_sandbox: str | None = None + self, + sandbox: str | None, + gdb_sandbox: str | None = None, + source_dirs: list[str] | None = None, ) -> tuple[list[str], dict[str, str]]: """Build gdb command for this report. @@ -2050,6 +2059,8 @@ def gdb_command( executable = sandbox + executable command += ["--ex", f'file "{executable}"'] + for source in source_dirs or []: + command += ["--directory", source] if "CoreDump" in self: core = self._provide_uncompressed_coredump_file() diff --git a/tests/integration/test_report.py b/tests/integration/test_report.py index d980edda8..6a458d8d8 100644 --- a/tests/integration/test_report.py +++ b/tests/integration/test_report.py @@ -763,6 +763,20 @@ def _validate_gdb_fields(self, pr: apport.report.Report) -> None: self.assertIn("Thread 1 (", pr["ThreadStacktrace"]) self.assertLessEqual(len(pr["StacktraceTop"].splitlines()), 5) + def _prepare_report_with_external_source( + self, + ) -> tuple[apport.report.Report, str, str]: + source_dir = tempfile.mkdtemp(prefix="apport-source-tree-") + self.addCleanup(shutil.rmtree, source_dir) + + report = self._build_sigsegv_report() + source_path = pathlib.Path(report["ExecutablePath"]).with_name("crash.c") + copied_source = pathlib.Path(source_dir) / "crash.c" + shutil.copy2(source_path, copied_source) + source_path.unlink() + source_line = "3\t int* p = 0; *p = x;" + return report, source_dir, source_line + def test_add_gdb_info(self) -> None: """add_gdb_info() with core dump file reference.""" pr = apport.report.Report() @@ -791,6 +805,20 @@ def test_add_gdb_info(self) -> None: self.assertNotEqual(pr["Disassembly"], "") self.assertNotIn("AssertionMessage", pr) + def test_add_gdb_info_without_source_dir(self) -> None: + """add_gdb_info() has no source lines without gdb_source_dirs.""" + pr, _, source_line = self._prepare_report_with_external_source() + pr.add_gdb_info() + self.assertNotIn(source_line, pr["Stacktrace"]) + self.assertNotIn(source_line, pr["ThreadStacktrace"]) + + def test_add_gdb_info_with_source_dir(self) -> None: + """add_gdb_info() can use a pre-fetched source tree.""" + pr, source_dir, source_line = self._prepare_report_with_external_source() + pr.add_gdb_info(gdb_source_dirs=[source_dir]) + self.assertIn(source_line, pr["Stacktrace"]) + self.assertIn(source_line, pr["ThreadStacktrace"]) + def test_add_gdb_info_load(self) -> None: """add_gdb_info() with inline core dump.""" with tempfile.NamedTemporaryFile() as rep: diff --git a/tests/unit/test_report.py b/tests/unit/test_report.py index 53c962844..27cc2a64e 100644 --- a/tests/unit/test_report.py +++ b/tests/unit/test_report.py @@ -940,6 +940,32 @@ def test_gdb_add_info_no_gdb(self) -> None: with self.assertRaises(FileNotFoundError): r.add_gdb_info() + @unittest.mock.patch( + "apport.report._get_gdb_path", MagicMock(return_value="/usr/bin/gdb") + ) + def test_gdb_command_with_source_dir(self) -> None: + report = apport.report.Report() + report["ExecutablePath"] = "/bin/true" + + command, _ = report.gdb_command(None, None, ["/tmp/source-tree"]) + self.assertIn("--directory /tmp/source-tree", " ".join(command)) + + command, _ = report.gdb_command( + None, None, ["/tmp/source-tree", "/tmp/another-source-tree"] + ) + self.assertIn("--directory /tmp/source-tree", " ".join(command)) + self.assertIn("--directory /tmp/another-source-tree", " ".join(command)) + + @unittest.mock.patch( + "apport.report._get_gdb_path", MagicMock(return_value="/usr/bin/gdb") + ) + def test_gdb_command_without_source_dir(self) -> None: + report = apport.report.Report() + report["ExecutablePath"] = "/bin/true" + + command, _ = report.gdb_command(None, None, None) + self.assertNotIn("--directory", command) + def test_crash_signature(self) -> None: """crash_signature().""" r = apport.report.Report() From 9a1892c7e43a267ffbaa0f079515a1aa5d254939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 6 May 2026 15:10:53 +0200 Subject: [PATCH 4/5] apport-retrace: Split out source fetching from source stack-trace So we can have a way to fetch the sources without actually doing any processing of them --- bin/apport-retrace | 46 ++++++++++++++++++++++++------- tests/unit/test_apport_retrace.py | 43 +++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/bin/apport-retrace b/bin/apport-retrace index f6d60bac8..9229b6a3d 100755 --- a/bin/apport-retrace +++ b/bin/apport-retrace @@ -19,7 +19,6 @@ import argparse import gettext import os import re -import shutil import subprocess import sys import tempfile @@ -273,24 +272,50 @@ def get_code(srcdir, filename, line, context=5): return result -def gen_source_stacktrace(report, sandbox): - """Generate StacktraceSource. +def _get_source_directory( + report: dict[str, str], sandbox: str | None +) -> tuple[str | None, tempfile.TemporaryDirectory | None]: + """Return source tree path and temporary work dir if any or ``(None, None)``. - This is a version of Stacktrace with the surrounding code lines (where - available) and with local variables removed. + The returned temporary directory object must be cleaned up by the caller. """ - if "Stacktrace" not in report or "SourcePackage" not in report: - return + if "SourcePackage" not in report: + return None, None - workdir = tempfile.mkdtemp() + # pylint: disable=R1732 # We do not want automatic cleanup for this. + workdir = tempfile.TemporaryDirectory() try: try: version = report["Package"].split()[1] except (IndexError, KeyError): version = None srcdir = packaging.get_source_tree( - report["SourcePackage"], workdir, version, sandbox=sandbox + report["SourcePackage"], workdir.name, version, sandbox=sandbox ) + if not srcdir: + workdir.cleanup() + return None, None + return srcdir, workdir + except Exception: + workdir.cleanup() + raise + + +def gen_source_stacktrace( + report: dict[str, str], sandbox: str | None, srcdir: str | None = None +) -> None: + """Generate StacktraceSource. + + This is a version of Stacktrace with the surrounding code lines (where + available) and with local variables removed. + """ + if "Stacktrace" not in report or "SourcePackage" not in report: + return + + workdir = None + try: + if not srcdir: + srcdir, workdir = _get_source_directory(report, sandbox) if not srcdir: return @@ -309,7 +334,8 @@ def gen_source_stacktrace(report, sandbox): report["StacktraceSource"] = result finally: - shutil.rmtree(workdir) + if workdir: + workdir.cleanup() def print_traces(report): diff --git a/tests/unit/test_apport_retrace.py b/tests/unit/test_apport_retrace.py index 35a679c31..6bdb8135a 100644 --- a/tests/unit/test_apport_retrace.py +++ b/tests/unit/test_apport_retrace.py @@ -123,3 +123,46 @@ def test_gen_source_stacktrace_ignores_frame_info_noise( assert "889: g_system_thread_set_name (thread->name);" in stacktrace_source assert "warning:" not in stacktrace_source assert "No locals." not in stacktrace_source + + +@unittest.mock.patch.object(apport_retrace.packaging, "get_source_tree") +def test_gen_source_stacktrace_with_preloaded_source_tree( + get_source_tree_mock: MagicMock, +) -> None: + """Test reusing a provided source tree without re-fetching sources.""" + with tempfile.TemporaryDirectory() as srcdir: + pathlib.Path(srcdir, "example.c").write_text( + "".join( + "crash_happens_here();\n" if line == 10 else f"line {line}\n" + for line in range(1, 20) + ), + encoding="utf-8", + ) + report = apport_retrace.Report() + report["SourcePackage"] = "example" + report["Package"] = "example 1.0" + report["Stacktrace"] = "#0 0x1 in fn () at /build/example.c:10\n" + + apport_retrace.gen_source_stacktrace(report, sandbox=None, srcdir=srcdir) + + assert report["StacktraceSource"].startswith( + "#0 0x1 in fn () at /build/example.c:10\n" + ) + assert "10: crash_happens_here();" in report["StacktraceSource"] + get_source_tree_mock.assert_not_called() + + +@unittest.mock.patch.object(apport_retrace.packaging, "get_source_tree") +def test_gen_source_stacktrace_without_available_source_tree( + get_source_tree_mock: MagicMock, +) -> None: + """Test skipping StacktraceSource when source fetching returns no tree.""" + get_source_tree_mock.return_value = None + report = apport_retrace.Report() + report["SourcePackage"] = "example" + report["Package"] = "example 1.0" + report["Stacktrace"] = "#0 0x1 in fn () at /build/example.c:10\n" + + apport_retrace.gen_source_stacktrace(report, sandbox=None) + + assert "StacktraceSource" not in report From 5473074fcac237df9e23160efbae98119c00be16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 6 May 2026 15:28:58 +0200 Subject: [PATCH 5/5] apport-retrace: Get the source tree early and use it for gdb reports In order to fully take advantage of -format source-and-location in gdb we do need to provide to gdb the source tree. So unless the --no-stacktrace-source option is provided, try to get the package source before running gdb, so that we can properly generate reports with source information. This does not change the behavior of the tool, given that the source download step was already happening by default, but we are doing it earlier now. `StacktraceSource` field is now not particularly interesting but I left it round since it still provides a different format than `Stacktrace`. Also I did not add a different option to control this, as introducing a second switch for source use in Stacktrace and ThreadedStacktrace would create overlapping controls and ambiguous combinations without adding practical value --- bin/apport-retrace | 47 ++++--- tests/system/test_apport_retrace.py | 10 ++ tests/unit/test_apport_retrace.py | 190 ++++++++++++++++++++++++++++ 3 files changed, 229 insertions(+), 18 deletions(-) diff --git a/bin/apport-retrace b/bin/apport-retrace index 9229b6a3d..5824f6f46 100755 --- a/bin/apport-retrace +++ b/bin/apport-retrace @@ -184,7 +184,7 @@ def parse_args(argv: list[str]) -> argparse.Namespace: "--no-stacktrace-source", action="store_false", dest="stacktrace_source", - help=_("Do not add StacktraceSource to the report."), + help=_("Do not fetch sources or add StacktraceSource to the report."), ) argparser.add_argument( "report", @@ -668,15 +668,29 @@ def main(argv): apport.logging.memdbg("before calling gdb") subprocess.call(gdb_cmd, env=os.environ | environ) else: + if options.sandbox == "system": + apt_root = os.path.join(cache, "system", "apt") + elif options.sandbox: + apt_root = os.path.join(cache, report["DistroRelease"], "apt") + else: + apt_root = None + source_dir = None + source_workdir = None + if options.stacktrace_source: + source_dir, source_workdir = _get_source_directory(report, apt_root) + # regenerate gdb info apport.logging.memdbg("before collecting gdb info") try: - report.add_gdb_info(sandbox, gdb_sandbox) - except OSError as error: - if not options.auth: - apport.logging.fatal("%s", str(error)) - if not options.confirm or confirm_traces(report): - invalid_msg = """Thank you for your report! + try: + report.add_gdb_info( + sandbox, gdb_sandbox, [source_dir] if source_dir else None + ) + except OSError as error: + if not options.auth: + apport.logging.fatal("%s", str(error)) + if not options.confirm or confirm_traces(report): + invalid_msg = """Thank you for your report! However, processing it in order to get sufficient information for the developers failed as the report has a core dump which is invalid. The @@ -685,17 +699,14 @@ transit. Thank you for your understanding, and sorry for the inconvenience! """ - crashdb.mark_retrace_failed(crashid, invalid_msg) - apport.logging.fatal("%s", str(error)) - if options.sandbox == "system": - apt_root = os.path.join(cache, "system", "apt") - elif options.sandbox: - apt_root = os.path.join(cache, report["DistroRelease"], "apt") - else: - apt_root = None - if options.stacktrace_source: - gen_source_stacktrace(report, apt_root) - report.add_kernel_crash_info() + crashdb.mark_retrace_failed(crashid, invalid_msg) + apport.logging.fatal("%s", str(error)) + if options.stacktrace_source: + gen_source_stacktrace(report, apt_root, source_dir) + report.add_kernel_crash_info() + finally: + if source_workdir: + source_workdir.cleanup() # Cleanup the .dwz machine symlink for LP: #1818918 if gdb_sandbox and sandbox and target: diff --git a/tests/system/test_apport_retrace.py b/tests/system/test_apport_retrace.py index 328a4bb05..57871c297 100644 --- a/tests/system/test_apport_retrace.py +++ b/tests/system/test_apport_retrace.py @@ -161,14 +161,22 @@ def _assert_divide_by_zero_retrace(report: Report) -> None: r" at (/usr/src/chaos-marmosets-[^/]+/|\./)?divide-by-zero.c:[0-9]+$", flags=re.M, ) + return_line = " return 42 / zero;\n" + source_line_regex = re.compile(rf"\n[0-9]+\t{re.escape(return_line)}") + printf_line = ' printf("42 / 0 = %i\\n", divide_by_zero());\n' + printf_line_regex = re.compile(rf"\n[0-9]+\t{re.escape(printf_line)}") assert "divide_by_zero" in report["Disassembly"] # Expect RIP point to divide_by_zero assert "divide_by_zero" in report["Registers"] assert frame_regex.match(report["Stacktrace"]) assert frame_regex.match(report["StacktraceSource"]) assert "42 / zero" in report["StacktraceSource"] + assert source_line_regex.search(report["Stacktrace"]) + assert printf_line_regex.search(report["Stacktrace"]) assert stack_regex.match(report["StacktraceTop"]) assert frame_regex.search(report["ThreadStacktrace"]) + assert source_line_regex.search(report["ThreadStacktrace"]) + assert printf_line_regex.search(report["ThreadStacktrace"]) def _assert_sleep_retrace(report: Report) -> None: @@ -178,11 +186,13 @@ def _assert_sleep_retrace(report: Report) -> None: assert "__GI___clock_nanosleep" in report["Registers"] assert stack_top in report["Stacktrace"] assert "seconds = 86400" in report["Stacktrace"] + assert "return nanosleep" in report["Stacktrace"] assert stack_top in report["StacktraceSource"] assert "return nanosleep" in report["StacktraceSource"] assert "__GI___clock_nanosleep (clock_id=" in report["StacktraceTop"] assert stack_top in report["ThreadStacktrace"] assert "seconds = 86400" in report["ThreadStacktrace"] + assert "return nanosleep" in report["ThreadStacktrace"] def _assert_cache_has_content( diff --git a/tests/unit/test_apport_retrace.py b/tests/unit/test_apport_retrace.py index 6bdb8135a..eb749fa5f 100644 --- a/tests/unit/test_apport_retrace.py +++ b/tests/unit/test_apport_retrace.py @@ -1,5 +1,6 @@ """Unit tests for apport-retrace.""" +import argparse import io import pathlib import tempfile @@ -166,3 +167,192 @@ def test_gen_source_stacktrace_without_available_source_tree( apport_retrace.gen_source_stacktrace(report, sandbox=None) assert "StacktraceSource" not in report + + +@unittest.mock.patch.object(apport_retrace, "parse_args") +@unittest.mock.patch.object(apport_retrace, "get_crashdb") +@unittest.mock.patch.object(apport_retrace, "load_report") +@unittest.mock.patch.object(apport_retrace, "_get_source_directory") +@unittest.mock.patch.object(apport_retrace, "gen_source_stacktrace") +@unittest.mock.patch.object(apport_retrace, "print_traces") +def test_main_fetches_sources_before_gdb( + print_traces_mock: MagicMock, + gen_source_stacktrace_mock: MagicMock, + get_source_tree_mock: MagicMock, + load_report_mock: MagicMock, + get_crashdb_mock: MagicMock, + parse_args_mock: MagicMock, +) -> None: + """Test prefetching sources before gdb and reusing them for source output.""" + parse_args_mock.return_value = argparse.Namespace( + report="some_binary.crash", + auth=None, + core_file=None, + executable=None, + procmaps=None, + rebuild_package_info=False, + gdb_sandbox=False, + sandbox=None, + sandbox_dir=None, + extra_package=[], + cache=None, + verbose=False, + timestamps=False, + dynamic_origins=False, + gdb=False, + # source stacktrace, so source tree should be pre-fetched. + stacktrace_source=True, + confirm=False, + stdout=True, + remove_core=False, + output=None, + duplicate_db=None, + ) + get_crashdb_mock.return_value = MagicMock() + + report = apport_retrace.Report() + report["ProblemType"] = "Crash" + report["CoreDump"] = ("/tmp/core",) + report["ExecutablePath"] = "/bin/true" + report["Package"] = "coreutils 1.0" + report["DistroRelease"] = "Ubuntu 26.04" + report["Architecture"] = "amd64" + report.add_gdb_info = MagicMock() + report.add_kernel_crash_info = MagicMock() + load_report_mock.return_value = (report, None) + + source_workdir = MagicMock() + get_source_tree_mock.return_value = ("/tmp/src-tree", source_workdir) + + assert apport_retrace.main([]) == 0 + + get_source_tree_mock.assert_called_once_with(report, None) + report.add_gdb_info.assert_called_once_with(None, None, ["/tmp/src-tree"]) + gen_source_stacktrace_mock.assert_called_once_with(report, None, "/tmp/src-tree") + report.add_kernel_crash_info.assert_called_once_with() + source_workdir.cleanup.assert_called_once_with() + print_traces_mock.assert_called_once_with(report) + + +@unittest.mock.patch.object(apport_retrace, "parse_args") +@unittest.mock.patch.object(apport_retrace, "get_crashdb") +@unittest.mock.patch.object(apport_retrace, "load_report") +@unittest.mock.patch.object(apport_retrace, "_get_source_directory") +@unittest.mock.patch.object(apport_retrace, "gen_source_stacktrace") +@unittest.mock.patch.object(apport_retrace, "print_traces") +def test_main_with_unavailable_source_tree( + print_traces_mock: MagicMock, + gen_source_stacktrace_mock: MagicMock, + get_source_tree_mock: MagicMock, + load_report_mock: MagicMock, + get_crashdb_mock: MagicMock, + parse_args_mock: MagicMock, +) -> None: + """Test retracing when source prefetch is enabled but unavailable.""" + parse_args_mock.return_value = argparse.Namespace( + report="some_binary.crash", + auth=None, + core_file=None, + executable=None, + procmaps=None, + rebuild_package_info=False, + gdb_sandbox=False, + sandbox=None, + sandbox_dir=None, + extra_package=[], + cache=None, + verbose=False, + timestamps=False, + dynamic_origins=False, + gdb=False, + stacktrace_source=True, + confirm=False, + stdout=True, + remove_core=False, + output=None, + duplicate_db=None, + ) + get_crashdb_mock.return_value = MagicMock() + + report = apport_retrace.Report() + report["ProblemType"] = "Crash" + report["CoreDump"] = ("/tmp/core",) + report["ExecutablePath"] = "/bin/true" + report["Package"] = "coreutils 1.0" + report["DistroRelease"] = "Ubuntu 24.04" + report["Architecture"] = "amd64" + report.add_gdb_info = MagicMock() + report.add_kernel_crash_info = MagicMock() + load_report_mock.return_value = (report, None) + + # source retrieval enabled but unavailable + get_source_tree_mock.return_value = (None, None) + + assert apport_retrace.main([]) == 0 + + get_source_tree_mock.assert_called_once_with(report, None) + report.add_gdb_info.assert_called_once_with(None, None, None) + gen_source_stacktrace_mock.assert_called_once_with(report, None, None) + report.add_kernel_crash_info.assert_called_once_with() + print_traces_mock.assert_called_once_with(report) + + +@unittest.mock.patch.object(apport_retrace, "parse_args") +@unittest.mock.patch.object(apport_retrace, "get_crashdb") +@unittest.mock.patch.object(apport_retrace, "load_report") +@unittest.mock.patch.object(apport_retrace, "_get_source_directory") +@unittest.mock.patch.object(apport_retrace, "gen_source_stacktrace") +@unittest.mock.patch.object(apport_retrace, "print_traces") +def test_main_without_stacktrace_source_does_not_prefetch_source( + print_traces_mock: MagicMock, + gen_source_stacktrace_mock: MagicMock, + get_source_tree_mock: MagicMock, + load_report_mock: MagicMock, + get_crashdb_mock: MagicMock, + parse_args_mock: MagicMock, +) -> None: + """Test that disabling stacktrace source skips source prefetch entirely.""" + parse_args_mock.return_value = argparse.Namespace( + report="some_binary.crash", + auth=None, + core_file=None, + executable=None, + procmaps=None, + rebuild_package_info=False, + gdb_sandbox=False, + sandbox=None, + sandbox_dir=None, + extra_package=[], + cache=None, + verbose=False, + timestamps=False, + dynamic_origins=False, + gdb=False, + # No source stacktrace, so source tree should not be pre-fetched. + stacktrace_source=False, + confirm=False, + stdout=True, + remove_core=False, + output=None, + duplicate_db=None, + ) + get_crashdb_mock.return_value = MagicMock() + + report = apport_retrace.Report() + report["ProblemType"] = "Crash" + report["CoreDump"] = ("/tmp/core",) + report["ExecutablePath"] = "/bin/true" + report["Package"] = "coreutils 1.0" + report["DistroRelease"] = "Ubuntu 24.04" + report["Architecture"] = "amd64" + report.add_gdb_info = MagicMock() + report.add_kernel_crash_info = MagicMock() + load_report_mock.return_value = (report, None) + + assert apport_retrace.main([]) == 0 + + get_source_tree_mock.assert_not_called() + report.add_gdb_info.assert_called_once_with(None, None, None) + gen_source_stacktrace_mock.assert_not_called() + report.add_kernel_crash_info.assert_called_once_with() + print_traces_mock.assert_called_once_with(report)