jasonmolenda created this revision. jasonmolenda added a reviewer: clayborg. jasonmolenda added a subscriber: lldb-commits. jasonmolenda set the repository for this revision to rL LLVM.
The Symtab has an array of symbol file addresses and sizes for address-to-symbol lookups, created in Symtab::InitAddressIndexes. The Symtab has a vector of symbol (m_symbols) and a parallel vector of Range entries (m_file_addr_to_index - the index being an index into m_symbols). Once the m_symbols have been filled in, it makes a pass over them and creates the m_file_addr_to_index entries. Then it does additional passes to fill in the sizes of the Range entries based on the other symbol start addresses. I'm working on a problem in an environment where the sections of a file are noncontiguous in memory - so we can have a gap of hundreds of megabytes between the sections, and the last symbol in the first section gets a 400MB size. If the unwinder (or one of the fallback unwind methods) happens to come up with an address in that 400MB gap, lldb will try to disassemble that entire section, thinking it is a function. Before this patch, InitAddressIndexes would create the m_file_addr_to_index array, then set sizes based on the next symbol, then use the last section's end address to set the last symbol(s) in the array. In this patch, I'm using the smaller of the containing section end OR the next symbol's address to determine the symbol's size. Looking up the section for each symbol would be expensive, so I make a local RangeVector with all of the section address/sizes before I loop over the entries. RangeVector::Sort sorts by start address and then by size so we'll get the smallest containing section for a given lookup. Repository: rL LLVM http://reviews.llvm.org/D19004 Files: include/lldb/Core/RangeMap.h source/Symbol/Symtab.cpp
Index: source/Symbol/Symtab.cpp =================================================================== --- source/Symbol/Symtab.cpp +++ source/Symbol/Symtab.cpp @@ -947,6 +947,33 @@ return -1; } +// Add all the section file start address & size to the RangeVector, +// recusively adding any children sections. +static void +AddSectionsToRangeMap (SectionList *sectlist, RangeVector<addr_t, addr_t> §ion_ranges) +{ + const int num_sections = sectlist->GetNumSections (0); + for (int i = 0; i < num_sections; i++) + { + SectionSP sect_sp = sectlist->GetSectionAtIndex (i); + if (sect_sp) + { + addr_t base_addr = sect_sp->GetFileAddress(); + size_t size = sect_sp->GetByteSize(); + RangeVector<addr_t, addr_t>::Entry entry; + entry.SetRangeBase (base_addr); + entry.SetByteSize (size); + section_ranges.Append (entry); + + SectionList &child_sectlist = sect_sp->GetChildren(); + if (child_sectlist.GetNumSections (0) > 0) + { + AddSectionsToRangeMap (&child_sectlist, section_ranges); + } + } + } +} + void Symtab::InitAddressIndexes() { @@ -972,37 +999,69 @@ 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) + + // Create a RangeVector with the start & size of all the sections for + // this objfile. We'll need to check this for any FileRangeToIndexMap + // entries with an uninitialized size, which could potentially be a + // large number so reconstituting the weak pointer is busywork when it + // is invariant information. + SectionList *sectlist = m_objfile->GetSectionList(); + RangeVector<addr_t, addr_t> section_ranges; + if (sectlist) { - 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; + AddSectionsToRangeMap (sectlist, section_ranges); + section_ranges.Sort(); + } - // 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) + // Iterate through the FileRangeToIndexMap and fill in the size for any + // entries that didn't already have a size from the Symbol (e.g. if we + // have a plain linker symbol with an address only, instead of debug info + // where we get an address and a size and a type, etc.) + for (int i = 0; i < num_entries; i++) + { + FileRangeToIndexMap::Entry *entry = m_file_addr_to_index.GetMutableEntryAtIndex (i); + if (entry->GetByteSize() == 0) { - 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) + addr_t curr_base_addr = entry->GetRangeBase(); + const RangeVector<addr_t, addr_t>::Entry *containing_section = + section_ranges.FindEntryThatContains (curr_base_addr); + + // Use the end of the section as the default max size of the symbol + addr_t sym_size = 0; + if (containing_section) { - Symbol &symbol = m_symbols[entry.data]; - if (!symbol.GetByteSizeIsValid()) + sym_size = containing_section->GetByteSize() - + (entry->GetRangeBase() - containing_section->GetRangeBase()); + } + + for (int j = i; j < num_entries; j++) + { + FileRangeToIndexMap::Entry *next_entry = m_file_addr_to_index.GetMutableEntryAtIndex (j); + addr_t next_base_addr = next_entry->GetRangeBase(); + if (next_base_addr > curr_base_addr) { - symbol.SetByteSize(end_section_file_addr - symbol_file_addr); - symbol.SetSizeIsSynthesized(true); + addr_t size_to_next_symbol = next_base_addr - curr_base_addr; + + // Take the difference between this symbol and the next one as its size, + // if it is less than the size of the section. + if (sym_size == 0 || size_to_next_symbol < sym_size) + { + sym_size = size_to_next_symbol; + } + break; } } + + if (sym_size > 0) + { + entry->SetByteSize (sym_size); + Symbol &symbol = m_symbols[entry->data]; + symbol.SetByteSize (sym_size); + symbol.SetSizeIsSynthesized (true); + } } } + // Sort again in case the range size changes the ordering m_file_addr_to_index.Sort(); } Index: include/lldb/Core/RangeMap.h =================================================================== --- include/lldb/Core/RangeMap.h +++ include/lldb/Core/RangeMap.h @@ -1181,7 +1181,13 @@ { return ((i < m_entries.size()) ? &m_entries[i] : nullptr); } - + + Entry * + GetMutableEntryAtIndex (size_t i) + { + return ((i < m_entries.size()) ? &m_entries[i] : nullptr); + } + // Clients must ensure that "i" is a valid index prior to calling this function const Entry & GetEntryRef (size_t i) const
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits