Short version:
ElfFileIdentifierForMapping is NOT idempotent as the name and its signature
suggests. It mangles the mapping path name and does that in a gross way.
Full version:
Currently ElfFileIdentifierForMapping() in
client/linux/minidump_writer/linux_dumper.cc has a very intricate logic.
1. The method signature takes both a const MappingInfo& and a mapping_id. At
first glance this might seem redundant, since a MappingInfo can be retrieved
from the |mappings_| list by its id. However this seems to deal with the case
of Mappings coming from *somewhere else* (?) and not part of the app's mappings
list. Eventually, at this point, this could take only a ref to MappingInfo ...
but (see next points). (Introduced in http://breakpad.appspot.com/242001)
2. The name ElfFileIdentifierForMapping() would suggests that the function is
idempotent and just retrieves an identifier given a mapping, especially
considering that it takes a *const* reference to MappingInfo. However, at
current state, it can alter the |name| of a mapping (to strip out the "
(deleted)" part from the pathname). This is extremely confusing.
3. There are call sites that depend on this behavior
(ElfFileIdentifierForMapping cleaning up the path name). When cleaning up
ElfFileIdentifierForMapping, those call sites should be refactored as well.
4. The way the "(deleted)" part of the path name is deleted is a bit gross:
HandleDeletedFileInMapping() returns the cleaned up |filename|. However,
ElfFileIdentifierForMapping never looks into that returned |path|, rather it
just speculates on its
implementation: when filename_modified == true, it assumes it did truncate the
path of exactly (sizeof(kDeletedSuffix) + 1) characters. Hence, the code in
ElfFileIdentifierForMapping (~line 136) adds a null-terminator on the basis of
such speculation @ filename_len - sizeof(kDeletedSuffix) + 1. (Introduced in
https://breakpad.appspot.com/228001)
More discussion in https://breakpad.appspot.com/7734002/
Original issue reported on code.google.com by primi...@chromium.org on 16 Oct 2014 at 9:42
Original issue reported on code.google.com by
primi...@chromium.orgon 16 Oct 2014 at 9:42