[LLDB] Consider only valid symbols while resolving by address

[mohit.bhakkad]

Summary

There are more than one symbols in symbol table with same file/load address.

Function FindSymbolContainingFileAddress is returning the first symbol with given address
in symbol table, even if it is of type Invalid.

This is causing issues while determining dynamic type of a variable or while providing summery
of image lookups, i.e. any place we get Address->Symbol.

This patch is providing a new function FindValidSymbolContainingFileAddress which considers all
the values with given address, and provides the first valid one.

[clayborg]

I would rather see this implemented using a ForEach type call that uses a std::function callback. That way we don't need to keep adding new variants that search for specific symbol types (like ignoring invalid symbols).

void
Symtab::ForEachSymbolContainingFileAddresss (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.FindEntryIndexesThatContains(file_addr, all_addr_indexes);

    for (size_t i=0; i<addr_match_count; ++i)
    {
        if (!callback(SymbolAtIndex(all_addr_indexes[i])))
            break;
    }
    return nullptr;
}

Then you can call this with a lambda:

Symbol *matching_symbol = nullptr;
symtab->ForEachSymbolContainingFileAddresss (file_addr, [&matching_symbol](Symbol *symbol) -> bool {
    if (symbol->GetType() != eSymbolTypeInvalid)
    {
        matching_symbol = symbol;
        return false; // Stop iterating
    }
    return true; // Keep iterating
});

[mohit.bhakkad]

Changed as suggested.

[clayborg]

Looks good.

[amccarth]

This patch broke approximately 10 tests on Windows, mostly related to thread step-over, thread step-out, etc. We probably need to fix something in PECOFF::GetSymTab that's causing all the symbols to be rejected. I'll look into it more.

lldb/trunk/source/Symbol/Symtab.cpp

1077 ↗

(On Diff #45789)

Addresss has too many ss.

1092 ↗

(On Diff #45789)

Indentation problem.

[zturner]

Also various other formatting problems. Please run clang-format in the
future.

Steps to running clang-format

1. Compile it with "ninja clang-format"
2. Add your bin directory to PATH
3. Stage your changes with git add as you normally do
4. Run "git clang-format"

include/lldb/Core/RangeMap.h

#endif
                 while(pos != begin && pos[-1].Contains(addr))
                     --pos;
 
                 if (pos != end && pos->Contains(addr))
                     return std::distance (begin, pos);
             }
             return UINT32_MAX;
         }
-        
+        uint32_t
+        FindEntryIndexesThatContains (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(pos = m_entries.begin(); pos != m_entries.end(); pos++)
+                {
+                    if (pos->Contains(addr))
+                        indexes.push_back (pos->data);
+                }
+            }
+            return indexes.size() ;
+        }
+        
         Entry *
         FindEntryThatContains (B addr)
         {
 #ifdef ASSERT_RANGEMAP_ARE_SORTED
             assert (IsSorted());
 #endif
             if ( !m_entries.empty() )
             {

include/lldb/Symbol/Symtab.h

const   Symbol *    GetParent (Symbol *symbol) const;
             uint32_t    AppendSymbolIndexesMatchingRegExAndType (const RegularExpression &regex, lldb::SymbolType symbol_type, std::vector<uint32_t>& indexes);
             uint32_t    AppendSymbolIndexesMatchingRegExAndType (const RegularExpression &regex, lldb::SymbolType symbol_type, Debug symbol_debug_type, Visibility symbol_visibility, std::vector<uint32_t>& indexes);
             size_t      FindAllSymbolsWithNameAndType (const ConstString &name, lldb::SymbolType symbol_type, std::vector<uint32_t>& symbol_indexes);
             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 &regex, 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 *    FindSymbolContainingFileAddress (lldb::addr_t file_addr);
+            void        ForEachSymbolContainingFileAddresss (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 ();
 
             void        SortSymbolIndexesByValue (std::vector<uint32_t>& indexes, bool remove_duplicates) const;
 
     static  void        DumpSymbolHeader (Stream *s);
 
     

source/Core/Module.cpp

if (section_sp && section_sp->GetModule().get() == this)
         }
 
         // Resolve the symbol if requested, but don't re-look it up if we've already found it.
         if (resolve_scope & eSymbolContextSymbol && !(resolved_flags & eSymbolContextSymbol))
         {
             Symtab *symtab = sym_vendor->GetSymtab();
             if (symtab && so_addr.IsSectionOffset())
             {
-                sc.symbol = symtab->FindSymbolContainingFileAddress(so_addr.GetFileAddress());
+                Symbol *matching_symbol = nullptr;
+                symtab->ForEachSymbolContainingFileAddresss (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))
                 {
                     bool verify_unique = false; // No need to check again since ResolveSymbolContext failed to find a symbol at this address.
                     if (ObjectFile *obj_file = sc.module_sp->GetObjectFile())
                         sc.symbol = obj_file->ResolveSymbolForAddress(so_addr, verify_unique);
                 }
 

source/Symbol/Symtab.cpp

if (entry)
         Symbol* symbol = SymbolAtIndex(entry->data);
         if (symbol->ContainsFileAddress(file_addr))
             return symbol;
     }
     return nullptr;
 }
 
 void
+Symtab::ForEachSymbolContainingFileAddresss (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.FindEntryIndexesThatContains(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
     // already thread safe.
     
     const bool merge_symbol_into_function = true;
     size_t num_indices = symbol_indexes.size();
     if (num_indices > 0)