[PATCH] D12874: [LLD/ELF2] Initial support for local symbols in symtab

[Rafael Ávila de Espíndola via llvm-commits]

rafael added a comment.

Looking pretty good.

Is there a big reason for not refactoring the duplicated code in this patch?


================
Comment at: ELF/InputFiles.cpp:75
@@ -74,1 +74,3 @@
 
+// XXX: Factor out common code between getLocalSymbols()
+//      and getNonLocalSymbols().
----------------
s/XXX/FIXME/

Please do fix it on the next patch.

================
Comment at: ELF/InputFiles.cpp:84
@@ +83,3 @@
+  uint32_t LastLocal = this->Symtab->sh_info - 1;
+  if (LastLocal > NumSymbols)
+    error("Invalid sh_info in symbol table");
----------------
This should be >=. Which is one of the reason I asked to avoid the code duplication.

================
Comment at: test/elf2/local-dynamic.s:4
@@ +3,3 @@
+// RUN: lld -flavor gnu2 %t -shared -o %t1.so
+// RUN: llvm-readobj -t -s %t1.so | FileCheck %s
+// REQUIRES: x86
----------------
Instead of -s you can use

llvm-leadobj -t -dyn-symbols.


http://reviews.llvm.org/D12874