[PATCH] D16405: [ELF] - Symbols from object files that override symbols in DSO are added to .dynsym table.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/Symbols.cpp:66
@@ +65,3 @@
+    if (isShared() != Other->isShared())
+      checkDynamicSymbolOverride<ELFT>(this, Other);
+  }
----------------
ruiu wrote:
> Let's do this here for consistency.
> 
>   // We want to export all symbols that exist both in the executable and in DSOs
>   // so that the symbols in the executable can interrupt symbols in the DSO at runtime.
>   if (isShared() != Other->isShared())
>     if (!isa<DefinedRegular<ELFT>>(isShared() ? this : Other))
>       IsUsedInDynamicReloc = Other->IsUsedInDynamicReloc = true;
> 
> By the way, is this correct to have this inside if (IsUsedInRegularObj || Other->IsUsedInRegularObj) {}?
I think it does not make sence here. SymbolBody::compare() does not accepts lazy symbols and I also call isa<DefinedRegular<ELFT>() check.
So I think this check only works for me like check that at least one symbol is not shared. Since I have direct check for that below, there is no much sence to keep that inside.
I moved logic outside for clarity.


http://reviews.llvm.org/D16405