[patch] Remove the linker_private and linker_private_weak linkages

[Nick Kledzik]

Rafael,

With your great changes in r201700 to automatically use ‘l’ labels in sections that need it, ripping out the linker_private* linkage types is the right thing to do.  

My only question is if someone tries to use older bitcode that uses the now obsolete linker_private linkage types, that the right thing happens.  It looks like the new bitcode reader will map it to GlobalValue::PrivateLinkage, and the changes in  r201700 will write that as ‘L’ or ‘l’ depending on the section.  Is there some scenario were older bitcode files will not create valid mach-o with this change?

Assuming there is no compatibility issue with older bitcode files, this LGTM.

-Nick

On Mar 13, 2014, at 5:09 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
> ping
> 
> On 7 March 2014 15:18, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>> Hi,
>> 
>> These linkages were introduced some time ago, but it was never very
>> clear what exactly their semantics were or what they should be used
>> for. Some investigation found these uses:
>> 
>> * utf-16 strings in clang.
>> * non-unnamed_addr strings produced by the sanitizers.
>> 
>> It turns out they were just working around a more fundamental problem.
>> For some sections a MachO linker needs a symbol in order to split the
>> section into atoms, and llvm had no idea that was the case. I fixed
>> that in r201700 and it is now safe to use the private linkage. When
>> the object ends up in a section that requires symbols, llvm will use a
>> 'l' prefix instead of a 'L' prefix and things just work.
>> 
>> With that, these linkages were already dead, but there was a potential
>> future user in the objc metadata information. I am still looking at
>> CGObjcMac.cpp, but at this point I am convinced that linker_private
>> and linker_private_weak are not what they need.
>> 
>> The objc uses are currently split in
>> 
>> * Regular symbols (no '\01' prefix). LLVM already directly provides
>> whatever semantics they need.
>> * Uses of a private name (start with "\01L" or "\01l") and private
>> linkage. We can drop the "\01L" and "\01l" prefixes as soon as llvm
>> agrees with clang on L being ok or not for a given section. I have two
>> patches in code review for this.
>> * Uses of private name and weak linkage.
>> 
>> The last case is the one that one could think would fit one of these
>> linkages. That is not the case. The semantics are
>> 
>> * the linker will merge these symbol by *name*.
>> * the linker will hide them in the final DSO.
>> 
>> Given that the merging is done by name, any of the private (or
>> internal) linkages would be a bad match. They allow llvm to rename the
>> symbols, and that is really not what we want. From the llvm point of
>> view, these objects should really be (linkonce|weak)(_odr)?.
>> 
>> For now, just keeping the "\01l" prefix is probably the best for these
>> symbols. If we one day want to have a more direct support in llvm,
>> IMHO what we should add is not a linkage, it is just a hidden_symbol
>> attribute. It would be applicable to multiple linkages. For example,
>> on weak it would produce the current behavior we have for objc
>> metadata. On internal, it would be equivalent to private (and we
>> should then remove private).
>> 
>> Cheers,
>> Rafael