Refactor code out of MinidumpModuleList::Read().

Add a StoreRange() helper method and an IsDevAshmem() helper function.

Change-Id: Iaec9dee1e08bd0155f1c33cfe9af722b0dcaef31
Reviewed-on: https://chromium-review.googlesource.com/1114188
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
This commit is contained in:
Lei Zhang 2018-06-25 14:31:04 -07:00
parent d0241bb91c
commit d531e1b2ba
2 changed files with 60 additions and 48 deletions

View file

@ -541,6 +541,11 @@ class MinidumpModuleList : public MinidumpStream,
bool Read(uint32_t expected_size); bool Read(uint32_t expected_size);
bool StoreRange(const MinidumpModule& module,
uint64_t base_address,
uint32_t module_index,
uint32_t module_count);
// The largest number of modules that will be read from a minidump. The // The largest number of modules that will be read from a minidump. The
// default is 1024. // default is 1024.
static uint32_t max_modules_; static uint32_t max_modules_;

View file

@ -416,6 +416,11 @@ string MDGUIDToString(const MDGUID& uuid) {
return std::string(buf); return std::string(buf);
} }
bool IsDevAshmem(const string& filename) {
const string kDevAshmem("/dev/ashmem/");
return filename.compare(0, kDevAshmem.length(), kDevAshmem) == 0;
}
} // namespace } // namespace
// //
@ -2674,8 +2679,7 @@ bool MinidumpModuleList::Read(uint32_t expected_size) {
scoped_ptr<MinidumpModules> modules( scoped_ptr<MinidumpModules> modules(
new MinidumpModules(module_count, MinidumpModule(minidump_))); new MinidumpModules(module_count, MinidumpModule(minidump_)));
for (unsigned int module_index = 0; for (uint32_t module_index = 0; module_index < module_count;
module_index < module_count;
++module_index) { ++module_index) {
MinidumpModule* module = &(*modules)[module_index]; MinidumpModule* module = &(*modules)[module_index];
@ -2693,17 +2697,16 @@ bool MinidumpModuleList::Read(uint32_t expected_size) {
// included in the loop above, additional seeks would be needed where // included in the loop above, additional seeks would be needed where
// none are now to read contiguous data. // none are now to read contiguous data.
uint64_t last_end_address = 0; uint64_t last_end_address = 0;
for (unsigned int module_index = 0; for (uint32_t module_index = 0; module_index < module_count;
module_index < module_count;
++module_index) { ++module_index) {
MinidumpModule* module = &(*modules)[module_index]; MinidumpModule& module = (*modules)[module_index];
// ReadAuxiliaryData fails if any data that the module indicates should // ReadAuxiliaryData fails if any data that the module indicates should
// exist is missing, but we treat some such cases as valid anyway. See // exist is missing, but we treat some such cases as valid anyway. See
// issue #222: if a debugging record is of a format that's too large to // issue #222: if a debugging record is of a format that's too large to
// handle, it shouldn't render the entire dump invalid. Check module // handle, it shouldn't render the entire dump invalid. Check module
// validity before giving up. // validity before giving up.
if (!module->ReadAuxiliaryData() && !module->valid()) { if (!module.ReadAuxiliaryData() && !module.valid()) {
BPLOG(ERROR) << "MinidumpModuleList could not read required module " BPLOG(ERROR) << "MinidumpModuleList could not read required module "
"auxiliary data for module " << "auxiliary data for module " <<
module_index << "/" << module_count; module_index << "/" << module_count;
@ -2713,52 +2716,35 @@ bool MinidumpModuleList::Read(uint32_t expected_size) {
// It is safe to use module->code_file() after successfully calling // It is safe to use module->code_file() after successfully calling
// module->ReadAuxiliaryData or noting that the module is valid. // module->ReadAuxiliaryData or noting that the module is valid.
uint64_t base_address = module->base_address(); uint64_t base_address = module.base_address();
uint64_t module_size = module->size(); uint64_t module_size = module.size();
if (base_address == static_cast<uint64_t>(-1)) { if (base_address == static_cast<uint64_t>(-1)) {
BPLOG(ERROR) << "MinidumpModuleList found bad base address " BPLOG(ERROR) << "MinidumpModuleList found bad base address for module "
"for module " << module_index << "/" << module_count << << module_index << "/" << module_count << ", "
", " << module->code_file(); << module.code_file();
return false; return false;
} }
if (!range_map_->StoreRange(base_address, module_size, module_index)) { if (!StoreRange(module, base_address, module_index, module_count)) {
// Android's shared memory implementation /dev/ashmem can contain if (base_address >= last_end_address) {
// duplicate entries for JITted code, so ignore these. BPLOG(ERROR) << "MinidumpModuleList could not store module "
// TODO(wfh): Remove this code when Android is fixed. << module_index << "/" << module_count << ", "
// See https://crbug.com/439531 << module.code_file() << ", " << HexString(base_address)
const string kDevAshmem("/dev/ashmem/"); << "+" << HexString(module_size);
if (module->code_file().compare( return false;
0, kDevAshmem.length(), kDevAshmem) != 0) { }
if (base_address < last_end_address) {
// If failed due to apparent range overlap the cause may be // If failed due to apparent range overlap the cause may be the client
// the client correction applied for Android packed relocations. // correction applied for Android packed relocations. If this is the
// If this is the case, back out the client correction and retry. // case, back out the client correction and retry.
module_size -= last_end_address - base_address; module_size -= last_end_address - base_address;
base_address = last_end_address; base_address = last_end_address;
if (!range_map_->StoreRange(base_address, if (!range_map_->StoreRange(base_address, module_size, module_index)) {
module_size, module_index)) { BPLOG(ERROR) << "MinidumpModuleList could not store module "
BPLOG(ERROR) << "MinidumpModuleList could not store module " << << module_index << "/" << module_count << ", "
module_index << "/" << module_count << ", " << << module.code_file() << ", " << HexString(base_address)
module->code_file() << ", " << << "+" << HexString(module_size) << ", after adjusting";
HexString(base_address) << "+" << return false;
HexString(module_size) << ", after adjusting";
return false;
}
} else {
BPLOG(ERROR) << "MinidumpModuleList could not store module " <<
module_index << "/" << module_count << ", " <<
module->code_file() << ", " <<
HexString(base_address) << "+" <<
HexString(module_size);
return false;
}
} else {
BPLOG(INFO) << "MinidumpModuleList ignoring overlapping module " <<
module_index << "/" << module_count << ", " <<
module->code_file() << ", " <<
HexString(base_address) << "+" <<
HexString(module_size);
} }
} }
last_end_address = base_address + module_size; last_end_address = base_address + module_size;
@ -2773,6 +2759,27 @@ bool MinidumpModuleList::Read(uint32_t expected_size) {
return true; return true;
} }
bool MinidumpModuleList::StoreRange(const MinidumpModule& module,
uint64_t base_address,
uint32_t module_index,
uint32_t module_count) {
if (range_map_->StoreRange(base_address, module.size(), module_index))
return true;
// Android's shared memory implementation /dev/ashmem can contain duplicate
// entries for JITted code, so ignore these.
// TODO(wfh): Remove this code when Android is fixed.
// See https://crbug.com/439531
if (IsDevAshmem(module.code_file())) {
BPLOG(INFO) << "MinidumpModuleList ignoring overlapping module "
<< module_index << "/" << module_count << ", "
<< module.code_file() << ", " << HexString(base_address) << "+"
<< HexString(module.size());
return true;
}
return false;
}
const MinidumpModule* MinidumpModuleList::GetModuleForAddress( const MinidumpModule* MinidumpModuleList::GetModuleForAddress(
uint64_t address) const { uint64_t address) const {