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

[Jan Kratochvil via Phabricator via llvm-commits]

jankratochvil added inline comments.


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h:276
+           st_size == rhs.st_size && st_other == rhs.st_other &&
+           st_shndx == rhs.st_shndx && st_value == rhs.st_value;
+  }
----------------
It could be in the same order as `ELFSymbol` fields as otherwise it is difficult to verify all the fields are matched here.


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:42
 #include "llvm/Support/MipsABIFlags.h"
+#include "lldb/Utility/StreamString.h"
 
----------------
Is it really needed?


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2205
+      m_unique_symbol_set.push_back(symbol);
+    }
   }
----------------
What if the symbol is ignored, the function will then incorrectly return a number of added symbols even when they were not added, wouldn't it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390