[lldb] Make symbol list output from `image dump symtab` not depend on internal ordering of DenseMap

`image dump symtab` seems to output the symbols in whatever order they appear in
the DenseMap that is used to filter out symbols with non-unique addresses. As
DenseMap is a hash map this order can change at any time so the output of this
command is pretty unstable. This also causes the `Breakpad/symtab.test` to fail
with enabled reverse iteration (which reverses the DenseMap order to find issues
like this).

This patch makes the DenseMap a std::vector and uses a separate DenseSet to do
the address filtering. The output order is now dependent on the order in which
the symbols are read (which should be deterministic). It might also avoid a bit
of work as all the work for creating the Symbol constructor parameters is only
done when we can actually emplace a new Symbol.

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D87036
This commit is contained in:
Raphael Isemann 2020-09-03 10:26:33 +02:00
parent 13cde6733b
commit 5b354d204d
2 changed files with 14 additions and 9 deletions

View File

@ -326,7 +326,8 @@ void SymbolFileBreakpad::AddSymbols(Symtab &symtab) {
}
const SectionList &list = *module.GetSectionList();
llvm::DenseMap<addr_t, Symbol> symbols;
llvm::DenseSet<addr_t> found_symbol_addresses;
std::vector<Symbol> symbols;
auto add_symbol = [&](addr_t address, llvm::Optional<addr_t> size,
llvm::StringRef name) {
address += base;
@ -338,8 +339,12 @@ void SymbolFileBreakpad::AddSymbols(Symtab &symtab) {
name, address);
return;
}
symbols.try_emplace(
address, /*symID*/ 0, Mangled(name), eSymbolTypeCode,
// Keep track of what addresses were already added so far and only add
// the symbol with the first address.
if (!found_symbol_addresses.insert(address).second)
return;
symbols.emplace_back(
/*symID*/ 0, Mangled(name), eSymbolTypeCode,
/*is_global*/ true, /*is_debug*/ false,
/*is_trampoline*/ false, /*is_artificial*/ false,
AddressRange(section_sp, address - section_sp->GetFileAddress(),
@ -359,8 +364,8 @@ void SymbolFileBreakpad::AddSymbols(Symtab &symtab) {
LLDB_LOG(log, "Failed to parse: {0}. Skipping record.", line);
}
for (auto &KV : symbols)
symtab.AddSymbol(std::move(KV.second));
for (Symbol &symbol : symbols)
symtab.AddSymbol(std::move(symbol));
symtab.CalculateSymbolSizes();
}

View File

@ -6,10 +6,10 @@
# CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 5:
# CHECK: Index UserID DSX Type File Address/Value Load Address Size Flags Name
# CHECK: [ 0] 0 SX Code 0x0000000000400000 0x00000000000000b0 0x00000000 ___lldb_unnamed_symbol{{[0-9]*}}$$symtab.out
# CHECK: [ 1] 0 X Code 0x00000000004000c0 0x0000000000000010 0x00000000 f2
# CHECK: [ 2] 0 X Code 0x00000000004000d0 0x0000000000000022 0x00000000 _start
# CHECK: [ 3] 0 X Code 0x00000000004000a0 0x000000000000000d 0x00000000 func_only
# CHECK: [ 4] 0 X Code 0x00000000004000b0 0x000000000000000c 0x00000000 f1_func
# CHECK: [ 1] 0 X Code 0x00000000004000b0 0x000000000000000c 0x00000000 f1_func
# CHECK: [ 2] 0 X Code 0x00000000004000a0 0x000000000000000d 0x00000000 func_only
# CHECK: [ 3] 0 X Code 0x00000000004000c0 0x0000000000000010 0x00000000 f2
# CHECK: [ 4] 0 X Code 0x00000000004000d0 0x0000000000000022 0x00000000 _start
# CHECK-LABEL: (lldb) image lookup -a 0x4000b0 -v
# CHECK: Address: symtab.out[0x00000000004000b0] (symtab.out.PT_LOAD[0]..text2 + 0)