scripts: cros_generate_breakpad_symbols: Fail more, part 1
We really don't want to ship out builds without good symbol files. If a
beta build fails to produce symbol files, we risk not being able to fix
crashes and risk shipped an unstable "stable" build.
Right now, however, we don't fail the build when we get bad symbols from
Chrome or session_manager or libc because we treat all ELFs the same,
and there are few ELF files we know never generate symbols.
This is part 1 of a three-part process to have
cros_generate_breakpad_symbols be more aggressive about failing the
build when we don't get good symbol files for files that should generate
good symbols. Here, we make an initial cut at separating out the files
that should produce good symbols from those we know will not, and we add
a grepable string that we can use to check what other files need to
moved to the latter list. In part 2, we'll add those other files into
the allowlist and then make bad symbol generation an error. In part 3,
we'll check the generated symbol files from the "expected good" list to
ensure they have all the expected sections.
BUG=b:241470012,b:270240549
TEST=Ran `cros_generate_breakpad_symbols --board=eve`, confirmed no
errors and no "unexpected failure" in output and symbol files seemed
reasonable.
Change-Id: I30add55101e479e26e4ffd5f2730b5c50a4ba880
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/4307796
Reviewed-by: Gilberto Contreras <gcontreras@google.com>
Commit-Queue: Ian Barkley-Yeung <iby@chromium.org>
Tested-by: Ian Barkley-Yeung <iby@chromium.org>
diff --git a/scripts/cros_generate_breakpad_symbols_unittest.py b/scripts/cros_generate_breakpad_symbols_unittest.py
index 1385a25..11a726c 100644
--- a/scripts/cros_generate_breakpad_symbols_unittest.py
+++ b/scripts/cros_generate_breakpad_symbols_unittest.py
@@ -292,7 +292,16 @@
# Not needed as the code itself should create it as needed.
self.breakpad_dir = os.path.join(self.debug_dir, "breakpad")
+ self.FILE_OUT = (
+ f"{self.elf_file}: ELF 64-bit LSB pie executable, x86-64, "
+ "version 1 (SYSV), dynamically linked, interpreter "
+ "/lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, "
+ "BuildID[sha1]=cf9a21fa6b14bfb2dfcb76effd713c4536014d95, stripped"
+ )
self.rc.SetDefaultCmdResult(stdout="MODULE OS CPU ID NAME")
+ self.rc.AddCmdResult(
+ ["/usr/bin/file", self.elf_file], stdout=self.FILE_OUT
+ )
self.assertCommandContains = self.rc.assertCommandContains
self.sym_file = os.path.join(self.breakpad_dir, "NAME/ID/NAME.sym")
@@ -305,12 +314,14 @@
def testNormal(self):
"""Normal run -- given an ELF and a debug file"""
ret = cros_generate_breakpad_symbols.GenerateBreakpadSymbol(
- self.elf_file, self.debug_file, self.breakpad_dir
+ self.elf_file,
+ self.debug_file,
+ self.breakpad_dir,
)
self.assertEqual(ret, self.sym_file)
- self.assertEqual(self.rc.call_count, 1)
+ self.assertEqual(self.rc.call_count, 2)
self.assertCommandArgs(
- 0, ["dump_syms", "-v", self.elf_file, self.debug_dir]
+ 1, ["dump_syms", "-v", self.elf_file, self.debug_dir]
)
self.assertExists(self.sym_file)
@@ -326,8 +337,8 @@
)
self.assertEqual(ret, self.sym_file)
self.assertEqual(num_errors.value, 0)
- self.assertCommandArgs(0, ["dump_syms", "-v", "-c", self.elf_file])
- self.assertEqual(self.rc.call_count, 1)
+ self.assertCommandArgs(1, ["dump_syms", "-v", "-c", self.elf_file])
+ self.assertEqual(self.rc.call_count, 2)
self.assertExists(self.sym_file)
def testNormalElfOnly(self):
@@ -336,8 +347,8 @@
self.elf_file, breakpad_dir=self.breakpad_dir
)
self.assertEqual(ret, self.sym_file)
- self.assertCommandArgs(0, ["dump_syms", "-v", self.elf_file])
- self.assertEqual(self.rc.call_count, 1)
+ self.assertCommandArgs(1, ["dump_syms", "-v", self.elf_file])
+ self.assertEqual(self.rc.call_count, 2)
self.assertExists(self.sym_file)
def testNormalSudo(self):
@@ -349,7 +360,7 @@
)
self.assertEqual(ret, self.sym_file)
self.assertCommandArgs(
- 0, ["sudo", "--", "dump_syms", "-v", self.elf_file]
+ 1, ["sudo", "--", "dump_syms", "-v", self.elf_file]
)
def testLargeDebugFail(self):
@@ -361,12 +372,18 @@
self.elf_file, self.debug_file, self.breakpad_dir
)
self.assertEqual(ret, self.sym_file)
- self.assertEqual(self.rc.call_count, 2)
+ self.assertEqual(self.rc.call_count, 4)
self.assertCommandArgs(
- 0, ["dump_syms", "-v", self.elf_file, self.debug_dir]
+ 1, ["dump_syms", "-v", self.elf_file, self.debug_dir]
+ )
+ # The current fallback from _DumpExpectingSymbols() to
+ # _DumpAllowingBasicFallback() causes the first dump_sums command to get
+ # repeated.
+ self.assertCommandArgs(
+ 2, ["dump_syms", "-v", self.elf_file, self.debug_dir]
)
self.assertCommandArgs(
- 1, ["dump_syms", "-v", "-c", "-r", self.elf_file, self.debug_dir]
+ 3, ["dump_syms", "-v", "-c", "-r", self.elf_file, self.debug_dir]
)
self.assertExists(self.sym_file)
@@ -383,14 +400,20 @@
self.elf_file, self.debug_file, self.breakpad_dir
)
self.assertEqual(ret, self.sym_file)
- self.assertEqual(self.rc.call_count, 3)
+ self.assertEqual(self.rc.call_count, 5)
self.assertCommandArgs(
- 0, ["dump_syms", "-v", self.elf_file, self.debug_dir]
+ 1, ["dump_syms", "-v", self.elf_file, self.debug_dir]
+ )
+ # The current fallback from _DumpExpectingSymbols() to
+ # _DumpAllowingBasicFallback() causes the first dump_sums command to get
+ # repeated.
+ self.assertCommandArgs(
+ 2, ["dump_syms", "-v", self.elf_file, self.debug_dir]
)
self.assertCommandArgs(
- 1, ["dump_syms", "-v", "-c", "-r", self.elf_file, self.debug_dir]
+ 3, ["dump_syms", "-v", "-c", "-r", self.elf_file, self.debug_dir]
)
- self.assertCommandArgs(2, ["dump_syms", "-v", self.elf_file])
+ self.assertCommandArgs(4, ["dump_syms", "-v", self.elf_file])
self.assertExists(self.sym_file)
def testCompleteFail(self):
@@ -408,6 +431,122 @@
self.assertEqual(ret, 1)
self.assertEqual(num_errors.value, 1)
+ def testKernelObjects(self):
+ """Kernel object files should call _DumpAllowingBasicFallback()"""
+ ko_file = os.path.join(self.tempdir, "elf.ko")
+ osutils.Touch(ko_file)
+ self.rc.AddCmdResult(
+ ["dump_syms", "-v", ko_file, self.debug_dir], returncode=1
+ )
+ self.rc.AddCmdResult(
+ ["dump_syms", "-v", "-c", "-r", ko_file, self.debug_dir],
+ returncode=1,
+ )
+ ret = cros_generate_breakpad_symbols.GenerateBreakpadSymbol(
+ ko_file, self.debug_file, self.breakpad_dir
+ )
+ self.assertEqual(ret, self.sym_file)
+ self.assertEqual(self.rc.call_count, 3)
+ # Only one call (at the beginning of _DumpAllowingBasicFallback())
+ # to "dump_syms -v"
+ self.assertCommandArgs(0, ["dump_syms", "-v", ko_file, self.debug_dir])
+ self.assertCommandArgs(
+ 1, ["dump_syms", "-v", "-c", "-r", ko_file, self.debug_dir]
+ )
+ self.assertCommandArgs(2, ["dump_syms", "-v", ko_file])
+ self.assertExists(self.sym_file)
+
+ def testGoBinary(self):
+ """Go binaries should call _DumpAllowingBasicFallback()
+
+ Also tests that dump_syms failing with 'file contains no debugging
+ information' does not fail the script.
+ """
+ go_binary = os.path.join(self.tempdir, "goprogram")
+ osutils.Touch(go_binary)
+ go_debug_file = os.path.join(self.debug_dir, "goprogram.debug")
+ osutils.Touch(go_debug_file, makedirs=True)
+ FILE_OUT_GO = go_binary + (
+ ": ELF 64-bit LSB executable, x86-64, "
+ "version 1 (SYSV), statically linked, "
+ "Go BuildID=KKXVlL66E8Qmngr4qll9/5kOKGZw9I7TmNhoqKLqq/SiYVJam6w5Fo39B3BtDo/ba8_ceezZ-3R4qEv6_-K, "
+ "not stripped"
+ )
+ self.rc.AddCmdResult(["/usr/bin/file", go_binary], stdout=FILE_OUT_GO)
+ self.rc.AddCmdResult(
+ ["dump_syms", "-v", go_binary, self.debug_dir], returncode=1
+ )
+ self.rc.AddCmdResult(
+ ["dump_syms", "-v", "-c", "-r", go_binary, self.debug_dir],
+ returncode=1,
+ )
+ self.rc.AddCmdResult(
+ ["dump_syms", "-v", go_binary],
+ returncode=1,
+ stderr=(
+ f"{go_binary}: file contains no debugging information "
+ '(no ".stab" or ".debug_info" sections)'
+ ),
+ )
+ num_errors = ctypes.c_int()
+ ret = cros_generate_breakpad_symbols.GenerateBreakpadSymbol(
+ go_binary, go_debug_file, self.breakpad_dir
+ )
+ self.assertEqual(ret, 0)
+ self.assertEqual(self.rc.call_count, 4)
+ self.assertCommandArgs(0, ["/usr/bin/file", go_binary])
+ # Only one call (at the beginning of _DumpAllowingBasicFallback())
+ # to "dump_syms -v"
+ self.assertCommandArgs(
+ 1, ["dump_syms", "-v", go_binary, self.debug_dir]
+ )
+ self.assertCommandArgs(
+ 2, ["dump_syms", "-v", "-c", "-r", go_binary, self.debug_dir]
+ )
+ self.assertCommandArgs(3, ["dump_syms", "-v", go_binary])
+ self.assertNotExists(self.sym_file)
+ self.assertEqual(num_errors.value, 0)
+
+ def testAllowlist(self):
+ """Binaries in the allowlist should call _DumpAllowingBasicFallback()"""
+ binary = os.path.join(self.tempdir, "usr/bin/goldctl")
+ osutils.Touch(binary, makedirs=True)
+ debug_dir = os.path.join(self.debug_dir, "usr/bin")
+ debug_file = os.path.join(debug_dir, "goldctl.debug")
+ osutils.Touch(debug_file, makedirs=True)
+ self.rc.AddCmdResult(["/usr/bin/file", binary], stdout=self.FILE_OUT)
+ self.rc.AddCmdResult(
+ ["dump_syms", "-v", binary, debug_dir], returncode=1
+ )
+ self.rc.AddCmdResult(
+ ["dump_syms", "-v", "-c", "-r", binary, debug_dir],
+ returncode=1,
+ )
+ self.rc.AddCmdResult(
+ ["dump_syms", "-v", binary],
+ returncode=1,
+ stderr=(
+ f"{binary}: file contains no debugging information "
+ '(no ".stab" or ".debug_info" sections)'
+ ),
+ )
+ num_errors = ctypes.c_int()
+ ret = cros_generate_breakpad_symbols.GenerateBreakpadSymbol(
+ binary, debug_file, self.breakpad_dir, sysroot=self.tempdir
+ )
+ self.assertEqual(ret, 0)
+ self.assertEqual(self.rc.call_count, 4)
+ self.assertCommandArgs(0, ["/usr/bin/file", binary])
+ # Only one call (at the beginning of _DumpAllowingBasicFallback())
+ # to "dump_syms -v"
+ self.assertCommandArgs(1, ["dump_syms", "-v", binary, debug_dir])
+ self.assertCommandArgs(
+ 2, ["dump_syms", "-v", "-c", "-r", binary, debug_dir]
+ )
+ self.assertCommandArgs(3, ["dump_syms", "-v", binary])
+ self.assertNotExists(self.sym_file)
+ self.assertEqual(num_errors.value, 0)
+
class UtilsTestDir(cros_test_lib.TempDirTestCase):
"""Tests ReadSymsHeader."""
@@ -416,7 +555,9 @@
"""Make sure ReadSymsHeader can parse sym files"""
sym_file = os.path.join(self.tempdir, "sym")
osutils.WriteFile(sym_file, "MODULE Linux x86 s0m31D chrooome")
- result = cros_generate_breakpad_symbols.ReadSymsHeader(sym_file)
+ result = cros_generate_breakpad_symbols.ReadSymsHeader(
+ sym_file, "unused_elfname"
+ )
self.assertEqual(result.cpu, "x86")
self.assertEqual(result.id, "s0m31D")
self.assertEqual(result.name, "chrooome")
@@ -429,7 +570,7 @@
def testReadSymsHeaderGoodBuffer(self):
"""Make sure ReadSymsHeader can parse sym file handles"""
result = cros_generate_breakpad_symbols.ReadSymsHeader(
- io.BytesIO(b"MODULE Linux arm MY-ID-HERE blkid")
+ io.BytesIO(b"MODULE Linux arm MY-ID-HERE blkid"), "unused_elfname"
)
self.assertEqual(result.cpu, "arm")
self.assertEqual(result.id, "MY-ID-HERE")
@@ -442,6 +583,7 @@
ValueError,
cros_generate_breakpad_symbols.ReadSymsHeader,
io.BytesIO(b"asdf"),
+ "unused_elfname",
)
def testBreakpadDir(self):