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> &section_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

Reply via email to