[PATCH] D13345: [ELF2] Add --undefined option

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 1 07:44:30 PDT 2015


ruiu added inline comments.

================
Comment at: ELF/SymbolTable.cpp:82-83
@@ +81,4 @@
+template <class ELFT> void SymbolTable::addUndefinedSym(StringRef Name) {
+  auto Sym =
+      new (Alloc) Undefined<ELFT>(Name, Undefined<ELFT>::SyntheticOptional);
+  resolve<ELFT>(Sym);
----------------
nit: If it gets two lines, I'd write like this

  resolve<ELFT>(
      new (Alloc) ...);

================
Comment at: ELF/SymbolTable.cpp:83
@@ +82,3 @@
+  auto Sym =
+      new (Alloc) Undefined<ELFT>(Name, Undefined<ELFT>::SyntheticOptional);
+  resolve<ELFT>(Sym);
----------------
Why optional? All symbols specified with -u must be resolved, no?

================
Comment at: ELF/Symbols.h:250-263
@@ -247,4 +249,16 @@
 
+namespace {
+template <class ELFT>
+static typename llvm::object::ELFFile<ELFT>::Elf_Sym initGlobalSym() {
+  typename llvm::object::ELFFile<ELFT>::Elf_Sym Sym;
+  Sym.setBinding(llvm::ELF::STB_GLOBAL);
+  return Sym;
+}
+}
+
+template <class ELFT>
+typename Undefined<ELFT>::Elf_Sym Undefined<ELFT>::SyntheticRequired;
 template <class ELFT>
-typename Undefined<ELFT>::Elf_Sym Undefined<ELFT>::Synthetic;
+typename Undefined<ELFT>::Elf_Sym
+    Undefined<ELFT>::SyntheticOptional = initGlobalSym<ELFT>();
 
----------------
This seems overkill because I don't think we are going to allocate a lot of Synthetic Undefined symbols. Can you simply make these static fields non-static? It's going to allocate a Elf_Sym per Undefined, but that wouldn't really affect anything to performance.


http://reviews.llvm.org/D13345





More information about the llvm-commits mailing list