[PATCH] D27711: ELF/AArch64: Fix dynamic relocation against local symbol in shared objects

[Adhemerval Zanella via llvm-commits]

On 13/12/2016 15:13, Rafael Avila de Espindola wrote:
> Rafael Avila de Espindola <rafael.espindola at gmail.com> writes:
> 
>> Adhemerval Zanella via Phabricator <reviews at reviews.llvm.org> writes:
>>
>>> Index: ELF/Relocations.cpp
>>> ===================================================================
>>> --- ELF/Relocations.cpp
>>> +++ ELF/Relocations.cpp
>>> @@ -154,8 +154,9 @@
>>>        Config->Shared) {
>>>      if (In<ELFT>::Got->addDynTlsEntry(Body)) {
>>>        uintX_t Off = In<ELFT>::Got->getGlobalDynOffset(Body);
>>> +      bool UseSymVA = Target->useSymVAForTls(Body);
>>>        In<ELFT>::RelaDyn->addReloc(
>>> -          {Target->TlsDescRel, In<ELFT>::Got, Off, false, &Body, 0});
>>> +          {Target->TlsDescRel, In<ELFT>::Got, Off, UseSymVA, &Body, 0});
>>>      }
>>
>> It is not clear that you need the virtual method. What cases fail if you
>> just always use the symbol VA?
>>
>> I.E., isn't it just a bug that we have a false in there instead of true?
> 
> My guess is that all that you need is
> 
> --- a/lld/ELF/Relocations.cpp
> +++ b/lld/ELF/Relocations.cpp
> @@ -156,7 +156,7 @@ static unsigned handleTlsRelocation(uint32_t Type, SymbolBody &Body,
>      if (In<ELFT>::Got->addDynTlsEntry(Body)) {
>        uintX_t Off = In<ELFT>::Got->getGlobalDynOffset(Body);
>        In<ELFT>::RelaDyn->addReloc(
> -          {Target->TlsDescRel, In<ELFT>::Got, Off, false, &Body, 0});
> +          {Target->TlsDescRel, In<ELFT>::Got, Off, !IsPreemptible, &Body, 0});
>      }
>      if (Expr != R_TLSDESC_CALL)
>        C.Relocations.push_back({Expr, Type, Offset, Addend, &Body});
> 
> no?

Checking on hos other ABI implements the same relocation on gold it seems
that this should be suffice.  I will update the patch.