[lldb] Fix lookup of symbols with the same address range but different binding

jankratochvil · jankratochvil · commit bf7225888a99 · 2020-01-13T12:05:26.000+01:00
This fixes a failing testcase on Fedora 30 x86_64 (regression Fedora 29->30): PASS: ./bin/lldb ./lldb-test-build.noindex/functionalities/unwind/noreturn/TestNoreturnUnwind.test_dwarf/a.out -o 'settings set symbols.enable-external-lookup false' -o r -o bt -o quit * frame #0: 0x00007ffff7aa6e75 libc.so.6`__GI_raise + 325 frame #1: 0x00007ffff7a91895 libc.so.6`__GI_abort + 295 frame #2: 0x0000000000401140 a.out`func_c at main.c:12:2 frame #3: 0x000000000040113a a.out`func_b at main.c:18:2 frame #4: 0x0000000000401134 a.out`func_a at main.c:26:2 frame #5: 0x000000000040112e a.out`main(argc=<unavailable>, argv=<unavailable>) at main.c:32:2 frame #6: 0x00007ffff7a92f33 libc.so.6`__libc_start_main + 243 frame #7: 0x000000000040106e a.out`_start + 46 vs. FAIL - unrecognized abort() function: ./bin/lldb ./lldb-test-build.noindex/functionalities/unwind/noreturn/TestNoreturnUnwind.test_dwarf/a.out -o 'settings set symbols.enable-external-lookup false' -o r -o bt -o quit * frame #0: 0x00007ffff7aa6e75 libc.so.6`.annobin_raise.c + 325 frame #1: 0x00007ffff7a91895 libc.so.6`.annobin_loadmsgcat.c_end.unlikely + 295 frame #2: 0x0000000000401140 a.out`func_c at main.c:12:2 frame #3: 0x000000000040113a a.out`func_b at main.c:18:2 frame #4: 0x0000000000401134 a.out`func_a at main.c:26:2 frame #5: 0x000000000040112e a.out`main(argc=<unavailable>, argv=<unavailable>) at main.c:32:2 frame #6: 0x00007ffff7a92f33 libc.so.6`.annobin_libc_start.c + 243 frame #7: 0x000000000040106e a.out`.annobin_init.c.hot + 46 The extra ELF symbols are there due to Annobin (I did not investigate why this problem happened specifically since F-30 and not since F-28). It is due to: Symbol table '.dynsym' contains 2361 entries: Valu e Size Type Bind Vis Name 0000000000022769 5 FUNC LOCAL DEFAULT _nl_load_domain.cold 000000000002276e 0 NOTYPE LOCAL HIDDEN .annobin_abort.c.unlikely ... 000000000002276e 0 NOTYPE LOCAL HIDDEN .annobin_loadmsgcat.c_end.unlikely ... 000000000002276e 0 NOTYPE LOCAL HIDDEN .annobin_textdomain.c_end.unlikely 000000000002276e 548 FUNC GLOBAL DEFAULT abort 000000000002276e 548 FUNC GLOBAL DEFAULT abort@@GLIBC_2.2.5 000000000002276e 548 FUNC LOCAL DEFAULT __GI_abort 0000000000022992 0 NOTYPE LOCAL HIDDEN .annobin_abort.c_end.unlikely GDB has some more complicated preferences between overlapping and/or sharing address symbols, I have made here so far the most simple fix for this case. Differential revision: https://reviews.llvm.org/D63540
diff --git a/lldb/include/lldb/Symbol/Symtab.h b/lldb/include/lldb/Symbol/Symtab.h
@@ -143,7 +143,29 @@ class Symtab {
   typedef std::vector<Symbol> collection;
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;
-  typedef RangeDataVector<lldb::addr_t, lldb::addr_t, uint32_t>
+  class FileRangeToIndexMapCompare {
+  public:
+    FileRangeToIndexMapCompare(const Symtab &symtab) : m_symtab(symtab) {}
+    bool operator()(const uint32_t a_data, const uint32_t b_data) const {
+      return rank(a_data) > rank(b_data);
+    }
+
+  private:
+    // How much preferred is this symbol?
+    int rank(const uint32_t data) const {
+      const Symbol &symbol = *m_symtab.SymbolAtIndex(data);
+      if (symbol.IsExternal())
+        return 3;
+      if (symbol.IsWeak())
+        return 2;
+      if (symbol.IsDebug())
+        return 0;
+      return 1;
+    }
+    const Symtab &m_symtab;
+  };
+  typedef RangeDataVector<lldb::addr_t, lldb::addr_t, uint32_t, 0,
+                          FileRangeToIndexMapCompare>
       FileRangeToIndexMap;
   void InitNameIndexes();
   void InitAddressIndexes();
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2258,6 +2258,8 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,
         symbol_size_valid,              // Symbol size is valid
         has_suffix,                     // Contains linker annotations?
         flags);                         // Symbol flags.
+    if (symbol.getBinding() == STB_WEAK)
+      dc_symbol.SetIsWeak(true);
     symtab->AddSymbol(dc_symbol);
   }
   return i;
diff --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
@@ -28,7 +28,7 @@ using namespace lldb;
 using namespace lldb_private;
 
 Symtab::Symtab(ObjectFile *objfile)
-    : m_objfile(objfile), m_symbols(), m_file_addr_to_index(),
+    : m_objfile(objfile), m_symbols(), m_file_addr_to_index(*this),
       m_name_to_index(), m_mutex(), m_file_addr_to_index_computed(false),
       m_name_indexes_computed(false) {}
 
diff --git a/lldb/test/Shell/SymbolFile/Inputs/symbol-binding.s b/lldb/test/Shell/SymbolFile/Inputs/symbol-binding.s
@@ -0,0 +1,22 @@
+        .text
+        .byte   0
+sizeless:
+sizeful:
+        .byte   0
+        .byte   0
+sizeend:
+        .size   sizeful, sizeend - sizeful
+        .byte   0
+case1_local:
+case1_global:
+        .globl  case1_global
+        .byte   0
+case2_local:
+case2_weak:
+        .weak   case2_weak
+        .byte   0
+case3_weak:
+        .weak   case3_weak
+case3_global:
+        .globl  case3_global
+        .byte   0
diff --git a/lldb/test/Shell/SymbolFile/symbol-binding.test b/lldb/test/Shell/SymbolFile/symbol-binding.test
@@ -0,0 +1,22 @@
+# Some targets do not have the .size directive.
+# RUN: %clang -target x86_64-unknown-unknown-elf %S/Inputs/symbol-binding.s -c -o %t.o
+# RUN: %lldb %t.o -s %s -o quit | FileCheck %s
+
+image lookup --address 4
+# CHECK: Summary: symbol-binding.test.tmp.o`case1_global
+image lookup --address 5
+# CHECK: Summary: symbol-binding.test.tmp.o`case2_weak
+image lookup --address 6
+# CHECK: Summary: symbol-binding.test.tmp.o`case3_global
+image dump symtab
+# CHECK:     Index   UserID DSX Type            File Address/Value Load Address       Size               Flags      Name
+# CHECK-NEXT:------- ------ --- --------------- ------------------ ------------------ ------------------ ---------- ----------------------------------
+# CHECK-NEXT:[    0]      1     Code            0x0000000000000004                    0x0000000000000001 0x00000000 case1_local
+# CHECK-NEXT:[    1]      2     Code            0x0000000000000005                    0x0000000000000001 0x00000000 case2_local
+# CHECK-NEXT:[    2]      3     Code            0x0000000000000003                    0x0000000000000001 0x00000000 sizeend
+# CHECK-NEXT:[    3]      4     Code            0x0000000000000001                    0x0000000000000002 0x00000000 sizeful
+# CHECK-NEXT:[    4]      5     Code            0x0000000000000001                    0x0000000000000002 0x00000000 sizeless
+# CHECK-NEXT:[    5]      6   X Code            0x0000000000000004                    0x0000000000000001 0x00000010 case1_global
+# CHECK-NEXT:[    6]      7     Code            0x0000000000000005                    0x0000000000000001 0x00000020 case2_weak
+# CHECK-NEXT:[    7]      8   X Code            0x0000000000000006                    0x0000000000000001 0x00000010 case3_global
+# CHECK-NEXT:[    8]      9     Code            0x0000000000000006                    0x0000000000000001 0x00000020 case3_weak

Original file line number	Diff line number	Diff line change
`@@ -2258,6 +2258,8 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,`
`2258`	`2258`	`symbol_size_valid, // Symbol size is valid`
`2259`	`2259`	`has_suffix, // Contains linker annotations?`
`2260`	`2260`	`flags); // Symbol flags.`
	`2261`	`+ if (symbol.getBinding() == STB_WEAK)`
	`2262`	`+ dc_symbol.SetIsWeak(true);`
`2261`	`2263`	`symtab->AddSymbol(dc_symbol);`
`2262`	`2264`	`}`
`2263`	`2265`	`return i;`