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

["H.J. Lu" <hjl dot tools at gmail dot 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.