[PATCH] D30485: Mark .symver versioned symbol as global

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: lib/MC/MCParser/ELFAsmParser.cpp:748
+  // for the linker.
+  getStreamer().EmitSymbolAttribute(Alias, MCSA_Global);
   return false;
----------------
tejohnson wrote:
> pcc wrote:
> > I don't think this is right. See `ELFObjectWriter::executePostLayoutBinding`: `.symver` symbols inherit their binding from the symbol they alias.
> > 
> > I think we may need something equivalent to the `executePostLayoutBinding` logic in `ModuleSymbolTable::CollectAsmSymbols`.
> Thanks for pointing me to that, I was having trouble finding good documentation on the .symver directive, but I see that if I make the aliased symbol local that gas makes the versioned symbol also local.
> 
> However, I'm not sure I agree that the handling in executePostLayoutBinding, which deals with .symver specifically, should be added to CollectAsmSymbols, which is doing more general handling of the symbols returned from the streamer. It seems like we should do this in the .symver handling here within the ELFAsmParser, and instead set the appropriate symbol attribute based on the symbol binding of the aliased Sym (instead of unconditionally setting to global as I have done here). 
> 
The problem is that we cannot resolve the binding until we have seen the entire input because a later directive may change the binding. For example:

```
foo:
.symver foo, bar@@baz

.weak foo
```
In this case `bar@@baz` has weak binding but that is not known until the end of the input.

It may be that we can handle this within the parser but I don't think it would be right to do it entirely at the point where we see the directive.


https://reviews.llvm.org/D30485