[lld] r294816 - Create only one section symbol per section.

[Simon Atanasyan via llvm-commits]

On Wed, Feb 15, 2017 at 12:13 AM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> On 14 February 2017 at 11:02, Simon Atanasyan <simon at atanasyan.com> wrote:
>> On Sat, Feb 11, 2017 at 4:40 AM, Rafael Espindola via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>> We could remove the -r special case of relocation sections when
>>> reading. We would instead have a copyRelocs function that is used
>>> instead of scanRelocs. It would create a DynamicReloc for each
>>> relocation and a RelocationSection for each input relocation section.
>>
>> [...]
>>
>>> Modified: lld/trunk/ELF/InputSection.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.cpp?rev=294816&r1=294815&r2=294816&view=diff
>>> ==============================================================================
>>> --- lld/trunk/ELF/InputSection.cpp (original)
>>> +++ lld/trunk/ELF/InputSection.cpp Fri Feb 10 19:40:49 2017
>>> @@ -236,6 +236,22 @@ void InputSection<ELFT>::copyRelocations
>>>      if (Config->Rela)
>>>        P->r_addend = getAddend<ELFT>(Rel);
>>>
>>> +    if (Body.Type == STT_SECTION) {
>>> +      // We combine multiple section symbols into only one per
>>> +      // section. This means we have to update the addend. That is
>>> +      // trivial for Elf_Rela, but for Elf_Rel we have to write to the
>>> +      // section data. We do that by adding to the Relocation vector.
>>> +      if (Config->Rela) {
>>> +        P->r_addend += Body.getVA<ELFT>() -
>>> +                       cast<DefinedRegular<ELFT>>(Body).Section->OutSec->Addr;
>>> +      } else if (Config->Relocatable) {
>>> +        const uint8_t *BufLoc = RelocatedSection->Data.begin() + Rel.r_offset;
>>> +        uint64_t Implicit = Target->getImplicitAddend(BufLoc, Type);
>>> +        RelocatedSection->Relocations.push_back(
>>> +            {R_ABS, Type, Rel.r_offset, Implicit, &Body});
>>> +      }
>>> +    }
>>
>> Unfortunately this does not work if original relocation point to this
>> offset expects that the addend is something different than 32/64-bit
>> number. For example, some ARM/MIPS relocations read addend as 12,16,24
>> etc bits number and then shift this number. If we write 0x1 as addend
>> for R_MIPS_26 relocation in the code above, when we read this addend
>> in the `getImplicitAddend` we get 0x1 << 2 = 4.
>>
>> As to MIPS this is not a priority problem because there are some other
>> issue related to `-r` mode linking. But it is a problem.
>
> I am not sure it is restricted to 32/64 values. The type is passed to
> the target and it can use it to write the number in any way the target
> wants. If this is broken, then even non -r cases are already broken,
> no? Take a look at Relocations.cpp and search for "!RelTy::IsRela".
> These are the cases where we have to write an addend.

My mistake. You are right, the issue is MIPS specific.

The problem related to R_MIPS_GOT16 relocation. On MIPS this
relocation might be against a local symbol. In that case it goes
together with paired R_MIPS_LO16 relocation. The 32-bit addend is
splitted among these paired relocations. The R_MIPS_GOT16's addend is
high 16-bit of 32-bit addend's value. When we read it we shift read
value to the left on 16 bits and combine with R_MIPS_LO16's addend.
Result of the R_MIPS_GOT16 relocation is 16-bit index in the GOT
table. So when we write _result_ of the relocation we should write the
value as-is, but if we like to write new addend for this relocation we
need to shift the addend right on 16 bits before writing.

I will try to find a solution for this issue.

-- 
Simon Atanasyan