[PATCH] D23514: [ELF] - Do not change binding for hidden symbols when creating relocatable output.

[George Rimar via llvm-commits]

Actually no, I see no real benefits from it.

But noticable positive moment is that --no-gnu-unique? option do what described for -r.

It might be confusing that with and without -r it works differently, there is no

warning about and we probably do not want to document that difference,

so I have little prefference to go with my version here, though your one has

simpler logic and also should work for users as scenario to use -r and --no-gnu-unique? together

does not look as high user path for me.?


Best regards,
George.
________________________________
??: Rui Ueyama <ruiu at google.com>
??????????: 16 ??????? 2016 ?. 23:36
????: reviews+D23514+public+a2377f544def6982 at reviews.llvm.org
?????: George Rimar; Rafael Ávila de Espíndola; evgeny777; llvm-commits
????: Re: [PATCH] D23514: [ELF] - Do not change binding for hidden symbols when creating relocatable output.

On Mon, Aug 15, 2016 at 11:02 PM, George Rimar <grimar at accesssoftek.com<mailto:grimar at accesssoftek.com>> wrote:
grimar added inline comments.

================
Comment at: ELF/OutputSections.cpp:1319
@@ -1318,2 +1318,3 @@
   Symbol *S = Body->symbol();
   uint8_t Visibility = S->Visibility;
+  if (!Config->Relocatable)
----------------
ruiu wrote:
> I'm not sure if this would fix the problem entirely. My gut is that we want to keep the original symbol type if -r is specified. If that's the case, we want to add
>
>   if (Config->Relocatable)
>     return S->Binding;
>
> here. But I don't know if that's correct.
gold allows GNU_UNIQUE->STB_GLOBAL transformation for relocatable output,
so I tried to be consistent with it here.
That is the only difference with your suggestion I think. And looks gnu ld does not support
--no-gnu-unique at all, so probably we can go with any solution here.

Do you think what GNU gold does makes sense?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160817/7e970b7d/attachment.html>