Re: PATCH: PR gold/13507: Gold assumes GOT entry size is the same as ELF class size

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Dec 15, 2011 at 9:53 PM, Ian Lance Taylor <iant@google.com> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>> On Thu, Dec 15, 2011 at 6:37 PM, Ian Lance Taylor <iant@google.com> wrote:
>>> "H.J. Lu" <hongjiu.lu@intel.com> writes:
>>>
>>>> This patch adds a parameter, got_entry_size, to Output_data_got
>>>> constructor so that a target can have a different GOT entry size.
>>>
>>> Why not just use Output_data_got<got_entry_size, endianness>?
>>>
>>> Ian
>>
>> There are
>>
>> template<int size, bool big_endian>
>> class Output_data_got : public Output_section_data_build
>> {
>>  public:
>>   typedef typename elfcpp::Elf_types<size>::Elf_Addr Valtype;
>>   typedef Output_data_reloc<elfcpp::SHT_REL, true, size, big_endian> Rel_dyn;
>>   typedef Output_data_reloc<elfcpp::SHT_RELA, true, size, big_endian> Rela_dyn;
>> ....
>>   bool
>>   add_local(Sized_relobj_file<size, big_endian>* object, unsigned int sym_index,
>>             unsigned int got_type);
>>
>>   // Like add_local, but use the PLT offset of the local symbol if it
>>   // has one.
>>   bool
>>   add_local_plt(Sized_relobj_file<size, big_endian>* object,
>>                 unsigned int sym_index,
>>                 unsigned int got_type);
>>
>>   // Add an entry for a local symbol to the GOT, and add a dynamic
>>   // relocation of type R_TYPE for the GOT entry.
>>   void
>>   add_local_with_rel(Sized_relobj_file<size, big_endian>* object,
>>                      unsigned int sym_index, unsigned int got_type,
>>                      Rel_dyn* rel_dyn, unsigned int r_type);
>>
>>   void
>>   add_local_with_rela(Sized_relobj_file<size, big_endian>* object,
>>                       unsigned int sym_index, unsigned int got_type,
>>                       Rela_dyn* rela_dyn, unsigned int r_type);
>>
>> The "size" parameter is the ELF class size.  32bit ELF targets
>> have to use 32.
>
> Ah, I see.  Output_data_got does conflate the GOT entry size with the
> Sized_relobj_file<size, big_endian> argument.  Sorry to be a pain, but
> that is really an error in how Output_data_got is written.  The size
> should be the size of a GOT entry, not the ELF class size.  The
> arguments should be Relobj, not Sized_relobj_file.  Of course this will
> require some other work as well.

How do we deal with

typedef typename elfcpp::Elf_types<size>::Elf_Addr Valtype;
typedef Output_data_reloc<elfcpp::SHT_REL, true, size, big_endian> Rel_dyn;
typedef Output_data_reloc<elfcpp::SHT_RELA, true, size, big_endian> Rela_dyn;

<size> here should be the ELF class size.

> In general I think the size of entries in the GOT ought to be a template
> parameter for Output_data_got.
>
> As far as I can tell your patch is incorrect, because you haven't
> changed Output_data_got<size, big_endian>::Got_entry::write.  When that
> function calls elfcpp::Swap<size, big_endian>::writeval, it has to use
> the size of a GOT entry.

How should it be fixed? Should we add another template parameter just
for GOT entry size?


-- 
H.J.