kwk updated this revision to Diff 221689.
kwk added a comment.
- [LLDB][ELF] Fixup for comments in D67390 <https://reviews.llvm.org/D67390>
- Change test expectation to find 2 instead of 1 symbol
- symbol uniqueness by using elf::ELFSymbol
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/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
lldb/unittests/Target/ModuleCacheTest.cpp
Index: lldb/unittests/Target/ModuleCacheTest.cpp
===================================================================
--- lldb/unittests/Target/ModuleCacheTest.cpp
+++ lldb/unittests/Target/ModuleCacheTest.cpp
@@ -129,7 +129,7 @@
ASSERT_TRUE(bool(module_sp));
SymbolContextList sc_list;
- EXPECT_EQ(1u, module_sp->FindFunctionSymbols(ConstString("boom"),
+ EXPECT_EQ(2u, module_sp->FindFunctionSymbols(ConstString("boom"),
eFunctionNameTypeFull, sc_list));
EXPECT_STREQ(GetDummyRemotePath().GetCString(),
module_sp->GetPlatformFileSpec().GetCString());
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/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
@@ -273,7 +273,7 @@
"a_function should not exist yet",
error=True,
matching=False,
- patterns=["1 match found"])
+ patterns=["2 matches found"])
# Use lldb 'process load' to load the dylib.
self.expect(
@@ -298,7 +298,7 @@
"image lookup -n a_function",
"a_function should now exist",
patterns=[
- "1 match found .*%s" %
+ "2 matches found .*%s" %
dylibName])
# Use lldb 'process unload' to unload the dylib.
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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
