Re: Mips target in gold - revision 3 - part 2

[Cary Coutant <ccoutant at google dot com>]

>> If I understand the code correctly, in this case the HI16 reloc
>> will remain on the got16_addends_ list forever. Is there a point
>> where you know it's safe to remove it? Can you clear the list
>> at the end of each section?
>
> If LO16 is removed by GCC, than HI16 reloc is paired with the next found LO16
> reloc (this means that two or more HI16 reloc will be paired with a single LO16 reloc).
> This is what GNU ld does - it always tries to find LO16 reloc for HI16 reloc, and if it
> can't find it, error is reported.
>
> When trying to find LO16 reloc, GNU ld examines next relocations in the section until
> it finds it, or reports an error.  I implemented it differently in Gold - when HI16 or GOT16
> reloc is found, it is recorded in the got16_addends_ list, and the normal scaning of
> relocations continues.  Then when the LO16 reloc is found, the got16_addends_ list is
> examined and all pending matching HI16 and GOT16 relocs are removed.  The error when
> HI16 or GOT16 reloc doesn't have the LO16 part is detected by checking whether the
> section of the pending HI16 or GOT16 relocation is different from the section of the current
> LO16 relocation. The check whether the final section that is scanned has HI16 or GOT16
> without the LO16 part is in the do_finalize_sections method - I just check whether the
> got16_addends_ list is empty - if it isn't, error is reported.

Ah, I see.

>> >   typename std::list<got16_addend<size, big_endian> > got16_addends_;
>>
>> Do you realy want to use a linked list here? Wouldn't a
>> vector be better?
>
> I used std::list because erase is called on it.

Sorry, I missed that. And now I understand that you don't expect the
list to ever grow very long.

>   // Check whether the final section that was scaned has HI16 or GOT16

Typo: "scanned".

>   // Fill the unused space with zeroes.
>   // TODO(sasa): Can we strip unused bytes during the relaxation?
>   unsigned char* end = oview + oview_size;
>   while (pov < end)
>     {
>       elfcpp::Swap<32, big_endian>::writeval(pov, 0);
>       pov += 4;
>     }

You can just use memset here:

  if (unused > 0)
    memset(pov, 0, unused);

If that's OK with you, I'll commit it with that change.

Thanks for working on this, and for putting up with the long review process!

-cary