fix: handle minidumps missing crashing thread data

tysmith · tysmith · commit d27555298b19 · 2025-05-06T17:29:53.000-07:00
diff --git a/src/ffpuppet/core.py b/src/ffpuppet/core.py
@@ -533,11 +533,17 @@ def close(self, force_close: bool = False) -> None:
                     MDSW_URL,
                 )
             elif dmp_files:
+                # use SAVE_DMP to include dmp files in report
                 if getenv("SAVE_DMP") == "1":
                     # save minidump directory contents (.dmp and .extra files)
                     dmps = self._logs.add_path("minidumps")
                     for md_file in (self.profile.path / "minidumps").glob("*"):
                         copy(md_file, dmps)
+                # use DEBUG_DMP to copy dmp files to a known location
+                debug_dmp = Path(getenv("DEBUG_DMP", ""))
+                if debug_dmp.is_dir():
+                    for md_file in (self.profile.path / "minidumps").glob("*"):
+                        copy(md_file, debug_dmp)
                 # check for local build symbols
                 if (self._bin_path.parent / "crashreporter-symbols").is_dir():
                     sym_path = self._bin_path.parent / "crashreporter-symbols"
diff --git a/src/ffpuppet/minidump_parser.py b/src/ffpuppet/minidump_parser.py
@@ -75,14 +75,19 @@ def _fmt_output(data: dict[str, Any], out_fp: IO[bytes], limit: int = 150) -> No
         """
         assert limit > 0
         # generate register information lines
-        frames = data["crashing_thread"]["frames"]
-        reg_lines: list[str] = []
-        for reg, value in frames[0]["registers"].items():
-            # display three registers per line
-            sep = "\t" if (len(reg_lines) + 1) % 3 else "\n"
-            reg_lines.append(f"{reg:>3} = {value}{sep}")
-        out_fp.write("".join(reg_lines).rstrip().encode())
-        out_fp.write(b"\n")
+        try:
+            frames = data["crashing_thread"]["frames"]
+        except KeyError:
+            LOG.warning("No frames available for 'crashing thread'")
+            frames = []
+        if frames:
+            reg_lines: list[str] = []
+            for reg, value in frames[0]["registers"].items():
+                # display three registers per line
+                sep = "\t" if (len(reg_lines) + 1) % 3 else "\n"
+                reg_lines.append(f"{reg:>3} = {value}{sep}")
+            out_fp.write("".join(reg_lines).rstrip().encode())
+            out_fp.write(b"\n")
 
         # generate OS information line
         line = "|".join(
@@ -104,7 +109,7 @@ def _fmt_output(data: dict[str, Any], out_fp: IO[bytes], limit: int = 150) -> No
         out_fp.write(b"\n")
 
         # generate Crash information line
-        crashing_thread = str(data["crash_info"]["crashing_thread"])
+        crashing_thread = str(data["crash_info"].get("crashing_thread", "?"))
         line = "|".join(
             (
                 "Crash",
diff --git a/src/ffpuppet/test_ffpuppet.py b/src/ffpuppet/test_ffpuppet.py
@@ -526,7 +526,9 @@ def _fake_create_log(src, filename, _timeout: int = 90):
         (dst / filename).write_text(src.read_text())
         return dst / filename
 
-    mocker.patch("ffpuppet.core.getenv", return_value="1")
+    debug_dmps = tmp_path / "debug_dmps"
+    debug_dmps.mkdir()
+    mocker.patch("ffpuppet.core.getenv", side_effect=("1", str(debug_dmps), "1"))
     mocker.patch.object(MinidumpParser, "mdsw_available", return_value=mdsw_available)
     mocker.patch.object(MinidumpParser, "create_log", side_effect=_fake_create_log)
     profile = tmp_path / "profile"
@@ -555,6 +557,9 @@ def _fake_create_log(src, filename, _timeout: int = 90):
             assert len(tuple((logs / "minidumps").glob("*"))) == 3
         else:
             assert not any(logs.glob("log_minidump_*"))
+    if mdsw_available:
+        # files should still be available after ffp cleanup
+        assert len(tuple(debug_dmps.iterdir())) == 3
 
 
 def test_ffpuppet_22(tmp_path):
diff --git a/src/ffpuppet/test_minidump_parser.py b/src/ffpuppet/test_minidump_parser.py
@@ -3,6 +3,7 @@
 # You can obtain one at http://mozilla.org/MPL/2.0/.
 """ffpuppet minidump parser tests"""
 
+from copy import deepcopy
 from json import dumps
 from pathlib import Path
 from subprocess import CompletedProcess
@@ -12,26 +13,11 @@
 
 from .minidump_parser import MinidumpParser
 
-MD_UNSYMBOLIZED_AMD64_WIN = {
+MD_BASE_AMD64_WIN = {
     "crash_info": {
         "address": "0x00007ffe4e09af8d",
-        "crashing_thread": 0,
         "type": "EXCEPTION_BREAKPOINT",
     },
-    "crashing_thread": {
-        "frame_count": 49,
-        "frames": [
-            {
-                "file": None,
-                "frame": 0,
-                "function": None,
-                "function_offset": None,
-                "line": None,
-                "module": "xul.dll",
-                "registers": {"r10": "0x0"},
-            },
-        ],
-    },
     "system_info": {
         "cpu_arch": "amd64",
         "cpu_count": 8,
@@ -41,6 +27,23 @@
     },
 }
 
+MD_UNSYMBOLIZED_AMD64_WIN = deepcopy(MD_BASE_AMD64_WIN)
+MD_UNSYMBOLIZED_AMD64_WIN["crash_info"]["crashing_thread"] = 0
+MD_UNSYMBOLIZED_AMD64_WIN["crashing_thread"] = {
+    "frame_count": 49,
+    "frames": [
+        {
+            "file": None,
+            "frame": 0,
+            "function": None,
+            "function_offset": None,
+            "line": None,
+            "module": "xul.dll",
+            "registers": {"r10": "0x0"},
+        },
+    ],
+}
+
 MD_UNSYMBOLIZED_ARM64_MAC = {
     "crash_info": {
         "address": "0x0000000000000000",
@@ -165,54 +168,43 @@ def test_minidump_parser_03(tmp_path, data, reg, operating_system, cpu, crash, f
 
 def test_minidump_parser_04(tmp_path):
     """test MinidumpParser._fmt_output() - symbolized"""
-    data = {
-        "crash_info": {
-            "address": "0x00007ffe4e09af8d",
-            "crashing_thread": 0,
-            "type": "EXCEPTION_BREAKPOINT",
-        },
-        "crashing_thread": {
-            "frames": [
-                {
-                    "file": "file0.cpp",
-                    "frame": 0,
-                    "function": "function00()",
-                    "function_offset": "0x00000000000001ed",
-                    "line": 47,
-                    "module": "xul.dll",
-                    "registers": {
-                        "r10": "0x12345678",
-                        "r11": "0x0badf00d",
-                        "r12": "0x00000000",
-                        "r13": "0x000000dceebfc2e8",
-                    },
-                },
-                {
-                    "file": "file1.cpp",
-                    "frame": 1,
-                    "function": "function01()",
-                    "function_offset": "0x00000000000001bb",
-                    "line": 210,
-                    "module": "xul.dll",
-                },
-                {
-                    "file": "file2.cpp",
-                    "frame": 2,
-                    "function": "function02()",
-                    "function_offset": "0x0000000000000123",
-                    "line": 123,
-                    "module": "xul.dll",
+    data = deepcopy(MD_BASE_AMD64_WIN)
+    data["crash_info"]["crashing_thread"] = 0
+    data["crashing_thread"] = {
+        "frames": [
+            {
+                "file": "file0.cpp",
+                "frame": 0,
+                "function": "function00()",
+                "function_offset": "0x00000000000001ed",
+                "line": 47,
+                "module": "xul.dll",
+                "registers": {
+                    "r10": "0x12345678",
+                    "r11": "0x0badf00d",
+                    "r12": "0x00000000",
+                    "r13": "0x000000dceebfc2e8",
                 },
-            ],
-        },
-        "system_info": {
-            "cpu_arch": "amd64",
-            "cpu_count": 8,
-            "cpu_info": "family 6 model 70 stepping 1",
-            "os": "Windows NT",
-            "os_ver": "10.0.19044",
-        },
+            },
+            {
+                "file": "file1.cpp",
+                "frame": 1,
+                "function": "function01()",
+                "function_offset": "0x00000000000001bb",
+                "line": 210,
+                "module": "xul.dll",
+            },
+            {
+                "file": "file2.cpp",
+                "frame": 2,
+                "function": "function02()",
+                "function_offset": "0x0000000000000123",
+                "line": 123,
+                "module": "xul.dll",
+            },
+        ],
     }
+
     with (tmp_path / "out.txt").open("w+b") as ofp:
         # pylint: disable=protected-access
         MinidumpParser._fmt_output(data, ofp, limit=2)
@@ -289,3 +281,16 @@ def test_minidump_parser_06(tmp_path):
     # add .extra file to prioritize .dmp file
     (tmp_path / "b.extra").write_text('{"MozCrashReason":"foo"}')
     assert MinidumpParser.dmp_files(tmp_path)[0] == (tmp_path / "b.dmp")
+
+
+def test_minidump_parser_missing_crashing_thread(tmp_path):
+    """test MinidumpParser._fmt_output() - missing crashing thread"""
+    with (tmp_path / "out.txt").open("w+b") as ofp:
+        # pylint: disable=protected-access
+        MinidumpParser._fmt_output(MD_BASE_AMD64_WIN, ofp)
+        ofp.seek(0)
+        formatted = ofp.read().rstrip().decode().split("\n")
+    assert len(formatted) == 3
+    assert formatted[0] == "OS|Windows NT|10.0.19044"
+    assert formatted[1] == "CPU|amd64|family 6 model 70 stepping 1|8"
+    assert formatted[2] == "Crash|EXCEPTION_BREAKPOINT|0x00007ffe4e09af8d|?"
