[llvm-branch-commits] [lldb] r259408 - Merging r258621, r258758, r258761:

Hans Wennborg via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Feb 1 13:21:19 PST 2016


Author: hans
Date: Mon Feb  1 15:21:19 2016
New Revision: 259408

URL: http://llvm.org/viewvc/llvm-project?rev=259408&view=rev
Log:
Merging r258621, r258758, r258761:

------------------------------------------------------------------------
r258621 | mohit.bhakkad | 2016-01-23 02:36:06 -0800 (Sat, 23 Jan 2016) | 5 lines

[LLDB] Consider only valid symbols while resolving by address

Reviewers: clayborg.
Subscribers: jaydeep, bhushan, sagar, nitesh.jain, lldb-commits.
Differential Revision: http://reviews.llvm.org/D16397
------------------------------------------------------------------------

------------------------------------------------------------------------
r258758 | amccarth | 2016-01-25 16:58:09 -0800 (Mon, 25 Jan 2016) | 13 lines

Set symbol types for function symbols loaded from PE/COFF

This fixes the regression of several tests on Windows after rL258621.

The root problem is that ObjectFilePECOFF was not setting type information for the symbols, and the new CL rejects symbols without type information, breaking functionality like thread step-over.

The fix sets the type information for functions (and creates a TODO for other types).

Along the way, I fixed some typos and formatting that made the code I was debugging harder to understand.

In the long run, we should consider replacing most of ObjectFilePECOFF with the COFF parsing code from LLVM.

Differential Revision: http://reviews.llvm.org/D16563
------------------------------------------------------------------------

------------------------------------------------------------------------
r258761 | zturner | 2016-01-25 17:09:38 -0800 (Mon, 25 Jan 2016) | 1 line

Remove XFAIL Windows from a test that was fixed by r258758.
------------------------------------------------------------------------

Modified:
    lldb/branches/release_38/   (props changed)
    lldb/branches/release_38/include/lldb/Core/RangeMap.h
    lldb/branches/release_38/include/lldb/Symbol/Symtab.h
    lldb/branches/release_38/packages/Python/lldbsuite/test/python_api/function_symbol/TestSymbolAPI.py
    lldb/branches/release_38/source/Core/Module.cpp
    lldb/branches/release_38/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
    lldb/branches/release_38/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
    lldb/branches/release_38/source/Symbol/Symtab.cpp

Propchange: lldb/branches/release_38/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Feb  1 15:21:19 2016
@@ -1,3 +1,3 @@
 /lldb/branches/apple/python-GIL:156467-162159
 /lldb/branches/iohandler:198360-200250
-/lldb/trunk:257691-257692,258684-258685,259188
+/lldb/trunk:257691-257692,258621,258684-258685,258758,258761,259188

Modified: lldb/branches/release_38/include/lldb/Core/RangeMap.h
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/include/lldb/Core/RangeMap.h?rev=259408&r1=259407&r2=259408&view=diff
==============================================================================
--- lldb/branches/release_38/include/lldb/Core/RangeMap.h (original)
+++ lldb/branches/release_38/include/lldb/Core/RangeMap.h Mon Feb  1 15:21:19 2016
@@ -1216,6 +1216,25 @@ namespace lldb_private {
             }
             return UINT32_MAX;
         }
+
+        uint32_t
+        FindEntryIndexesThatContain(B addr, std::vector<uint32_t> &indexes) const
+        {
+#ifdef ASSERT_RANGEMAP_ARE_SORTED
+            assert (IsSorted());
+#endif
+
+            if (!m_entries.empty())
+            {
+                typename Collection::const_iterator pos;
+                for (const auto &entry : m_entries)
+                {
+                    if (entry.Contains(addr))
+                        indexes.push_back(entry.data);
+                }
+            }
+            return indexes.size() ;
+        }
         
         Entry *
         FindEntryThatContains (B addr)

Modified: lldb/branches/release_38/include/lldb/Symbol/Symtab.h
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/include/lldb/Symbol/Symtab.h?rev=259408&r1=259407&r2=259408&view=diff
==============================================================================
--- lldb/branches/release_38/include/lldb/Symbol/Symtab.h (original)
+++ lldb/branches/release_38/include/lldb/Symbol/Symtab.h Mon Feb  1 15:21:19 2016
@@ -81,6 +81,7 @@ public:
             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 *    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);
             void        CalculateSymbolSizes ();
 

Modified: lldb/branches/release_38/packages/Python/lldbsuite/test/python_api/function_symbol/TestSymbolAPI.py
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/packages/Python/lldbsuite/test/python_api/function_symbol/TestSymbolAPI.py?rev=259408&r1=259407&r2=259408&view=diff
==============================================================================
--- lldb/branches/release_38/packages/Python/lldbsuite/test/python_api/function_symbol/TestSymbolAPI.py (original)
+++ lldb/branches/release_38/packages/Python/lldbsuite/test/python_api/function_symbol/TestSymbolAPI.py Mon Feb  1 15:21:19 2016
@@ -24,7 +24,6 @@ class SymbolAPITestCase(TestBase):
         self.line2 = line_number('main.c', '// Find the line number for breakpoint 2 here.')
 
     @add_test_categories(['pyapi'])
-    @expectedFailureWindows("llvm.org/pr24778")
     def test(self):
         """Exercise some SBSymbol and SBAddress APIs."""
         self.build()

Modified: lldb/branches/release_38/source/Core/Module.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/source/Core/Module.cpp?rev=259408&r1=259407&r2=259408&view=diff
==============================================================================
--- lldb/branches/release_38/source/Core/Module.cpp (original)
+++ lldb/branches/release_38/source/Core/Module.cpp Mon Feb  1 15:21:19 2016
@@ -559,7 +559,18 @@ Module::ResolveSymbolContextForAddress (
             Symtab *symtab = sym_vendor->GetSymtab();
             if (symtab && so_addr.IsSectionOffset())
             {
-                sc.symbol = symtab->FindSymbolContainingFileAddress(so_addr.GetFileAddress());
+                Symbol *matching_symbol = nullptr;
+
+                symtab->ForEachSymbolContainingFileAddress(so_addr.GetFileAddress(),
+                                                           [&matching_symbol](Symbol *symbol) -> bool {
+                                                               if (symbol->GetType() != eSymbolTypeInvalid)
+                                                               {
+                                                                   matching_symbol = symbol;
+                                                                   return false; // Stop iterating
+                                                               }
+                                                               return true; // Keep iterating
+                                                           });
+                sc.symbol = matching_symbol;
                 if (!sc.symbol &&
                     resolve_scope & eSymbolContextFunction && !(resolved_flags & eSymbolContextFunction))
                 {

Modified: lldb/branches/release_38/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp?rev=259408&r1=259407&r2=259408&view=diff
==============================================================================
--- lldb/branches/release_38/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp (original)
+++ lldb/branches/release_38/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp Mon Feb  1 15:21:19 2016
@@ -169,6 +169,18 @@ ObjectFilePECOFF::MagicBytesMatch (DataB
     return magic == IMAGE_DOS_SIGNATURE;
 }
 
+lldb::SymbolType
+ObjectFilePECOFF::MapSymbolType(uint16_t coff_symbol_type)
+{
+    // TODO:  We need to complete this mapping of COFF symbol types to LLDB ones.
+    // For now, here's a hack to make sure our function have types.
+    const auto complex_type = coff_symbol_type >> llvm::COFF::SCT_COMPLEX_TYPE_SHIFT;
+    if (complex_type == llvm::COFF::IMAGE_SYM_DTYPE_FUNCTION)
+    {
+        return lldb::eSymbolTypeCode;
+    }
+    return lldb::eSymbolTypeInvalid;
+}
 
 ObjectFilePECOFF::ObjectFilePECOFF (const lldb::ModuleSP &module_sp, 
                                     DataBufferSP& data_sp,
@@ -534,8 +546,8 @@ ObjectFilePECOFF::GetSymtab()
             {
                 const uint32_t symbol_size = 18;
                 const uint32_t addr_byte_size = GetAddressByteSize ();
-                const size_t symbol_data_size = num_syms * symbol_size; 
-                // Include the 4 bytes string table size at the end of the symbols
+                const size_t symbol_data_size = num_syms * symbol_size;
+                // Include the 4-byte string table size at the end of the symbols
                 DataBufferSP symtab_data_sp(m_file.ReadFileContents (m_coff_header.symoff, symbol_data_size + 4));
                 DataExtractor symtab_data (symtab_data_sp, GetByteOrder(), addr_byte_size);
                 lldb::offset_t offset = symbol_data_size;
@@ -556,8 +568,8 @@ ObjectFilePECOFF::GetSymtab()
                     coff_symbol_t symbol;
                     const uint32_t symbol_offset = offset;
                     const char *symbol_name_cstr = NULL;
-                    // If the first 4 bytes of the symbol string are zero, then we
-                    // it is followed by a 4 byte string table offset. Else these
+                    // If the first 4 bytes of the symbol string are zero, then they
+                    // are followed by a 4-byte string table offset. Else these
                     // 8 bytes contain the symbol name
                     if (symtab_data.GetU32 (&offset) == 0)
                     {
@@ -586,6 +598,7 @@ ObjectFilePECOFF::GetSymtab()
                     {
                         Address symbol_addr(sect_list->GetSectionAtIndex(symbol.sect-1), symbol.value);
                         symbols[i].GetAddressRef() = symbol_addr;
+                        symbols[i].SetType(MapSymbolType(symbol.type));
                     }
 
                     if (symbol.naux > 0)

Modified: lldb/branches/release_38/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h?rev=259408&r1=259407&r2=259408&view=diff
==============================================================================
--- lldb/branches/release_38/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h (original)
+++ lldb/branches/release_38/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h Mon Feb  1 15:21:19 2016
@@ -101,7 +101,10 @@ public:
 
     static bool
     MagicBytesMatch (lldb::DataBufferSP& data_sp);
-    
+
+    static lldb::SymbolType
+    MapSymbolType(uint16_t coff_symbol_type);
+
     bool
     ParseHeader() override;
     
@@ -116,7 +119,7 @@ public:
     
     uint32_t
     GetAddressByteSize() const override;
-    
+
 //    virtual lldb_private::AddressClass
 //    GetAddressClass (lldb::addr_t file_addr);
 

Modified: lldb/branches/release_38/source/Symbol/Symtab.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/source/Symbol/Symtab.cpp?rev=259408&r1=259407&r2=259408&view=diff
==============================================================================
--- lldb/branches/release_38/source/Symbol/Symtab.cpp (original)
+++ lldb/branches/release_38/source/Symbol/Symtab.cpp Mon Feb  1 15:21:19 2016
@@ -1071,6 +1071,26 @@ Symtab::FindSymbolContainingFileAddress
 }
 
 void
+Symtab::ForEachSymbolContainingFileAddress(addr_t file_addr, std::function<bool(Symbol *)> const &callback)
+{
+    Mutex::Locker locker (m_mutex);
+
+    if (!m_file_addr_to_index_computed)
+        InitAddressIndexes();
+
+    std::vector<uint32_t> all_addr_indexes;
+
+    // Get all symbols with file_addr
+    const size_t addr_match_count = m_file_addr_to_index.FindEntryIndexesThatContain(file_addr, all_addr_indexes);
+
+    for (size_t i = 0; i < addr_match_count; ++i)
+    {
+        if (!callback(SymbolAtIndex(all_addr_indexes[i])))
+        break;
+    }
+}
+
+void
 Symtab::SymbolIndicesToSymbolContextList (std::vector<uint32_t> &symbol_indexes, SymbolContextList &sc_list)
 {
     // No need to protect this call using m_mutex all other method calls are




More information about the llvm-branch-commits mailing list