This revision was automatically updated to reflect the committed changes. Closed by commit rL260369: Improve the handling of missing elf symtab and missing symbol sizes (authored by tberghammer).
Changed prior to commit: http://reviews.llvm.org/D16996?vs=47314&id=47433#toc Repository: rL LLVM http://reviews.llvm.org/D16996 Files: lldb/trunk/include/lldb/Core/RangeMap.h lldb/trunk/include/lldb/Symbol/DWARFCallFrameInfo.h lldb/trunk/include/lldb/Symbol/ObjectFile.h lldb/trunk/include/lldb/Symbol/Symtab.h lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp lldb/trunk/source/Symbol/ObjectFile.cpp lldb/trunk/source/Symbol/Symtab.cpp
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp =================================================================== --- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -2332,11 +2332,11 @@ mangled.SetDemangledName( ConstString((demangled_name + suffix).str()) ); } - // In ELF all symbol should have a valid size but it is not true for some code symbols - // coming from hand written assembly. As none of the code symbol should have 0 size we try - // to calculate the size for these symbols in the symtab with saying that their original + // In ELF all symbol should have a valid size but it is not true for some function symbols + // coming from hand written assembly. As none of the function symbol should have 0 size we + // try to calculate the size for these symbols in the symtab with saying that their original // size is not valid. - bool symbol_size_valid = symbol.st_size != 0 || symbol_type != eSymbolTypeCode; + bool symbol_size_valid = symbol.st_size != 0 || symbol.getType() != STT_FUNC; Symbol dc_symbol( i + start_id, // ID is the original symbol table index. @@ -2863,6 +2863,14 @@ } } + DWARFCallFrameInfo* eh_frame = GetUnwindTable().GetEHFrameInfo(); + if (eh_frame) + { + if (m_symtab_ap == nullptr) + m_symtab_ap.reset(new Symtab(this)); + ParseUnwindSymbols (m_symtab_ap.get(), eh_frame); + } + // If we still don't have any symtab then create an empty instance to avoid do the section // lookup next time. if (m_symtab_ap == nullptr) @@ -2892,57 +2900,64 @@ return m_symtab_ap.get(); } -Symbol * -ObjectFileELF::ResolveSymbolForAddress(const Address& so_addr, bool verify_unique) +void +ObjectFileELF::ParseUnwindSymbols(Symtab *symbol_table, DWARFCallFrameInfo* eh_frame) { - if (!m_symtab_ap.get()) - return nullptr; // GetSymtab() should be called first. - - const SectionList *section_list = GetSectionList(); + SectionList* section_list = GetSectionList(); if (!section_list) - return nullptr; + return; - if (DWARFCallFrameInfo *eh_frame = GetUnwindTable().GetEHFrameInfo()) - { - AddressRange range; - if (eh_frame->GetAddressRange (so_addr, range)) + // First we save the new symbols into a separate list and add them to the symbol table after + // we colleced all symbols we want to add. This is neccessary because adding a new symbol + // invalidates the internal index of the symtab what causing the next lookup to be slow because + // it have to recalculate the index first. + std::vector<Symbol> new_symbols; + + eh_frame->ForEachFDEEntries( + [this, symbol_table, section_list, &new_symbols](lldb::addr_t file_addr, + uint32_t size, + dw_offset_t) { + Symbol* symbol = symbol_table->FindSymbolAtFileAddress(file_addr); + if (symbol) { - const addr_t file_addr = range.GetBaseAddress().GetFileAddress(); - Symbol * symbol = verify_unique ? m_symtab_ap->FindSymbolContainingFileAddress(file_addr) : nullptr; - if (symbol) - return symbol; - - // Note that a (stripped) symbol won't be found by GetSymtab()... - lldb::SectionSP eh_sym_section_sp = section_list->FindSectionContainingFileAddress(file_addr); - if (eh_sym_section_sp.get()) + if (!symbol->GetByteSizeIsValid()) { - addr_t section_base = eh_sym_section_sp->GetFileAddress(); - addr_t offset = file_addr - section_base; - uint64_t symbol_id = m_symtab_ap->GetNumSymbols(); - + symbol->SetByteSize(size); + symbol->SetSizeIsSynthesized(true); + } + } + else + { + SectionSP section_sp = section_list->FindSectionContainingFileAddress(file_addr); + if (section_sp) + { + addr_t offset = file_addr - section_sp->GetFileAddress(); + const char* symbol_name = GetNextSyntheticSymbolName().GetCString(); + uint64_t symbol_id = symbol_table->GetNumSymbols(); Symbol eh_symbol( - symbol_id, // Symbol table index. - "???", // Symbol name. - false, // Is the symbol name mangled? - eSymbolTypeCode, // Type of this symbol. - true, // Is this globally visible? - false, // Is this symbol debug info? - false, // Is this symbol a trampoline? - true, // Is this symbol artificial? - eh_sym_section_sp, // Section in which this symbol is defined or null. - offset, // Offset in section or symbol value. - range.GetByteSize(), // Size in bytes of this symbol. - true, // Size is valid. - false, // Contains linker annotations? - 0); // Symbol flags. - if (symbol_id == m_symtab_ap->AddSymbol(eh_symbol)) - return m_symtab_ap->SymbolAtIndex(symbol_id); + symbol_id, // Symbol table index. + symbol_name, // Symbol name. + false, // Is the symbol name mangled? + eSymbolTypeCode, // Type of this symbol. + true, // Is this globally visible? + false, // Is this symbol debug info? + false, // Is this symbol a trampoline? + true, // Is this symbol artificial? + section_sp, // Section in which this symbol is defined or null. + offset, // Offset in section or symbol value. + 0, // Size: Don't specify the size as an FDE can + false, // Size is valid: cover multiple symbols. + false, // Contains linker annotations? + 0); // Symbol flags. + new_symbols.push_back(eh_symbol); } } - } - return nullptr; -} + return true; + }); + for (const Symbol& s : new_symbols) + symbol_table->AddSymbol(s); +} bool ObjectFileELF::IsStripped () Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h =================================================================== --- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h +++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h @@ -150,9 +150,6 @@ lldb_private::Symtab * GetSymtab() override; - lldb_private::Symbol * - ResolveSymbolForAddress(const lldb_private::Address& so_addr, bool verify_unique) override; - bool IsStripped () override; @@ -349,6 +346,10 @@ const ELFSectionHeaderInfo *rela_hdr, lldb::user_id_t section_id); + void + ParseUnwindSymbols(lldb_private::Symtab *symbol_table, + lldb_private::DWARFCallFrameInfo* eh_frame); + /// Relocates debug sections unsigned RelocateDebugSections(const elf::ELFSectionHeader *rel_hdr, lldb::user_id_t rel_id); Index: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp =================================================================== --- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -4517,7 +4517,6 @@ if (function_starts_count > 0) { - char synthetic_function_symbol[PATH_MAX]; uint32_t num_synthetic_function_symbols = 0; for (i=0; i<function_starts_count; ++i) { @@ -4532,7 +4531,6 @@ num_syms = sym_idx + num_synthetic_function_symbols; sym = symtab->Resize (num_syms); } - uint32_t synthetic_function_symbol_idx = 0; for (i=0; i<function_starts_count; ++i) { const FunctionStarts::Entry *func_start_entry = function_starts.GetEntryAtIndex (i); @@ -4567,13 +4565,8 @@ { symbol_byte_size = section_end_file_addr - symbol_file_addr; } - snprintf (synthetic_function_symbol, - sizeof(synthetic_function_symbol), - "___lldb_unnamed_function%u$$%s", - ++synthetic_function_symbol_idx, - module_sp->GetFileSpec().GetFilename().GetCString()); sym[sym_idx].SetID (synthetic_sym_id++); - sym[sym_idx].GetMangled().SetDemangledName(ConstString(synthetic_function_symbol)); + sym[sym_idx].GetMangled().SetDemangledName(GetNextSyntheticSymbolName()); sym[sym_idx].SetType (eSymbolTypeCode); sym[sym_idx].SetIsSynthetic (true); sym[sym_idx].GetAddressRef() = symbol_addr; Index: lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp =================================================================== --- lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp +++ lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp @@ -899,3 +899,17 @@ } return false; } + +void +DWARFCallFrameInfo::ForEachFDEEntries( + const std::function<bool(lldb::addr_t, uint32_t, dw_offset_t)>& callback) +{ + GetFDEIndex(); + + for (size_t i = 0, c = m_fde_index.GetSize(); i < c; ++i) + { + const FDEEntryMap::Entry& entry = m_fde_index.GetEntryRef(i); + if (!callback(entry.base, entry.size, entry.data)) + break; + } +} Index: lldb/trunk/source/Symbol/ObjectFile.cpp =================================================================== --- lldb/trunk/source/Symbol/ObjectFile.cpp +++ lldb/trunk/source/Symbol/ObjectFile.cpp @@ -241,8 +241,7 @@ lldb::offset_t file_offset, lldb::offset_t length, const lldb::DataBufferSP& data_sp, - lldb::offset_t data_offset -) : + lldb::offset_t data_offset) : ModuleChild (module_sp), m_file (), // This file could be different from the original module's file m_type (eTypeInvalid), @@ -254,7 +253,8 @@ m_process_wp(), m_memory_addr (LLDB_INVALID_ADDRESS), m_sections_ap(), - m_symtab_ap () + m_symtab_ap (), + m_synthetic_symbol_idx (0) { if (file_spec_ptr) m_file = *file_spec_ptr; @@ -286,7 +286,8 @@ m_process_wp (process_sp), m_memory_addr (header_addr), m_sections_ap(), - m_symtab_ap () + m_symtab_ap (), + m_synthetic_symbol_idx (0) { if (header_data_sp) m_data.SetData (header_data_sp, 0, header_data_sp->GetByteSize()); @@ -653,3 +654,13 @@ } return symbol_type_hint; } + +ConstString +ObjectFile::GetNextSyntheticSymbolName() +{ + StreamString ss; + ConstString file_name = GetModule()->GetFileSpec().GetFilename(); + ss.Printf("___lldb_unnamed_symbol%u$$%s", ++m_synthetic_symbol_idx, file_name.GetCString()); + return ConstString(ss.GetData()); +} + Index: lldb/trunk/source/Symbol/Symtab.cpp =================================================================== --- lldb/trunk/source/Symbol/Symtab.cpp +++ lldb/trunk/source/Symbol/Symtab.cpp @@ -894,35 +894,6 @@ addr_t match_offset; } SymbolSearchInfo; -static int -SymbolWithClosestFileAddress (SymbolSearchInfo *info, const uint32_t *index_ptr) -{ - const Symbol *symbol = info->symtab->SymbolAtIndex (index_ptr[0]); - if (symbol == nullptr) - return -1; - - const addr_t info_file_addr = info->file_addr; - if (symbol->ValueIsAddress()) - { - const addr_t curr_file_addr = symbol->GetAddressRef().GetFileAddress(); - if (info_file_addr < curr_file_addr) - return -1; - - // Since we are finding the closest symbol that is greater than or equal - // to 'info->file_addr' we set the symbol here. This will get set - // multiple times, but after the search is done it will contain the best - // symbol match - info->match_symbol = const_cast<Symbol *>(symbol); - info->match_index_ptr = index_ptr; - info->match_offset = info_file_addr - curr_file_addr; - - if (info_file_addr > curr_file_addr) - return +1; - return 0; - } - return -1; -} - void Symtab::InitAddressIndexes() { @@ -944,43 +915,33 @@ m_file_addr_to_index.Append(entry); } } + + lldb::addr_t total_size = 0; + const size_t num_entries = m_file_addr_to_index.GetSize(); if (num_entries > 0) { m_file_addr_to_index.Sort(); - m_file_addr_to_index.CalculateSizesOfZeroByteSizeRanges(); - + // Now our last symbols might not have had sizes because there // was no subsequent symbol to calculate the size from. If this is // the case, then calculate the size by capping it at the end of the // section in which the symbol resides for (int i = num_entries - 1; i >= 0; --i) { - const FileRangeToIndexMap::Entry &entry = m_file_addr_to_index.GetEntryRef(i); - // As we iterate backwards, as soon as we find a symbol with a valid - // byte size, we are done + const FileRangeToIndexMap::Entry& entry = m_file_addr_to_index.GetEntryRef(i); + + // As we iterate backwards, as soon as we find a symbol with a valid byte size, we + // are done. if (entry.GetByteSize() > 0) break; - // Cap the size to the end of the section in which the symbol resides - SectionSP section_sp (m_objfile->GetSectionList()->FindSectionContainingFileAddress (entry.GetRangeBase())); - if (section_sp) - { - const lldb::addr_t end_section_file_addr = section_sp->GetFileAddress() + section_sp->GetByteSize(); - const lldb::addr_t symbol_file_addr = entry.GetRangeBase(); - if (end_section_file_addr > symbol_file_addr) - { - Symbol &symbol = m_symbols[entry.data]; - if (!symbol.GetByteSizeIsValid()) - { - symbol.SetByteSize(end_section_file_addr - symbol_file_addr); - symbol.SetSizeIsSynthesized(true); - } - } - } + const Address& address = m_symbols[entry.data].GetAddressRef(); + if (SectionSP section_sp = address.GetSection()) + total_size = entry.base + section_sp->GetByteSize() - address.GetOffset(); } - // Sort again in case the range size changes the ordering - m_file_addr_to_index.Sort(); + + m_file_addr_to_index.CalculateSizesOfZeroByteSizeRanges(total_size); } } } @@ -1020,37 +981,18 @@ } Symbol * -Symtab::FindSymbolContainingFileAddress (addr_t file_addr, const uint32_t* indexes, uint32_t num_indexes) +Symtab::FindSymbolAtFileAddress (addr_t file_addr) { Mutex::Locker locker (m_mutex); + if (!m_file_addr_to_index_computed) + InitAddressIndexes(); - - SymbolSearchInfo info = { this, file_addr, nullptr, nullptr, 0 }; - - ::bsearch (&info, - indexes, - num_indexes, - sizeof(uint32_t), - (ComparisonFunction)SymbolWithClosestFileAddress); - - if (info.match_symbol) + const FileRangeToIndexMap::Entry *entry = m_file_addr_to_index.FindEntryStartsAt(file_addr); + if (entry) { - if (info.match_offset == 0) - { - // We found an exact match! - return info.match_symbol; - } - - if (!info.match_symbol->GetByteSizeIsValid()) - { - // The matched symbol dosn't have a valid byte size so lets just go with that match... - return info.match_symbol; - } - - // We were able to figure out a symbol size so lets make sure our - // offset puts "file_addr" in the symbol's address range. - if (info.match_offset < info.match_symbol->GetByteSize()) - return info.match_symbol; + Symbol* symbol = SymbolAtIndex(entry->data); + if (symbol->GetFileAddress() == file_addr) + return symbol; } return nullptr; } @@ -1088,8 +1030,12 @@ for (size_t i = 0; i < addr_match_count; ++i) { - if (!callback(SymbolAtIndex(all_addr_indexes[i]))) - break; + Symbol* symbol = SymbolAtIndex(all_addr_indexes[i]); + if (symbol->ContainsFileAddress(file_addr)) + { + if (!callback(symbol)) + break; + } } } Index: lldb/trunk/include/lldb/Symbol/DWARFCallFrameInfo.h =================================================================== --- lldb/trunk/include/lldb/Symbol/DWARFCallFrameInfo.h +++ lldb/trunk/include/lldb/Symbol/DWARFCallFrameInfo.h @@ -73,6 +73,9 @@ void GetFunctionAddressAndSizeVector (FunctionAddressAndSizeVector &function_info); + void + ForEachFDEEntries(const std::function<bool(lldb::addr_t, uint32_t, dw_offset_t)>& callback); + private: enum { Index: lldb/trunk/include/lldb/Symbol/ObjectFile.h =================================================================== --- lldb/trunk/include/lldb/Symbol/ObjectFile.h +++ lldb/trunk/include/lldb/Symbol/ObjectFile.h @@ -860,6 +860,7 @@ const lldb::addr_t m_memory_addr; std::unique_ptr<lldb_private::SectionList> m_sections_ap; std::unique_ptr<lldb_private::Symtab> m_symtab_ap; + uint32_t m_synthetic_symbol_idx; //------------------------------------------------------------------ /// Sets the architecture for a module. At present the architecture @@ -873,7 +874,11 @@ /// Returns \b true if the architecture was changed, \b /// false otherwise. //------------------------------------------------------------------ - bool SetModulesArchitecture (const ArchSpec &new_arch); + bool + SetModulesArchitecture (const ArchSpec &new_arch); + + ConstString + GetNextSyntheticSymbolName(); private: DISALLOW_COPY_AND_ASSIGN (ObjectFile); Index: lldb/trunk/include/lldb/Symbol/Symtab.h =================================================================== --- lldb/trunk/include/lldb/Symbol/Symtab.h +++ lldb/trunk/include/lldb/Symbol/Symtab.h @@ -79,7 +79,7 @@ size_t FindAllSymbolsWithNameAndType (const ConstString &name, lldb::SymbolType symbol_type, Debug symbol_debug_type, Visibility symbol_visibility, std::vector<uint32_t>& symbol_indexes); size_t FindAllSymbolsMatchingRexExAndType (const RegularExpression ®ex, lldb::SymbolType symbol_type, Debug symbol_debug_type, Visibility symbol_visibility, std::vector<uint32_t>& symbol_indexes); Symbol * FindFirstSymbolWithNameAndType (const ConstString &name, lldb::SymbolType symbol_type, Debug symbol_debug_type, Visibility symbol_visibility); - Symbol * FindSymbolContainingFileAddress (lldb::addr_t file_addr, const uint32_t* indexes, uint32_t num_indexes); + Symbol * FindSymbolAtFileAddress (lldb::addr_t file_addr); Symbol * FindSymbolContainingFileAddress (lldb::addr_t file_addr); void ForEachSymbolContainingFileAddress(lldb::addr_t file_addr, std::function<bool(Symbol *)> const &callback); size_t FindFunctionSymbols (const ConstString &name, uint32_t name_type_mask, SymbolContextList& sc_list); Index: lldb/trunk/include/lldb/Core/RangeMap.h =================================================================== --- lldb/trunk/include/lldb/Core/RangeMap.h +++ lldb/trunk/include/lldb/Core/RangeMap.h @@ -1123,7 +1123,7 @@ // Calculate the byte size of ranges with zero byte sizes by finding // the next entry with a base address > the current base address void - CalculateSizesOfZeroByteSizeRanges () + CalculateSizesOfZeroByteSizeRanges (S full_size = 0) { #ifdef ASSERT_RANGEMAP_ARE_SORTED assert (IsSorted()); @@ -1148,6 +1148,8 @@ break; } } + if (next == end && full_size > curr_base) + pos->SetByteSize (full_size - curr_base); } } } @@ -1305,6 +1307,22 @@ return nullptr; } + const Entry* + FindEntryStartsAt (B addr) const + { +#ifdef ASSERT_RANGEMAP_ARE_SORTED + assert (IsSorted()); +#endif + if (!m_entries.empty()) + { + auto begin = m_entries.begin(), end = m_entries.end(); + auto pos = std::lower_bound (begin, end, Entry(addr, 1), BaseLessThan); + if (pos != end && pos->base == addr) + return &(*pos); + } + return nullptr; + } + Entry * Back() {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits