kwk updated this revision to Diff 221690.
kwk added a comment.

- Revert "Change test expectation to find 2 instead of 1 symbol"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67390/new/

https://reviews.llvm.org/D67390

Files:
  lldb/lit/Modules/ELF/Inputs/load-from-dynsym-alone.c
  lldb/lit/Modules/ELF/Inputs/load-symtab-and-dynsym.c
  lldb/lit/Modules/ELF/load-from-dynsym-alone.test
  lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
  lldb/lit/helper/toolchain.py
  lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===================================================================
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -12,6 +12,7 @@
 #include <stdint.h>
 
 #include <vector>
+#include <utility>
 
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
@@ -221,6 +222,9 @@
   /// The address class for each symbol in the elf file
   FileAddressToAddressClassMap m_address_class_map;
 
+  /// A unique set of (mangled symbol name, type, address, size) tuple  
+  std::vector<elf::ELFSymbol> m_unique_symbol_set;
+
   /// Returns the index of the given section header.
   size_t SectionIndex(const SectionHeaderCollIter &I);
 
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -39,6 +39,7 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/MipsABIFlags.h"
+#include "lldb/Utility/StreamString.h"
 
 #define CASE_AND_STREAM(s, def, width)                                         \
   case def:                                                                    \
@@ -2196,7 +2197,12 @@
         symbol_size_valid,              // Symbol size is valid
         has_suffix,                     // Contains linker annotations?
         flags);                         // Symbol flags.
-    symtab->AddSymbol(dc_symbol);
+    
+    if (std::find(m_unique_symbol_set.begin(), m_unique_symbol_set.end(),
+                  symbol) == m_unique_symbol_set.end()) {
+      symtab->AddSymbol(dc_symbol);
+      m_unique_symbol_set.push_back(symbol);
+    }
   }
   return i;
 }
@@ -2644,24 +2650,33 @@
 
     // Sharable objects and dynamic executables usually have 2 distinct symbol
     // tables, one named ".symtab", and the other ".dynsym". The dynsym is a
-    // smaller version of the symtab that only contains global symbols. The
-    // information found in the dynsym is therefore also found in the symtab,
-    // while the reverse is not necessarily true.
+    // smaller version of the symtab that only contains global symbols.
+    // Information in the dynsym section is *usually* also found in the symtab,
+    // but this is not required as symtab entries can be removed after linking.
+    // The minidebuginfo format makes use of this facility to create smaller
+    // symbol tables.
     Section *symtab =
         section_list->FindSectionByType(eSectionTypeELFSymbolTable, true).get();
-    if (!symtab) {
-      // The symtab section is non-allocable and can be stripped, so if it
-      // doesn't exist then use the dynsym section which should always be
-      // there.
-      symtab =
-          section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
-              .get();
-    }
     if (symtab) {
       m_symtab_up.reset(new Symtab(symtab->GetObjectFile()));
       symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, symtab);
     }
 
+    // The symtab section is non-allocable and can be stripped, while the dynsym
+    // section which should always be always be there. If both exist we load
+    // both to support the minidebuginfo case. Otherwise we just load the dynsym
+    // section.
+    Section *dynsym =
+        section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
+            .get();
+    if (dynsym) {
+      if (!m_symtab_up) {
+        auto sec = symtab ? symtab : dynsym;
+        m_symtab_up.reset(new Symtab(sec->GetObjectFile()));
+      }
+      symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, dynsym);
+    }
+
     // DT_JMPREL
     //      If present, this entry's d_ptr member holds the address of
     //      relocation
Index: lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
===================================================================
--- lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
+++ lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
@@ -269,6 +269,12 @@
   void Dump(lldb_private::Stream *s, uint32_t idx,
             const lldb_private::DataExtractor *strtab_data,
             const lldb_private::SectionList *section_list);
+  
+  bool operator==(const ELFSymbol &rhs) const {
+    return st_info == rhs.st_info && st_name == rhs.st_name &&
+           st_size == rhs.st_size && st_other == rhs.st_other &&
+           st_shndx == rhs.st_shndx && st_value == rhs.st_value;
+  }
 };
 
 /// \class ELFDynamic
Index: lldb/lit/helper/toolchain.py
===================================================================
--- lldb/lit/helper/toolchain.py
+++ lldb/lit/helper/toolchain.py
@@ -126,6 +126,6 @@
 
     support_tools = ['yaml2obj', 'obj2yaml', 'llvm-pdbutil',
                      'llvm-mc', 'llvm-readobj', 'llvm-objdump',
-                     'llvm-objcopy', 'lli']
+                     'llvm-objcopy', 'lli', 'llvm-strip']
     additional_tool_dirs += [config.lldb_tools_dir, config.llvm_tools_dir]
     llvm_config.add_tool_substitutions(support_tools, additional_tool_dirs)
Index: lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
===================================================================
--- /dev/null
+++ lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
@@ -0,0 +1,48 @@
+# REQUIRES: system-linux
+
+# This test ensures that we will load .dynsym even if there's a .symtab section.
+# We do this by compiling a small C program with two functions and we direct the
+# linker where to put the symbols so that in the end the layout is as follows:
+#
+#   Symbol table '.dynsym' contains 4 entries:
+#   Num:    Value          Size Type    Bind   Vis      Ndx Name
+#     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
+#     1: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND __libc_start_main@GLIBC_2.2.5
+#     2: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
+#     3: 0000000000401120    13 FUNC    GLOBAL DEFAULT   10 functionInDynsym
+#   
+#   Symbol table '.symtab' contains 2 entries:
+#   Num:    Value          Size Type    Bind   Vis      Ndx Name
+#     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
+#     1: 0000000000401110    15 FUNC    GLOBAL DEFAULT   10 functionInSymtab
+
+# We want to keep the symbol "functionInDynsym" in the .dynsym section and not
+# have it put the default .symtab section.
+# RUN: echo "{functionInDynsym;};" > %T/dynmic-symbols.txt
+# RUN: %clang -Wl,--dynamic-list=%T/dynmic-symbols.txt -g -o %t.binary %p/Inputs/load-symtab-and-dynsym.c
+
+# Remove not needed symbols
+# RUN: echo "functionInSymtab" > %t.keep_symbols
+# RUN: echo "functionInDynsym" >> %t.keep_symbols
+# RUN: llvm-objcopy --strip-all --remove-section .gdb_index --remove-section .comment --keep-symbols=%t.keep_symbols %t.binary
+
+# Remove functionInDynsym symbol from .symtab (will leave symbol in .dynsym intact)
+# RUN: llvm-strip --strip-symbol=functionInDynsym %t.binary
+
+# RUN: %lldb -b -o 'b functionInSymtab' -o 'b functionInDynsym' -o 'run' -o 'continue' %t.binary | FileCheck %s
+
+# CHECK: (lldb) b functionInSymtab
+# CHECK-NEXT: Breakpoint 1: where = {{.*}}.binary`functionInSymtab, address = 0x{{.*}}
+
+# CHECK: (lldb) b functionInDynsym
+# CHECK-NEXT: Breakpoint 2: where = {{.*}}.binary`functionInDynsym, address = 0x{{.*}}
+
+# CHECK: (lldb) run
+# CHECK-NEXT: Process {{.*}} stopped
+# CHECK-NEXT: * thread #1, name = 'load-symtab-and', stop reason = breakpoint 1.1
+
+# CHECK: (lldb) continue
+# CHECK-NEXT: Process {{.*}} resuming
+# CHECK-NEXT: Process {{.*}} stopped
+# CHECK-NEXT: * thread #1, name = 'load-symtab-and', stop reason = breakpoint 2.1
+
Index: lldb/lit/Modules/ELF/load-from-dynsym-alone.test
===================================================================
--- /dev/null
+++ lldb/lit/Modules/ELF/load-from-dynsym-alone.test
@@ -0,0 +1,33 @@
+# REQUIRES: system-linux
+
+# This test ensures that we will load .dynsym even if there's no .symtab section.
+# We do this by compiling a small C program with a function and we direct the
+# linker where to put the symbols so that in the end the layout is as follows:
+#
+#   Symbol table '.dynsym' contains 4 entries:
+#      Num:    Value          Size Type    Bind   Vis      Ndx Name
+#        0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
+#        1: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND __libc_start_main@GLIBC_2.2.5
+#        2: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
+#        3: 0000000000401110    13 FUNC    GLOBAL DEFAULT   10 functionInDynsym
+
+# We want to keep the symbol "functionInDynsym" in the .dynsym section and not
+# have it put the default .symtab section.
+# RUN: echo "{functionInDynsym;};" > %T/dynmic-symbols.txt
+# RUN: %clang -Wl,--dynamic-list=%T/dynmic-symbols.txt -g -o %t.binary %p/Inputs/load-from-dynsym-alone.c
+
+# Remove not needed symbols
+# RUN: echo "functionInDynsym" > %t.keep_symbols
+# RUN: llvm-objcopy --strip-all --remove-section .gdb_index --remove-section .comment --keep-symbols=%t.keep_symbols %t.binary
+
+# Remove functionInDynsym symbol from .symtab (will leave symbol in .dynsym intact)
+# RUN: llvm-strip --strip-symbol=functionInDynsym %t.binary
+
+# RUN: %lldb -b -o 'b functionInDynsym' -o 'run' -o 'continue' %t.binary | FileCheck %s
+
+# CHECK: (lldb) b functionInDynsym
+# CHECK-NEXT: Breakpoint 1: where = {{.*}}.binary`functionInDynsym, address = 0x{{.*}}
+
+# CHECK: (lldb) run
+# CHECK-NEXT: Process {{.*}} stopped
+# CHECK-NEXT: * thread #1, name = 'load-from-dynsy', stop reason = breakpoint 1.1
Index: lldb/lit/Modules/ELF/Inputs/load-symtab-and-dynsym.c
===================================================================
--- /dev/null
+++ lldb/lit/Modules/ELF/Inputs/load-symtab-and-dynsym.c
@@ -0,0 +1,12 @@
+// This function will be embedded within the .symtab section of the
+// .gnu_debugdata section.
+int functionInSymtab(int num) { return num * 4; }
+
+// This function will be embedded within the .dynsym section of the main binary.
+int functionInDynsym(int num) { return num * 3; }
+
+int main(int argc, char *argv[]) {
+  int x = functionInSymtab(argc);
+  int y = functionInDynsym(x);
+  return y;
+}
Index: lldb/lit/Modules/ELF/Inputs/load-from-dynsym-alone.c
===================================================================
--- /dev/null
+++ lldb/lit/Modules/ELF/Inputs/load-from-dynsym-alone.c
@@ -0,0 +1,7 @@
+// This function will be embedded within the .dynsym section of the main binary.
+int functionInDynsym(int num) { return num * 3; }
+
+int main(int argc, char *argv[]) {
+  int y = functionInDynsym(argc);
+  return y;
+}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to