[PATCH] D59275: [ELF] Do not emit weak-undef symbols in .dynsym under -pie --no-dynamic-linker.

Wed Mar 13 14:29:08 PDT 2019

sivachandra marked an inline comment as done.
sivachandra added inline comments.

================
Comment at: lld/ELF/Symbols.cpp:270-271
     return false;
+  if (isUndefWeak() && Config->Pie && Config->DynamicLinker.empty())
+    return false;
   if (!isDefined())
----------------
ruiu wrote:
> sivachandra wrote:
> > ruiu wrote:
> > > This really needs comment. Also, why do you have to check if `DynamicLinker` was not set? In lld, `--dynamic-linker` is used only for setting the name of the dynamic linker. Shouldn't this be `--static`?
> > Reading pcc's comment from yesterday, it seemed to me that aligning with what other linkers do would be nice. So, ld.bfd for example, does not just really use "-pie -static" to prevent weak-undef symbols from showing up in .dynsym. It does so if --no-dynamic-linker is specified. Hence, the check for the dynamic linker name as above.
> What we really should do is to the thing that makes the most sense. Aligning behavior with other linkers makes sense in many cases, but we don't necessarily blindly copy all corner-case behaviors. To me, the logic that "if an executable is not dynamically linked, there's no point of exporting weak symbols" makes the most sense -- thus, checking the value of `--dynamic-linker` looks weird.
To avoid more round trips: how does diff 2 look? It is as per what you are expecting and that is what I started with. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59275