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

[H.J. Lu]

On Fri, Dec 16, 2011 at 8:43 AM, Ian Lance Taylor <iant@google.com> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>> 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.
>
> As far as I can see in the definition of Valtype the size should be the
> size of a GOT entry, not the ELF class size.

For 32bit ELF class, address is 32bit. When we use

elfcpp::Swap<64, big_endian>::writeval(pov, val);

to write 64bit GOT entry for 32bit ELF class, we can use ELF class size for
Valtype.

> The simple way to handle Rel_dyn and Rela_dyn would be to change
> Output_data_reloc<>::add_global and add_local to virtual functions, add
> them to Output_data_reloc_generic, and use Output_data_reloc_generic in
> Output_data_got.  Perhaps that would lead to other issues, I'm not sure.
> If that works it would let us eliminate add_global_with_rela and
> friends.
>
> Of course since add_global and add_local take addresses it would be
> necessary for the generic versions to take uint64_t and for specific
> versions to take the appropriate size address, and use convert_types to
> make sure that there is no overflow.  Actually it might be simpler to
> leave add_global and add_local as they are and add virtual functions
> with a different name to Output_data_reloc_generic.
>

Unless we can remove all dependencies on the ELF class from
Output_data_got, I don't see how it will work.

>
>>> 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?
>
> That would be another approach, yes.

Considering there are codes in Output_data_got like

            Sized_symbol<size>* sgsym;
            // This cast is a bit ugly.  We don't want to put a
            // virtual method in Symbol, because we want Symbol to be
            // as small as possible.
            sgsym = static_cast<Sized_symbol<size>*>(gsym);
            val = sgsym->value();

add another template parameter for GOT entry size is less intrusive.
FWIW, I enclosed a new patch without adding a template parameter for
GOT entry size.  If it isn't acceptable, I will work on a patch to add
a new template parameter, which will be added to most of template
classes.

Thanks.

-- 
H.J
---
.2011-12-16  H.J. Lu  <hongjiu.lu@intel.com>

	PR gold/13507
	* arm.cc (Arm_output_data_got): Pass 32 to Output_data_got
	constructor as got_entry_size.
	* i386.cc (Target_i386::got_section): Likewise.
	(Target_i386::got_section): Likewise.

	* output.cc (write): Add a parameter, got_entry_size, and
	use it to write GOT entry.
	(add_global_pair_with_rel) Replace size with this->got_entry_size_.
	(add_global_pair_with_rela): Likewise.
	(add_local_pair_with_rel): Likewise.
	(add_local_pair_with_rela): Likewise.
	(add_got_entry): Likewise.
	(add_got_entry_pair): Likewise.
	(do_write): Likewise.  Pass this->got_entry_size_ to write.

	* output.h (Output_data_got): Add a parameter, got_entry_size,
	to constructor.  Replace size with with got_entry_size.
	(reserve_slot): Replace size with this->got_entry_size_.
	(got_offset): Likewise.
	(write): Add parameter, got_entry_size.
	(got_entry_size_): New.

	* powerpc.cc (got_section): Pass size to Output_data_got
	constructor as got_entry_size.
	* sparc.cc (got_section): Likewise.

	* x86_64.cc (got_section): Pass 64 to Output_data_got
	constructor as got_entry_size.
	(init_got_plt_for_update): Likewise.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: binutils-pr13507-3.patch
Type: text/x-diff
Size: 12721 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20111216/2e5e8a20/attachment.bin>