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

Wed Sep 11 06:06:22 PDT 2019

labath added inline comments.

================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2668-2671
+      if (!m_symtab_up) {
+        auto sec = symtab ? symtab : dynsym;
+        m_symtab_up.reset(new Symtab(sec->GetObjectFile()));
+      }
----------------
kwk wrote:
> labath wrote:
> > I wouldn't bother with this. You can just unconditionally create a Symtab object before you start parsing any symbol tables.
> I don't fully agree that it is that simple because further down in the code we do check for `if (m_symtab_up == nullptr)` and that is a condition I need to respect because of relocation, don't I?
Well.. I'm pretty sure you could delete those null checks too. But, given that these null checks seem to be the prevailing pattern in this function, changing that might be better left for a separate patch...

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67390