From 5b354d204d0952a6dd39e41fb41b51414bff5f0b Mon Sep 17 00:00:00 2001 From: Raphael Isemann Date: Thu, 3 Sep 2020 10:26:33 +0200 Subject: [PATCH] [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 --- .../SymbolFile/Breakpad/SymbolFileBreakpad.cpp | 15 ++++++++++----- lldb/test/Shell/SymbolFile/Breakpad/symtab.test | 8 ++++---- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp index eeec7296747e..07e5b284eab8 100644 --- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp +++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp @@ -326,7 +326,8 @@ void SymbolFileBreakpad::AddSymbols(Symtab &symtab) { } const SectionList &list = *module.GetSectionList(); - llvm::DenseMap symbols; + llvm::DenseSet found_symbol_addresses; + std::vector symbols; auto add_symbol = [&](addr_t address, llvm::Optional 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(); } diff --git a/lldb/test/Shell/SymbolFile/Breakpad/symtab.test b/lldb/test/Shell/SymbolFile/Breakpad/symtab.test index a02d94c30aa3..1eb03fa43deb 100644 --- a/lldb/test/Shell/SymbolFile/Breakpad/symtab.test +++ b/lldb/test/Shell/SymbolFile/Breakpad/symtab.test @@ -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)