Page MenuHomePhabricator

[LLDB] Consider only valid symbols while resolving by address
ClosedPublic

Authored by mohit.bhakkad on Jan 21 2016, 2:36 AM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

mohit.bhakkad retitled this revision from to [LLDB] Consider only valid symbols while resolving by address.
mohit.bhakkad updated this object.
mohit.bhakkad added a reviewer: clayborg.
mohit.bhakkad set the repository for this revision to rL LLVM.
clayborg requested changes to this revision.Jan 21 2016, 10:48 AM
clayborg edited edge metadata.

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
});
This revision now requires changes to proceed.Jan 21 2016, 10:48 AM
mohit.bhakkad edited edge metadata.

Changed as suggested.

clayborg accepted this revision.Jan 22 2016, 10:43 AM
clayborg edited edge metadata.

Looks good.

This revision is now accepted and ready to land.Jan 22 2016, 10:43 AM
mohit.bhakkad edited edge metadata.Jan 23 2016, 2:38 AM
mohit.bhakkad added a subscriber: lldb-commits.
This revision was automatically updated to reflect the committed changes.

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

Addresss has too many ss.

1092

Indentation problem.

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"