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

[Denis Protivensky via llvm-commits]

denis-protivensky added a comment.

In http://reviews.llvm.org/D13345#257809, @rafael wrote:

> Change the last test to create a shared library and check the the 'unknown' symbol shows up only on the regular symbol table, not the dynamic one.


Good.


================
Comment at: ELF/SymbolTable.cpp:83
@@ +82,3 @@
+  auto Sym =
+      new (Alloc) Undefined<ELFT>(Name, Undefined<ELFT>::SyntheticOptional);
+  resolve<ELFT>(Sym);
----------------
ruiu wrote:
> denis-protivensky wrote:
> > ruiu wrote:
> > > Why optional? All symbols specified with -u must be resolved, no?
> > As I understand, the doc says it shouldn't make linking process degrade, I mean, it shouldn't cause link errors if some of these undefined symbols are not resolved. The only purpose of this is to trigger adding of object files from archives if they contain symbols specified.
> > These symbols technically don't have any logical impact on binary, so with these left undefined there won't be any changes in runtime behavior since these undefines are not used by any code.
> Yeah, that's right.
> 
> Is there any reason we can't merge canKeepUndefined with isWeak?
We can't use weak undefined symbol to trigger resolution and addition of object files from archives.
This is explicitly stated in the ELF reference:
"The link editor does not extract archive members to resolve undefined weak symbols."

================
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>();
 
----------------
denis-protivensky wrote:
> ruiu wrote:
> > 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.
> Good.
Looks like I won't be able to rework this fully. I need a flag to indicate that the symbol is not "regular" undefined, but a special one added with -u. This flag is the `SyntheticOptional` symbol itself, and I check its address with what current `Undefined` object holds in `canKeepUndefined`. Otherwise I'll need to hold `std::unique_ptr` with synthetic symbol generated dynamically, for example, and check that it's not empty, but I won't be able to distinguish between entry point and --undefined added symbol (they now have two separate static symbols `SyntheticRequired` and `SyntheticOptional`).
I'll move static initializer to a better place though.


http://reviews.llvm.org/D13345