labath accepted this revision.
labath added a comment.

lgtm. The reason I requested this to be put separately, is because it is 
implemented in a way that kicks in even without minidebuginfo. I think this is 
fine, because entries can be removed from the symtab even without the whole 
minidebuginfo business. While this format will likely be the only real user of 
these partial symtabs, in theory, there isn't anything stopping someone from 
removing symtab entries independently of that. While unlikely, I see no harm in 
supporting that, if it does not incur any extra maintenance costs.



================
Comment at: lldb/lit/Modules/ELF/load-from-dynsym-alone.test:14
+
+# We want to keep the symbol "functionInDynsym" in the .dynamic section and not
+# have it put the default .symtab section.
----------------
s/.dynamic/.dynsym/


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2650-2652
+    // One exception to the above rule is when we have minidebuginfo embedded
+    // into a compressed .gnu_debugdata section. This section contains a 
.symtab
+    // from which all symbols already contained in the .dynsym are stripped.
----------------
How about we make this less layered, and rephrase the existing comment a bit: 
"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.


================
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()));
+      }
----------------
I wouldn't bother with this. You can just unconditionally create a Symtab 
object before you start parsing any symbol tables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to