report: Include source-and-location into Stacktrace and ThreadStacktrace

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
@@ -1009,8 +1012,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
@@ -1021,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.
@@ -1031,12 +1039,13 @@ 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",
         }
-        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",
@@ -1981,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.
 
@@ -2047,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()

bin/apport-retrace

@@ -19,7 +19,6 @@ import argparse
 import gettext
 import os
 import re
-import shutil
 import subprocess
 import sys
 import tempfile
@@ -185,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",
@@ -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):
@@ -642,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
@@ -659,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:

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

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"])
@@ -741,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()
@@ -769,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:

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(