search

acrisci / simple-breakpad-server

Simple breakpad crash reports collecting server
https://www.npmjs.com/package/simple-breakpad-server
MIT License
67 stars 29 forks source link

Fix incorrect symbol file name for .pdb files #3

Closed Jimbly closed 7 years ago

Jimbly commented 7 years ago

For PDBs, do not append .sym, replace the extension, as per https://chromium.googlesource.com/breakpad/breakpad/+/master/src/processor/simple_symbol_supplier.cc#179

I was baffled as to how this appeared to be totally non-functional, but upon debugging the breakpad source code, it appears this is a special case with Windows PDB files, and I'll have to assume others were only using this on OSX/Linux =).

acrisci commented 7 years ago

I think it's bad practice to tie yourself too heavily to file extensions because it causes issues like this, and part of the goal of managing symfiles is to abstract away this very unforgiving file structure requirement.

Is there any other way of telling if the file is a pdb file by looking at the contents? I think that would be a preferable approach if it is possible.

Jimbly commented 7 years ago

It doesn't actually matter what the contents is - what matters is when we later call minidump_stackwalk, does that process actually load our symbol file. If a PDB file is not named .pdb, then it doesn't need to do this renaming - e.g. I could also change my build settings to name my debug files .mypdb, and then it looks like breakpad would use the other naming rule (append extension instead of change extension), of course, that might break native debugging in other ways, and users should not need to significantly change their build process to use breakpad.

I totally agree with your thought though. Unfortunately breakpad has this odd code in it, presumably to assist in migrating from the Win32-only thing they were using for a symbol server before breakpad, so their symbol file lookup is weird.

Other option is to attempt to get a change into Chromium to change breakpad to look for files under either name, though that might be non-trivial (or impossible to do without performance impact), and sending change requests to Chromium is a much more involved process than GitHub PRs ;).

acrisci commented 7 years ago

I agree, but is there any way to look inside a file and determine what file extension to give it?

For instance, if it is a pdb file if and only if the OS is windows, we can look at the contents of the symfile, see that the OS is windows and then give a pdb extension regardless of what the user named the file.

There is probably something in the spec for the file format that will tell you this.

Jimbly commented 7 years ago

This is already coming from inside the symfile. The uploaded file has no (relevant) name. Regardless of platform, all symfiles are the same format, but on the first line there is a "code" and "name", which is what is also referenced in a minidump. The minidump -> symfile lookup uses the code and name to generate a path in the referenced breakpad code, so when we're writing a symfile, we need to use the code and name to generate exactly the same path.

Jimbly commented 7 years ago

Also, to be clear, I'm pretty certain this is purely a file naming on disk thing, it has nothing to do with what's in the files. If you change your build of a test project on Linux so that your debug symbols are placed in a file that happens to end in .pdb, the current implementation of simple-breakpad-server will be unable to look up symbols for your new builds. We just need our storage of .sym files in the symbol path to match how breakpad looks up symbols. Even if the way they look up files is stupid =).

acrisci commented 7 years ago

Oh I see. I misunderstood what was going on. I have no way to test this because I don't have a windows machine.

Looks good :+1:.