Linux code should use build id produced by ld --build-id if available

Modern binutils actually include a Build ID in an ELF NOTE section, as I noted
in issue 243. This patch lets the Linux code use that ID as the file ID if
available.

[Ted Mielczarek]

I uploaded this patch because I had written it a while ago. I think we might as
well land it, I don't think it will do any harm and it seems like the right
thing to do.

It could use some unit tests, which ought to be feasible to write with the test
classes in my patch in http://breakpad.appspot.com/277001/show

[Lei Zhang (chromium)]

mostly LGTM, just some nits:

http://breakpad.appspot.com/185001/diff/1/2
File src/common/linux/file_id.cc (right):

http://breakpad.appspot.com/185001/diff/1/2#newcode56
Line 56: #define NT_GNU_BUILD_ID	3
nit: The space before the 3 is actually a tab. Can you make it a space?

http://breakpad.appspot.com/185001/diff/1/2#newcode174
Line 174: const Nhdr* note_header =
nit: there's an extra space at the end of the line.

http://breakpad.appspot.com/185001/diff/1/2#newcode177
Line 177: || note_header->n_descsz == 0)
nit: || goes on the end of line 176.

Wrap the continue in {}.

http://breakpad.appspot.com/185001/diff/1/2#newcode198
Line 198: {
This looks very similar to FindElfTextSection(). Any chance we can de-duplicate
some of this code?

[Ted Mielczarek]

Mind giving this a second once-over? While addressing your last comment I wound
up refactoring a good bit of the code. I also added some unit tests in this
patch.

[Lei Zhang (chromium)]

LGTM with some silly nits.

Apologies for the delay. You sent out patch set 2 right after I went on
vacation. @_@

http://breakpad.appspot.com/185001/diff/5001/6001
File src/common/linux/file_id.cc (right):

http://breakpad.appspot.com/185001/diff/5001/6001#newcode174
Line 174: reinterpret_cast<const Nhdr*>(section);
nit: this fits on the previous line.

http://breakpad.appspot.com/185001/diff/5001/6001#newcode225
Line 225: 
nit: lots of blank spaces on this line.

http://breakpad.appspot.com/185001/diff/5001/6003
File src/common/linux/file_id_unittest.cc (right):

http://breakpad.appspot.com/185001/diff/5001/6003#newcode141
Line 141: char expected_identifier_string[] =
nit: extra space at the end of line.

http://breakpad.appspot.com/185001/diff/5001/6004
File src/common/linux/synth_elf.cc (right):

http://breakpad.appspot.com/185001/diff/5001/6004#newcode187
Line 187: 
nit: extra spaces on this line.

[Ted Mielczarek]

On 2011/08/01 20:23:47, thestig wrote:
> Apologies for the delay. You sent out patch set 2 right after I went on
> vacation. @_@

No problem. I went on vacation somewhere in there as well. And then I forgot
about this patch for a month anyway. Thanks for the reviews!