[PATCH] MCJIT honor symbol visibility settings when populating the global symbol table

[Anthony Pesch]

I've updated the patch to reflect Rafael's comments.

Also, as we discussed Lang, I've added .globl directives for the
symbols used in the MachO tests that were breaking.

Sorry in advance for the added noise in the test diffs. The tests I
patched had some mixed white spacing, so I converted them to tabs as
that seemed to be more common in those particular files.

 - Anthony

On Fri, Aug 22, 2014 at 1:54 PM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> if (ESym->getVisibility() == ELF::STV_DEFAULT &&
> +      (ESym->getBinding() == ELF::STB_GLOBAL || ESym->getBinding() ==
> ELF::STB_WEAK))
> +    Result |= SymbolRef::SF_Exported;
>
> ELF::STV_PROTECTED should also be exported.
>
>
>
> On 22 August 2014 16:15, Lang Hames <lhames at gmail.com> wrote:
>> The RuntimeDyld side of this looks good to me.
>>
>> Rafael - I'd appreciate your take on the ELF changes. If the "st_other"
>> field in Elf_Sym_Base is going to be used solely for visibility (as it seems
>> in this patch), should it be renamed to "st_visibility" ?
>
> No, st_other is the name used in the spec. The low 2 bits are the
> visibility, the others are 0. Maybe expand the comment a bit?
>
> +  unsigned char getVisibility() const { return st_other; }
>
> Add a " & 3 " to get only the low bits. If you want, assert that the
> high bits are zero.
>
> +    SF_Exported = 1U << 6,       // Symbol is visible by other modules
>
> Modules is ambiguous.  A hidden symbol is visible from another .o. DSO
> is probably a better description.
>
>
> + void setVisibility(unsigned char v) { st_other = v; }
>
> assert that v has only two bits set. Only set the last two bits or
> assert the rest is zero.
>
> Cheers,
> Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: symvis2.patch
Type: application/octet-stream
Size: 13038 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140829/5f5e7093/attachment.obj>