tberghammer created this revision.
tberghammer added a reviewer: clayborg.
tberghammer added a subscriber: lldb-commits.

Unconditionally accept symbol sizes from .dynsym

If an elf object file have no .symtab section then we create our symtab
representation based on the .dynsym section. The symtab creation have a
heuristic what updates the size of symbols with 0 size to last until the
beginning of the next symbol what is valid when we have full symbol table
but it isn't valid whit .dynsym because of the missing private symbols.
As a result if we have symbols with 0 size in the .dynsym we will expand
them until the next symbol and these artificially created symbol sizes will
confuse the stack unwinder.

This change disables the artificial symbol size calculation for the scenario
when we are using .dynsym.

http://reviews.llvm.org/D16186

Files:
  include/lldb/Symbol/Symbol.h
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  source/Symbol/Symbol.cpp
  source/Symbol/Symtab.cpp

Index: source/Symbol/Symtab.cpp
===================================================================
--- source/Symbol/Symtab.cpp
+++ source/Symbol/Symtab.cpp
@@ -971,9 +971,11 @@
                     if (end_section_file_addr > symbol_file_addr)
                     {
                         Symbol &symbol = m_symbols[entry.data];
-
-                        symbol.SetByteSize(end_section_file_addr - symbol_file_addr);
-                        symbol.SetSizeIsSynthesized(true);
+                        if (!symbol.GetByteSizeIsValid())
+                        {
+                            symbol.SetByteSize(end_section_file_addr - symbol_file_addr);
+                            symbol.SetSizeIsSynthesized(true);
+                        }
                     }
                 }
             }
@@ -1039,18 +1041,15 @@
             return info.match_symbol;
         }
 
-        const size_t symbol_byte_size = info.match_symbol->GetByteSize();
-        
-        if (symbol_byte_size == 0)
+        if (!info.match_symbol->GetByteSizeIsValid())
         {
-            // We weren't able to find the size of the symbol so lets just go 
-            // with that match we found in our search...
+            // 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 < symbol_byte_size)
+        if (info.match_offset < info.match_symbol->GetByteSize())
             return info.match_symbol;
     }
     return nullptr;
@@ -1066,7 +1065,11 @@
 
     const FileRangeToIndexMap::Entry *entry = m_file_addr_to_index.FindEntryThatContains(file_addr);
     if (entry)
-        return SymbolAtIndex(entry->data);
+    {
+        Symbol* symbol = SymbolAtIndex(entry->data);
+        if (symbol->ContainsFileAddress(file_addr))
+            return symbol;
+    }
     return nullptr;
 }
 
Index: source/Symbol/Symbol.cpp
===================================================================
--- source/Symbol/Symbol.cpp
+++ source/Symbol/Symbol.cpp
@@ -737,3 +737,10 @@
     }
     return false;
 }
+
+bool
+Symbol::ContainsFileAddress (lldb::addr_t file_addr) const
+{
+    return m_addr_range.ContainsFileAddress(file_addr);
+}
+
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -327,16 +327,18 @@
     unsigned
     ParseSymbolTable(lldb_private::Symtab *symbol_table,
                      lldb::user_id_t start_id,
-                     lldb_private::Section *symtab);
+                     lldb_private::Section *symtab,
+                     bool is_dynsym);
 
     /// Helper routine for ParseSymbolTable().
     unsigned
     ParseSymbols(lldb_private::Symtab *symbol_table, 
                  lldb::user_id_t start_id,
                  lldb_private::SectionList *section_list,
                  const size_t num_symbols,
                  const lldb_private::DataExtractor &symtab_data,
-                 const lldb_private::DataExtractor &strtab_data);
+                 const lldb_private::DataExtractor &strtab_data,
+                 bool is_dynsym);
 
     /// Scans the relocation entries and adds a set of artificial symbols to the
     /// given symbol table for each PLT slot.  Returns the number of symbols
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1979,7 +1979,8 @@
                              SectionList *section_list,
                              const size_t num_symbols,
                              const DataExtractor &symtab_data,
-                             const DataExtractor &strtab_data)
+                             const DataExtractor &strtab_data,
+                             bool is_dynsym)
 {
     ELFSymbol symbol;
     lldb::offset_t offset = 0;
@@ -2283,6 +2284,12 @@
                 mangled.SetDemangledName( ConstString((demangled_name + suffix).str()) );
         }
 
+        // If we are parsing the symbol table from the .dynsym section then we can't calculate the
+        // size of the symbol based on the next item because the next symbol might be removed. To
+        // handle this we accept the symbol size specified in the symtab even if it is 0 as 0 sized
+        // symbols are valid in some situation
+        bool symbol_size_is_valid = is_dynsym || symbol.st_size != 0;
+
         Symbol dc_symbol(
             i + start_id,       // ID is the original symbol table index.
             mangled,
@@ -2295,23 +2302,26 @@
                 symbol_section_sp,  // Section in which this symbol is defined or null.
                 symbol_value,       // Offset in section or symbol value.
                 symbol.st_size),    // Size in bytes of this symbol.
-            symbol.st_size != 0,    // Size is valid if it is not 0
+            symbol_size_is_valid,
             has_suffix,             // Contains linker annotations?
             flags);                 // Symbol flags.
         symtab->AddSymbol(dc_symbol);
     }
     return i;
 }
 
 unsigned
-ObjectFileELF::ParseSymbolTable(Symtab *symbol_table, user_id_t start_id, lldb_private::Section *symtab)
+ObjectFileELF::ParseSymbolTable(Symtab *symbol_table,
+                                user_id_t start_id,
+                                lldb_private::Section *symtab,
+                                bool is_dynsym)
 {
     if (symtab->GetObjectFile() != this)
     {
         // If the symbol table section is owned by a different object file, have it do the
         // parsing.
         ObjectFileELF *obj_file_elf = static_cast<ObjectFileELF *>(symtab->GetObjectFile());
-        return obj_file_elf->ParseSymbolTable (symbol_table, start_id, symtab);
+        return obj_file_elf->ParseSymbolTable (symbol_table, start_id, symtab, is_dynsym);
     }
 
     // Get section list for this object file.
@@ -2342,7 +2352,7 @@
             size_t num_symbols = symtab_data.GetByteSize() / symtab_hdr->sh_entsize;
 
             return ParseSymbols(symbol_table, start_id, section_list,
-                                num_symbols, symtab_data, strtab_data);
+                                num_symbols, symtab_data, strtab_data, is_dynsym);
         }
     }
 
@@ -2767,17 +2777,19 @@
         // version of the symtab that only contains global symbols. The information found
         // in the dynsym is therefore also found in the symtab, while the reverse is not
         // necessarily true.
+        bool is_dynsym = false;
         Section *symtab = section_list->FindSectionByType (eSectionTypeELFSymbolTable, true).get();
         if (!symtab)
         {
             // The symtab section is non-allocable and can be stripped, so if it doesn't exist
             // then use the dynsym section which should always be there.
+            is_dynsym = true;
             symtab = section_list->FindSectionByType (eSectionTypeELFDynamicSymbols, true).get();
         }
         if (symtab)
         {
             m_symtab_ap.reset(new Symtab(symtab->GetObjectFile()));
-            symbol_id += ParseSymbolTable (m_symtab_ap.get(), symbol_id, symtab);
+            symbol_id += ParseSymbolTable (m_symtab_ap.get(), symbol_id, symtab, is_dynsym);
         }
 
         // DT_JMPREL
Index: include/lldb/Symbol/Symbol.h
===================================================================
--- include/lldb/Symbol/Symbol.h
+++ include/lldb/Symbol/Symbol.h
@@ -383,6 +383,9 @@
                     bool prefer_file_cache,
                     Stream &strm);
 
+    bool
+    ContainsFileAddress (lldb::addr_t file_addr) const;
+
 protected:
     // This is the internal guts of ResolveReExportedSymbol, it assumes reexport_name is not null, and that module_spec
     // is valid.  We track the modules we've already seen to make sure we don't get caught in a cycle.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to