Remove duplicate FUNC entries from STABS dump_syms output.

Remove duplicate FUNC entries from dump_syms output.

Committed: http://code.google.com/p/google-breakpad/source/detail?r=623

[Lei Zhang (chromium)]

When running minidump_stackwalk on a Chromium binary with STABS symbols, I get
tons of warnings for StoreRange in ./src/processor/range_map-inl.h. Turns out we
have a lot of duplicate data in the symbol file generated by dump_syms. This
trims the symbol file down to 67 MB from 76 MB.

[Lei Zhang (chromium)]

Sigh, we have the same problem with DWARF. I changed Module's function container
to a set in patch set 2.

[jimb]

Thanks for patching this!

http://breakpad.appspot.com/128001/diff/3001/4001
File src/common/module.cc (right):

http://breakpad.appspot.com/128001/diff/3001/4001#newcode65
Line 65: functions_.insert(function);
Won't this leak the old Function?  The Module owns all the functions that have
been added to it.

[jimb]

A few more comments.

http://breakpad.appspot.com/128001/diff/3001/4002
File src/common/module.h (right):

http://breakpad.appspot.com/128001/diff/3001/4002#newcode152
Line 152: return lhs->address < rhs->address;
Perhaps if the addresses are equal, this should compare names. I don't think it
matters, but it would make the behavior more well-defined, and easier to test.

http://breakpad.appspot.com/128001/diff/3001/4003
File src/common/module_unittest.cc (right):

http://breakpad.appspot.com/128001/diff/3001/4003#newcode398
Line 398: }
Since we've changed Module's contract, I think we should have unit tests here,
too.

[Lei Zhang (chromium)]

http://breakpad.appspot.com/128001/diff/3001/4001
File src/common/module.cc (right):

http://breakpad.appspot.com/128001/diff/3001/4001#newcode65
Line 65: functions_.insert(function);
On 2010/07/12 21:07:25, jimb wrote:
> Won't this leak the old Function?  The Module owns all the functions that have
> been added to it.

Yes, fixed.

http://breakpad.appspot.com/128001/diff/3001/4002
File src/common/module.h (right):

http://breakpad.appspot.com/128001/diff/3001/4002#newcode152
Line 152: return lhs->address < rhs->address;
On 2010/07/12 21:12:30, jimb wrote:
> Perhaps if the addresses are equal, this should compare names. I don't think
it
> matters, but it would make the behavior more well-defined, and easier to test.

Done.

http://breakpad.appspot.com/128001/diff/3001/4003
File src/common/module_unittest.cc (right):

http://breakpad.appspot.com/128001/diff/3001/4003#newcode398
Line 398: }
On 2010/07/12 21:12:30, jimb wrote:
> Since we've changed Module's contract, I think we should have unit tests here,
> too.

Done.

[jimb]

LGTM.  New tests look just right.

http://breakpad.appspot.com/128001/diff/11001/12001
File src/common/module.cc (right):

http://breakpad.appspot.com/128001/diff/11001/12001#newcode153
Line 153: if (!file_it->second->source_id)
Thanks --- old GNU habit.

http://breakpad.appspot.com/128001/diff/11001/12001#newcode159
Line 159: strerror(errno));
ibid