Stop using an external fileid program

The symbolic-debuginfo crate has everything required to replace it
already.

At the same time, look for breakpad symbols in the directory
they're supposed to be based on their fileid, rather than assuming that
if there's only one subdirectory, it's the right one.
This means breakpad syms directories for tests need to be adjusted,
which they are by taking the id as appearing on the MODULE line in the
sym file. This validates that obj.debug_id().breakpad() returns the
expected value.

Also change the instructions to generate the breakpad symbols for
examples, using https://github.com/mozilla/dump_syms, which simplifies
things, albeit modifies the linux example file.

src/main.rs

@@ -13,7 +13,6 @@ use std::env;
 use std::fs;
 use std::io::{self, BufRead, Write};
 use std::path::{Path, PathBuf};
-use std::process::Command;
 use std::str;
 use symbolic_common::{Arch, Name};
 use symbolic_debuginfo::{Archive, FileFormat, Function, Object, ObjectDebugSession};
@@ -256,7 +255,6 @@ impl FileInfo {
 /// Info provided via the `-b` flag.
 struct BreakpadInfo {
     syms_dir: String,
-    fileid_exe: String,
 }
 
 /// The top level structure that does the work.
@@ -338,10 +336,7 @@ impl Fixer {
 
     fn build_file_info_breakpad(
         bin_file: &str,
-        BreakpadInfo {
-            syms_dir,
-            fileid_exe,
-        }: &BreakpadInfo,
+        BreakpadInfo { syms_dir }: &BreakpadInfo,
     ) -> Result<FileInfo> {
         // We must find the `.sym` file for this `bin_file`, as produced by the
         // Firefox build system, which is in the symbols directory under
@@ -381,26 +376,14 @@ impl Fixer {
         db_dir.push(&syms_dir);
         db_dir.push(&db_seg);
 
-        // - Unix: `db_entries` iterates over `syms/libxul.so/`
-        // - Windows: `db_entries` iterates over `syms/xul.pdb/`
-        let mut db_entries = fs::read_dir(&db_dir)
-            .context(
-                "note: this is expected and harmless for system libraries on debug automation runs",
-            )
-            .with_context(|| format!("read breakpad symbols dir `{}` for", db_dir.display()))?;
-
         // - Unix: `uuid_dir` is `syms/libxul.so/<uuid>/`
         // - Windows: `uuid_dir` is `syms/xul.pdb/<uuid>/`
-        let uuid_dir = if let (Some(d), None) = (db_entries.next(), db_entries.next()) {
-            d.with_context(|| format!("read breakpad symbols dir `{}` for", db_dir.display()))?
-                .path()
-        } else {
-            // Use `fileid` to determine the right directory.
-            let output = Command::new(fileid_exe)
-                .arg(&bin_file)
-                .output()
-                .with_context(|| format!("run `{}` for", fileid_exe))?;
-            let uuid_seg = str::from_utf8(&output.stdout).unwrap().trim_end();
+        let uuid_dir = {
+            let data = fs::read(bin_file).context("read")?;
+            let object = Object::parse(&data)
+                .map_err(Fail::compat)
+                .context("parse")?;
+            let uuid_seg = object.debug_id().breakpad().to_string();
             let mut uuid_dir = db_dir;
             uuid_dir.push(uuid_seg);
             uuid_dir
@@ -417,6 +400,9 @@ impl Fixer {
         sym_file.push(&sym_seg);
 
         let data = fs::read(&sym_file)
+            .context(
+                "note: this is expected and harmless for system libraries on debug automation runs",
+            )
             .with_context(|| format!("read symbols file `{}` for", sym_file.display()))?;
         Fixer::build_file_info_direct(&data)
     }
@@ -835,8 +821,7 @@ Post-process the stack frames produced by MozFormatCodeAddress().
 options:
   -h, --help              Show this message and exit
   -j, --json              Treat input and output as JSON fragments
-  -b, --breakpad DIR,EXE  Use breakpad symbols in directory DIR,
-                          and `fileid` EXE to choose among possibilities
+  -b, --breakpad DIR      Use breakpad symbols in directory DIR
 "##;
 
 fn main_inner() -> io::Result<()> {
@@ -857,15 +842,9 @@ fn main_inner() -> io::Result<()> {
         } else if arg == "-b" || arg == "--breakpad" {
             match args.next() {
                 Some(arg2) => {
-                    let v: Vec<_> = arg2.split(',').collect();
-                    if v.len() == 2 {
-                        bp_info = Some(BreakpadInfo {
-                            syms_dir: v[0].to_string(),
-                            fileid_exe: v[1].to_string(),
-                        });
-                    } else {
-                        return err(format!("bad argument `{}` to option `{}`.", arg2, arg));
-                    }
+                    bp_info = Some(BreakpadInfo {
+                        syms_dir: arg2.to_string(),
+                    });
                 }
                 _ => {
                     return err(format!("missing argument to option `{}`.", arg));

src/tests.rs

@@ -297,15 +297,10 @@ fn test_linux_breakpad() {
     // LINE 0x11cd line=13 file=/home/njn/moz/fix-stacks/tests/example.c
     // LINE 0x11d1 line=13 file=/home/njn/moz/fix-stacks/tests/example.c
     // LINE 0x11db line=14 file=/home/njn/moz/fix-stacks/tests/example.c
-
-    // We can use "" for `fileid_exe` because that field is only used for
-    // binaries with multiple Breakpad symbol dirs, which this test doesn't
-    // have.
     let mut fixer = Fixer::new(
         JsonMode::No,
         Some(BreakpadInfo {
             syms_dir: "tests/bpsyms".to_string(),
-            fileid_exe: "".to_string(),
         }),
     );
 
@@ -367,15 +362,10 @@ fn test_windows_breakpad() {
     // LINE 0x6c49 line=12 file=c:\Users\njn\moz\fix-stacks\tests\example.c
     // LINE 0x6c55 line=13 file=c:\Users\njn\moz\fix-stacks\tests\example.c
     // LINE 0x6c61 line=14 file=c:\Users\njn\moz\fix-stacks\tests\example.c
-
-    // We can use "" for `fileid_exe` because that field is only used for
-    // binaries with multiple Breakpad symbol dirs, which this test doesn't
-    // have.
     let mut fixer = Fixer::new(
         JsonMode::No,
         Some(BreakpadInfo {
             syms_dir: "tests/bpsyms".to_string(),
-            fileid_exe: "".to_string(),
         }),
     );
 

tests/README.md

@@ -75,33 +75,29 @@ which avoids the need for more complex changes to that file.)
 
 ### Breakpad symbols
 
-`bpsyms/example-linux/` was produced on an Ubuntu 19.10 box by `dump_syms`
-(from a development build of Firefox), with these commands within `tests/`:
+`bpsyms/example-*/` was produced by `dump_syms` (from a build of
+https://github.com/mozilla/dump_syms), with these commands within `tests/`:
 ```
-# 123456781234567812345678123456789 is a fake UUID whose exact value doesn't
-# matter.
+# $DUMP_SYMS is the location of the dump_syms executable.
+$DUMP_SYMS example-linux > example-linux.sym
+# Replace 123456781234567812345678123456789 below with the UUID found on the
+# MODULE line in example-linux.sym.
 DIR="bpsyms/example-linux/123456781234567812345678123456789/"
 mkdir $DIR
-# $OBJDIR is the object directory of the Firefox build.
-$OBJDIR/dist/host/bin/dump_syms example-linux > $DIR/example-linux.sym
+mv example-linux.sym $DIR
+
+$DUMP_SYMS example-windows.pdb > example-windows.sym
+# Replace 123456781234567812345678123456789 below with the UUID found on the
+# MODULE line in example-windows.sym.
+DIR="bpsyms/example-windows/123456781234567812345678123456789/"
+mkdir $DIR
+mv example-windows.sym $DIR
 ```
 
 `example-linux.sym` was then edited to change the `FILE` line mentioning
 `example.c` to be prefixed with a Mercurial repository and suffixed with a
 revision ID, in order to test the removal of this Firefox Breakpad junk.
 
-`bpsyms/example-windows.pdb/` was produced on Windows 10 laptop by
-`dump_syms.exe`, with these commands within `tests/`:
-```
-# $SRCDIR is the source directory of a Firfox build.
-$SRCDIR/mach artifact toolchain --from-build win64-dump-syms
-# 123456781234567812345678123456789 is a fake UUID whose exact value doesn't
-# matter.
-DIR="bpsyms/example-windows.pdb/123456781234567812345678123456789/"
-mkdir $DIR
-dump_syms/dump_syms.exe example-windows.exe > $DIR/example-windows.sym
-```
-
 ## Obtaining the debug info
 
 The unit tests refer to specific addresses within the generated binaries. These

tests/bpsyms/example-linux/123456781234567812345678123456789/example-linux.sym → tests/bpsyms/example-linux/BE4E976C325246EE9D6B7847A670B2A90/example-linux.sym

@@ -1,64 +1,57 @@
 MODULE Linux x86_64 BE4E976C325246EE9D6B7847A670B2A90 example-linux
 INFO CODE_ID 6C974EBE5232EE469D6B7847A670B2A956F8AEDE
 FILE 0 hg:hg.mozilla.org/integration/autoland:/home/njn/moz/fix-stacks/tests/example.c:94d31f914f29e72dd81a8880100d12f67e48a5b0
+PUBLIC 1000 0 _init
+PUBLIC 1040 0 _start
+PUBLIC 1070 0 _dl_relocate_static_pie
+PUBLIC 1080 0 deregister_tm_clones
+PUBLIC 10b0 0 register_tm_clones
+PUBLIC 10f0 0 __do_global_dtors_aux
+PUBLIC 1120 0 frame_dummy
 FUNC 1130 28 0 main
 1130 f 24 0
 113f 7 25 0
 1146 9 26 0
-114f 3 27 0
-1152 6 27 0
+114f 9 27 0
 FUNC 1160 45 0 f
 1160 c 16 0
-116c 4 17 0
-1170 7 17 0
-1177 4 18 0
-117b 5 18 0
-1180 4 19 0
-1184 7 19 0
-118b 4 20 0
-118f 5 20 0
-1194 4 21 0
-1198 7 21 0
+116c b 17 0
+1177 9 18 0
+1180 b 19 0
+118b 9 20 0
+1194 b 21 0
 119f 6 22 0
 FUNC 11b0 31 0 g
 11b0 c 11 0
 11bc 11 12 0
-11cd 4 13 0
-11d1 a 13 0
+11cd e 13 0
 11db 6 14 0
-PUBLIC 1000 0 _init
-PUBLIC 1040 0 _start
-PUBLIC 1070 0 _dl_relocate_static_pie
-PUBLIC 1080 0 deregister_tm_clones
-PUBLIC 10b0 0 register_tm_clones
-PUBLIC 10f0 0 __do_global_dtors_aux
-PUBLIC 1120 0 frame_dummy
 PUBLIC 11f0 0 __libc_csu_init
 PUBLIC 1250 0 __libc_csu_fini
 PUBLIC 1254 0 _fini
+STACK CFI INIT 1040 2b .cfa: $rsp 8 +
+STACK CFI INIT 1070 1 .cfa: $rsp 8 + .ra: .cfa -8 + ^
 STACK CFI INIT 1020 20 .cfa: $rsp 16 + .ra: .cfa -8 + ^
 STACK CFI 1026 .cfa: $rsp 24 +
-STACK CFI INIT 1040 2b .cfa: $rsp 8 + .ra: .cfa -8 + ^
-STACK CFI INIT 1070 1 .cfa: $rsp 8 + .ra: .cfa -8 + ^
 STACK CFI INIT 1130 28 .cfa: $rsp 8 + .ra: .cfa -8 + ^
-STACK CFI 1131 $rbp: .cfa -16 + ^ .cfa: $rsp 16 +
+STACK CFI 1131 .cfa: $rsp 16 + $rbp: .cfa -16 + ^
 STACK CFI 1134 .cfa: $rbp 16 +
 STACK CFI 1157 .cfa: $rsp 8 +
 STACK CFI INIT 1160 45 .cfa: $rsp 8 + .ra: .cfa -8 + ^
-STACK CFI 1161 $rbp: .cfa -16 + ^ .cfa: $rsp 16 +
+STACK CFI 1161 .cfa: $rsp 16 + $rbp: .cfa -16 + ^
 STACK CFI 1164 .cfa: $rbp 16 +
 STACK CFI 11a4 .cfa: $rsp 8 +
 STACK CFI INIT 11b0 31 .cfa: $rsp 8 + .ra: .cfa -8 + ^
-STACK CFI 11b1 $rbp: .cfa -16 + ^ .cfa: $rsp 16 +
+STACK CFI 11b1 .cfa: $rsp 16 + $rbp: .cfa -16 + ^
 STACK CFI 11b4 .cfa: $rbp 16 +
 STACK CFI 11e0 .cfa: $rsp 8 +
 STACK CFI INIT 11f0 5d .cfa: $rsp 8 + .ra: .cfa -8 + ^
-STACK CFI 11f2 $r15: .cfa -16 + ^ .cfa: $rsp 16 +
-STACK CFI 11f7 $r14: .cfa -24 + ^ .cfa: $rsp 24 +
-STACK CFI 11fc $r13: .cfa -32 + ^ .cfa: $rsp 32 +
-STACK CFI 1201 $r12: .cfa -40 + ^ .cfa: $rsp 40 +
-STACK CFI 1209 $rbp: .cfa -48 + ^ .cfa: $rsp 48 +
-STACK CFI 1211 $rbx: .cfa -56 + ^ .cfa: $rsp 56 +
+STACK CFI 11f2 .cfa: $rsp 16 + $r15: .cfa -16 + ^
+STACK CFI 11f7 .cfa: $rsp 24 + $r14: .cfa -24 + ^
+STACK CFI 11fc .cfa: $rsp 32 + $r13: .cfa -32 + ^
+STACK CFI 1201 .cfa: $rsp 40 + $r12: .cfa -40 + ^
+STACK CFI 1209 .cfa: $rsp 48 + $rbp: .cfa -48 + ^
+STACK CFI 1211 .cfa: $rsp 56 + $rbx: .cfa -56 + ^
 STACK CFI 1218 .cfa: $rsp 64 +
 STACK CFI 1242 .cfa: $rsp 56 +
 STACK CFI 1243 .cfa: $rsp 48 +