[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Ok, let's give this one more shot. Thanks for your patience. I do have a couple of additional comments inline, but I don't think we need another round of review for those.



================
Comment at: lldb/lit/Modules/ELF/merge-symbols.yaml:22
+# CHECK-NEXT: [ 5] 7 Code 0x0000000000500000 0x0000000000000008 0x00000002 different_size1
+# CHECK-NEXT: [ 6] 8 Code 0x0000000000500000 0x0000000000000009 0x00000002 different_size1
+
----------------
Please add a `CHECK-EMPTY:` after this line to ensure there are no additional symbols here.


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ELFHeader.cpp:253-256
+ELFSymbol::ELFSymbol(const ELFSymbol &other)
+    : st_value(other.st_value), st_size(other.st_size), st_name(other.st_name),
+      st_info(other.st_info), st_other(other.st_other),
+      st_shndx(other.st_shndx) {}
----------------
Is this really needed? It looks like the default compiler-generated copy constructor would be sufficient here.


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1918-1919
+  if (!unique_elf_symbols_sp) {
+    fprintf(stderr, "\n\nCREATING NEW SYMBOL SET(num_symbols: %lu)\n\n",
+            num_symbols);
+    unique_elf_symbols_sp.reset(new UniqueElfSymbolColl(num_symbols));
----------------
delete


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2651
 Symtab *ObjectFileELF::GetSymtab() {
+  fprintf(stderr, "GetSymtab()\n");
   ModuleSP module_sp(GetModule());
----------------
delete


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390