From 854b9f74a0771368d78ecfa27a3876a0bd2b9c88 Mon Sep 17 00:00:00 2001 From: "hashimoto@chromium.org" Date: Tue, 3 Feb 2015 07:16:04 +0000 Subject: [PATCH] Follow debug link correctly As thestig@chromium.org pointed out in https://breakpad.appspot.com/9684002, LoadSymbols() should return false if |read_gnu_debug_link| is false. BUG=chromium:453498 R=thestig@chromium.org Review URL: https://breakpad.appspot.com/2844002 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1422 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/common/linux/dump_symbols.cc | 76 ++++++++++++++++---------------- src/common/module.cc | 28 ++++++------ 2 files changed, 53 insertions(+), 51 deletions(-) diff --git a/src/common/linux/dump_symbols.cc b/src/common/linux/dump_symbols.cc index 5a74d805..eb7f1820 100644 --- a/src/common/linux/dump_symbols.cc +++ b/src/common/linux/dump_symbols.cc @@ -628,6 +628,35 @@ bool LoadSymbols(const string& obj_file, "DWARF debugging information\n", obj_file.c_str()); } } + + // See if there are export symbols available. + const Shdr* dynsym_section = + FindElfSectionByName(".dynsym", SHT_DYNSYM, + sections, names, names_end, + elf_header->e_shnum); + const Shdr* dynstr_section = + FindElfSectionByName(".dynstr", SHT_STRTAB, + sections, names, names_end, + elf_header->e_shnum); + if (dynsym_section && dynstr_section) { + info->LoadedSection(".dynsym"); + + const uint8_t* dynsyms = + GetOffset(elf_header, + dynsym_section->sh_offset); + const uint8_t* dynstrs = + GetOffset(elf_header, + dynstr_section->sh_offset); + bool result = + ELFSymbolsToModule(dynsyms, + dynsym_section->sh_size, + dynstrs, + dynstr_section->sh_size, + big_endian, + ElfClass::kAddrSize, + module); + found_usable_info = found_usable_info || result; + } } if (options.symbol_data != NO_CFI) { @@ -689,8 +718,6 @@ bool LoadSymbols(const string& obj_file, names_end, elf_header->e_shnum); if (gnu_debuglink_section) { if (!info->debug_dirs().empty()) { - found_debug_info_section = true; - const char* debuglink_contents = GetOffset(elf_header, gnu_debuglink_section->sh_offset); @@ -709,45 +736,18 @@ bool LoadSymbols(const string& obj_file, fprintf(stderr, "%s does not contain a .gnu_debuglink section.\n", obj_file.c_str()); } + } else { + // Return true if some usable information was found, since the caller + // doesn't want to use .gnu_debuglink. + return found_usable_info; } + + // No debug info was found, let the user try again with .gnu_debuglink + // if present. + return false; } - if (options.symbol_data != ONLY_CFI) { - const Shdr* dynsym_section = - FindElfSectionByName(".dynsym", SHT_DYNSYM, - sections, names, names_end, - elf_header->e_shnum); - const Shdr* dynstr_section = - FindElfSectionByName(".dynstr", SHT_STRTAB, - sections, names, names_end, - elf_header->e_shnum); - if (dynsym_section && dynstr_section) { - info->LoadedSection(".dynsym"); - - const uint8_t* dynsyms = - GetOffset(elf_header, - dynsym_section->sh_offset); - const uint8_t* dynstrs = - GetOffset(elf_header, - dynstr_section->sh_offset); - bool result = - ELFSymbolsToModule(dynsyms, - dynsym_section->sh_size, - dynstrs, - dynstr_section->sh_size, - big_endian, - ElfClass::kAddrSize, - module); - found_usable_info = found_usable_info || result; - } - } - - if (read_gnu_debug_link) { - return found_debug_info_section; - } - - // Return true if some usable information was found - return found_usable_info; + return true; } // Return the breakpad symbol file identifier for the architecture of diff --git a/src/common/module.cc b/src/common/module.cc index f682e24a..d880ce57 100644 --- a/src/common/module.cc +++ b/src/common/module.cc @@ -79,6 +79,17 @@ void Module::AddFunction(Function *function) { // FUNC lines must not hold an empty name, so catch the problem early if // callers try to add one. assert(!function->name.empty()); + + // FUNCs are better than PUBLICs as they come with sizes, so remove an extern + // with the same address if present. + Extern ext(function->address); + ExternSet::iterator it_ext = externs_.lower_bound(&ext); + if (it_ext != externs_.end() && + (*it_ext)->address < function->address + function->size) { + delete *it_ext; + externs_.erase(it_ext); + } + std::pair ret = functions_.insert(function); if (!ret.second && (*ret.first != function)) { // Free the duplicate that was not inserted because this Module @@ -98,19 +109,10 @@ void Module::AddStackFrameEntry(StackFrameEntry *stack_frame_entry) { } void Module::AddExtern(Extern *ext) { - Function func(ext->name, ext->address); - - // Since parsing debug section and public info are not necessarily - // mutually exclusive, check if the symbol has already been read - // as a function to avoid duplicates. - if (functions_.find(&func) == functions_.end()) { - std::pair ret = externs_.insert(ext); - if (!ret.second) { - // Free the duplicate that was not inserted because this Module - // now owns it. - delete ext; - } - } else { + std::pair ret = externs_.insert(ext); + if (!ret.second) { + // Free the duplicate that was not inserted because this Module + // now owns it. delete ext; } }