[PATCH] gold: Native Client target support
Ian Lance Taylor
iant@google.com
Tue May 1 21:35:00 GMT 2012
Roland McGrath <mcgrathr@google.com> writes:
> 2012-04-24 Roland McGrath <mcgrathr@google.com>
>
> * configure.ac (ENABLE_GOLD): Consider *-*-nacl* targets ELF.
> * configure: Regenerate.
>
> gold/
> 2012-04-24 Roland McGrath <mcgrathr@google.com>
>
> * nacl.cc: New file.
> * nacl.h: New file.
> * Makefile.am (CCFILES, HFILES): Add them.
> * Makefile.in: Regenerate.
> * i386.cc (Output_data_plt_i386_nacl): New class.
> (Output_data_plt_i386_nacl_exec): New class.
> (Output_data_plt_i386_nacl_dyn): New class.
> (Target_i386_nacl): New class.
> (Target_selector_i386_nacl): New class.
> (target_selector_i386): Use it instead of Target_selector_i386.
> * x86_64.cc (Output_data_plt_x86_64_nacl): New class.
> (Target_x86_64_nacl): New class.
> (Target_selector_x86_64_nacl): New class.
> (target_selector_x86_64, target_selector_x32): Use it instead of
> Target_selector_x86_64.
> * arm.cc (Output_data_plt_arm_nacl): New class.
> (Target_arm_nacl): New class.
> (Target_selector_arm_nacl): New class.
> (target_selector_arm, target_selector_armbe): Use it instead of
> Target_selector_arm.
>
> * target-select.cc (select_target): Take new Input_file*
> argument, pass it on to recognize method of selector.
> * object.cc (make_elf_sized_object): Update caller.
> * parameters.cc (parameters_force_valid_target): Likewise.
> * incremental.cc (make_sized_incremental_binary): Likewise.
> * target-select.h: Update decl.
> (Target_selector::recognize): Take new Input_file* argument,
> pass it on to do_recognize.
> (Target_selector::do_recognize): Take new Input_file* argument.
> * freebsd.h (Target_selector_freebsd::do_recognize): Likewise.
> * powerpc.cc (Target_selector_powerpc::do_recognize): Likewise.
> * sparc.cc (Target_selector_sparc::do_recognize): Likewise.
> * testsuite/testfile.cc (Target_selector::do_recognize): Likewise.
>
> * target.h (Target::Target_info): New members isolate_execinstr
> and rosegment_gap.
> (Target::isolate_execinstr, Target::rosegment_gap): New methods.
> * arm.cc (Target_arm::arm_info): Update initializer.
> * i386.cc (Target_i386::i386_info): Likewise.
> * powerpc.cc (Target_powerpc::powerpc_info): Likewise.
> * sparc.cc (Target_sparc::sparc_info): Likewise.
> * x86_64.cc (Target_x86_64::x86_64_info): Likewise.
> * testsuite/testfile.cc (Target_test::test_target_info): Likewise.
> * layout.cc (Layout::attach_allocated_section_to_segment):
> Take new const Target* argument. If target->isolate_execinstr(), act
> like --rosegment.
> (Layout::find_first_load_seg): Take new const Target* argument;
> If target->isolate_execinstr(), reject PF_X segments.
> (Layout::relaxation_loop_body): Update caller.
> (Layout::set_segment_offsets): If target->isolate_execinstr(),
> reset file offset to zero when we hit LOAD_SEG, and then a second
> loop over the segments before LOAD_SEG to reassign offsets after
> addresses have been determined. Handle target->rosegment_gap().
> (Layout::attach_section_to_segment): Take new const Target* argument;
> pass it to attach_allocated_section_to_segment.
> (Layout::make_output_section): Update caller.
> (Layout::attach_sections_to_segments): Take new const Target* argument;
> pass it to attach_section_to_segment.
> * gold.cc (queue_middle_tasks): Update caller.
> * layout.h (Layout): Update method decls with new arguments.
>
> * arm.cc (Target_arm::Target_arm): Take optional argument for the
> Target_info pointer to use.
> (Target_arm::make_data_plt): New virtual method.
> (Target_arm::make_plt_entry): Use it.
> (Output_data_plt_arm::Output_data_plt_arm): Take additional argument
> for the section alignment.
> (Output_data_plt_arm::first_plt_entry_offset): Make abstract virtual.
> (Output_data_plt_arm::get_plt_entry_size): Likewise.
> (Output_data_plt_arm::fill_plt_entry): New abstract virtual method.
> (Output_data_plt_arm::fill_first_plt_entry): Likewise.
> (Output_data_plt_arm::set_final_data_size): Use get_plt_entry_size
> method instead of sizeof(plt_entry).
> (Output_data_plt_arm::add_entry): Likewise.
> Use first_plt_entry_offset method instead of sizeof(first_plt_entry).
> (Target_arm::first_plt_entry_offset): Call method on this->plt_ rather
> than static method.
> (Target_arm::plt_entry_size): Likewise.
> (Output_data_plt_arm::first_plt_entry, Output_data_plt_arm::plt_entry):
> Move to ...
> (Output_data_plt_arm_standard): ... here, new class.
> (Output_data_plt_arm::do_write): Move guts of PLT filling to...
> (Output_data_plt_arm_standard::fill_first_plt_entry): ... here ...
> (Output_data_plt_arm_standard::fill_plt_entry): ... and here.
>
> * x86_64.cc (Output_data_plt_x86_64::Output_data_plt_x86_64):
> Take additional argument for the PLT entry size.
> (Output_data_plt_x86_64::get_tlsdesc_plt_offset):
> Use get_plt_entry_size method rather than plt_entry_size variable.
> (Output_data_plt_x86_64::reserve_slot): Likewise.
> (Output_data_plt_x86_64::do_adjust_output_section): Likewise.
> (Output_data_plt_x86_64::add_entry): Likewise.
> (Output_data_plt_x86_64::add_local_ifunc_entry): Likewise.
> (Output_data_plt_x86_64::address_for_global): Likewise.
> (Output_data_plt_x86_64::address_for_local): Likewise.
> (Output_data_plt_x86_64::set_final_data_size): Likewise.
> (Output_data_plt_x86_64::first_plt_entry_offset): Likewise.
> Make method non-static.
> (Output_data_plt_x86_64::get_plt_entry_size): Make abstract virtual.
> (Output_data_plt_x86_64::add_eh_frame): New abstract virtual method.
> (Output_data_plt_x86_64::fill_first_plt_entry): Likewise.
> (Output_data_plt_x86_64::fill_plt_entry): Likewise.
> (Output_data_plt_x86_64::fill_tlsdesc_entry): Likewise.
> (Output_data_plt_x86_64::plt_entry_size)
> (Output_data_plt_x86_64::first_plt_entry)
> (Output_data_plt_x86_64::plt_entry)
> (Output_data_plt_x86_64::tlsdesc_plt_entry)
> (Output_data_plt_x86_64::plt_eh_frame_fde_size)
> (Output_data_plt_x86_64::plt_eh_frame_fde): Move to ...
> (Output_data_plt_x86_64_standard): ... here, new class.
> (Target_x86_64::Target_x86_64): Take optional argument for the
> Target_info pointer to use.
> (Target_x86_64::make_data_plt): New virtual method.
> (Target_x86_64::init_got_plt_for_update): Use it.
> Call this->plt_->add_eh_frame method here.
> (Output_data_plt_x86_64::init): Don't do add_eh_frame_for_plt here.
> (Target_x86_64::first_plt_entry_offset): Call method on this->plt_
> rather than static method.
> (Target_x86_64::plt_entry_size): Likewise.
> (Output_data_plt_x86_64::do_write): Use get_plt_entry_size method
> rather than plt_entry_size variable. Move guts of PLT filling to...
> (Output_data_plt_x86_64_standard::fill_first_plt_entry): ... here ...
> (Output_data_plt_x86_64_standard::fill_plt_entry): ... and here ...
> (Output_data_plt_x86_64_standard::fill_tlsdesc_entry): ... and here.
>
> * i386.cc (Output_data_plt_i386::Output_data_plt_i386): Take
> additional argument for the section alignment.
> Don't do add_eh_frame_for_plt here.
> (Output_data_plt_i386::first_plt_entry_offset): Make the method
> non-static. Use get_plt_entry_size method rather than plt_entry_size
> variable.
> (Output_data_plt_i386::get_plt_entry_size): Make abstract virtual.
> (Output_data_plt_i386::add_eh_frame): New abstract virtual method.
> (Output_data_plt_i386::fill_first_plt_entry): Likewise.
> (Output_data_plt_i386::fill_plt_entry): Likewise.
> (Output_data_plt_i386::set_final_data_size): Use get_plt_entry_size
> method instead of plt_entry_size.
> (Output_data_plt_i386::plt_entry_size)
> (Output_data_plt_i386::plt_eh_frame_fde_size)
> (Output_data_plt_i386::plt_eh_frame_fde): Move to ...
> (Output_data_plt_i386_standard): ... here, new class.
> (Output_data_plt_i386::exec_first_plt_entry)
> (Output_data_plt_i386::exec_plt_entry): Move to ...
> (Output_data_plt_i386_exec): ... here, new class.
> (Output_data_plt_i386::dyn_first_plt_entry): Move to ...
> (Output_data_plt_i386::dyn_plt_entry)
> (Output_data_plt_i386_dyn): ... here, new class.
> (Target_i386::Target_i386): Take optional argument for the Target_info
> pointer to use.
> (Target_i386::make_data_plt): New virtual method.
> (Target_i386::make_plt_section): Use it.
> Call this->plt_->add_eh_frame method here.
> (Output_data_plt_i386::add_entry): Use get_plt_entry_size method
> rather than plt_entry_size variable.
> (Output_data_plt_i386::add_local_ifunc_entry): Likewise.
> (Output_data_plt_i386::address_for_local): Likewise.
> (Output_data_plt_i386::do_write): Likewise.
> Move guts of PLT filling to...
> (Output_data_plt_i386_exec::fill_first_plt_entry): ... here ...
> (Output_data_plt_i386_exec::fill_plt_entry): ... and here ...
> (Output_data_plt_i386_dyn::fill_first_plt_entry): ... and here ...
> (Output_data_plt_i386_dyn::fill_plt_entry): ... and here.
> + virtual unsigned int
> + first_plt_entry_offset() const = 0;
>
> // Return the size of a PLT entry.
> - static unsigned int
> - get_plt_entry_size()
> - { return sizeof(plt_entry); }
> + virtual unsigned int
> + get_plt_entry_size() const = 0;
gold has a general pattern that you are not following here: public
functions in a class are never virtual, except for pure abstract base
classes. The idea is that we don't want to confuse the public interface
that a class presents to its users with the interface that child classes
present to the parent class. So I would be happier if these functions
were not virtual, but instead called protected virtual functions
implemented by the child classes. There are many existing examples in
gold, usually a public function NAME calling a protected virtual
function do_NAME.
> + protected:
> + virtual void
> + fill_first_plt_entry(unsigned char* pov,
> + Arm_address got_address,
> + Arm_address plt_address)
> + {
> + // Write first PLT entry. All but the last word are constants.
> + const size_t num_first_plt_words = (sizeof(first_plt_entry)
> + / sizeof(plt_entry[0]));
> + for (size_t i = 0; i < num_first_plt_words - 1; i++)
> + elfcpp::Swap<32, big_endian>::writeval(pov + i * 4, first_plt_entry[i]);
> + // Last word in first PLT entry is &GOT[0] - .
> + elfcpp::Swap<32, big_endian>::writeval(pov + 16,
> + got_address - (plt_address + 16));
> + }
> +
> + virtual void
> + fill_plt_entry(unsigned char* pov,
> + Arm_address got_address,
> + Arm_address plt_address,
> + unsigned int got_offset,
> + unsigned int plt_offset)
> + {
> + int32_t offset = ((got_address + got_offset)
> + - (plt_address + plt_offset + 8));
> +
> + gold_assert(offset >= 0 && offset < 0x0fffffff);
> + uint32_t plt_insn0 = plt_entry[0] | ((offset >> 20) & 0xff);
> + elfcpp::Swap<32, big_endian>::writeval(pov, plt_insn0);
> + uint32_t plt_insn1 = plt_entry[1] | ((offset >> 12) & 0xff);
> + elfcpp::Swap<32, big_endian>::writeval(pov + 4, plt_insn1);
> + uint32_t plt_insn2 = plt_entry[2] | (offset & 0xfff);
> + elfcpp::Swap<32, big_endian>::writeval(pov + 8, plt_insn2);
> + }
There is no reason for these large functions to be declared in the class
itself. The function implementations should be moved out.
> + protected:
> + virtual void
> + fill_first_plt_entry(unsigned char* pov,
> + Arm_address got_address,
> + Arm_address plt_address)
> + {
> + // Write first PLT entry. All but first two words are constants.
> + const size_t num_first_plt_words = (sizeof(first_plt_entry)
> + / sizeof(first_plt_entry[0]));
> +
> + int32_t got_displacement = got_address + 8 - (plt_address + 16);
> +
> + elfcpp::Swap<32, big_endian>::writeval
> + (pov + 0, first_plt_entry[0] | arm_movw_immediate (got_displacement));
> + elfcpp::Swap<32, big_endian>::writeval
> + (pov + 4, first_plt_entry[1] | arm_movt_immediate (got_displacement));
> +
> + for (size_t i = 2; i < num_first_plt_words; ++i)
> + elfcpp::Swap<32, big_endian>::writeval(pov + i * 4, first_plt_entry[i]);
> + }
> +
> + virtual void
> + fill_plt_entry(unsigned char* pov,
> + Arm_address got_address,
> + Arm_address plt_address,
> + unsigned int got_offset,
> + unsigned int plt_offset)
> + {
> + // Calculate the displacement between the PLT slot and the
> + // common tail that's part of the special initial PLT slot.
> + int32_t tail_displacement = (plt_address + (11 * sizeof(uint32_t))
> + - (plt_address + plt_offset
> + + sizeof(plt_entry) + sizeof(uint32_t)));
Add parentheses around the whole expression so that emacs indents
correctly.
> + gold_assert((tail_displacement & 3) == 0);
> + tail_displacement >>= 2;
> +
> + gold_assert ((tail_displacement & 0xff000000) == 0
> + || (-tail_displacement & 0xff000000) == 0);
> +
> + // Calculate the displacement between the PLT slot and the entry
> + // in the GOT. The offset accounts for the value produced by
> + // adding to pc in the penultimate instruction of the PLT stub.
> + const int32_t got_displacement = (got_address + got_offset
> + - (plt_address + sizeof(plt_entry)));
> +
> + elfcpp::Swap<32, big_endian>::writeval
> + (pov + 0, plt_entry[0] | arm_movw_immediate (got_displacement));
> + elfcpp::Swap<32, big_endian>::writeval
> + (pov + 4, plt_entry[1] | arm_movt_immediate (got_displacement));
> + elfcpp::Swap<32, big_endian>::writeval
> + (pov + 8, plt_entry[2]);
> + elfcpp::Swap<32, big_endian>::writeval
> + (pov + 12, plt_entry[3] | (tail_displacement & 0x00ffffff));
> + }
Move two large virtual functions above out of the class definition.
> diff --git a/gold/freebsd.h b/gold/freebsd.h
> index 175dd05..da7b85f 100644
> --- a/gold/freebsd.h
> +++ b/gold/freebsd.h
> @@ -1,6 +1,6 @@
> // freebsd.h -- FreeBSD support for gold -*- C++ -*-
>
> -// Copyright 2009, 2011 Free Software Foundation, Inc.
> +// Copyright 2009,2011,2012 Free Software Foundation, Inc.
Keep the spaces between the copyright years.
> diff --git a/gold/gold.cc b/gold/gold.cc
> index f810bf9..e9453e6 100644
> --- a/gold/gold.cc
> +++ b/gold/gold.cc
> @@ -1,6 +1,6 @@
> // gold.cc -- main linker functions
>
> -// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
> +// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
Keep the spaces between the copyright years. Break the line after 2012.
> @@ -90,14 +95,13 @@ class Output_data_plt_i386 : public Output_section_data
> { return this->count_ + this->irelative_count_; }
>
> // Return the offset of the first non-reserved PLT entry.
> - static unsigned int
> + unsigned int
> first_plt_entry_offset()
> - { return plt_entry_size; }
> + { return get_plt_entry_size(); }
Add explicit this: "this->get_plt_entry_size();".
> // Return the size of a PLT entry.
> - static unsigned int
> - get_plt_entry_size()
> - { return plt_entry_size; }
> + virtual unsigned int
> + get_plt_entry_size() const = 0;
Same comments as above about not having public virtual functions.
> // Set the final size.
> void
> set_final_data_size()
> {
> this->set_data_size((this->count_ + this->irelative_count_ + 1)
> - * plt_entry_size);
> + * get_plt_entry_size());
Add explicit this->
> + protected:
> + virtual void
> + fill_first_plt_entry(unsigned char* pov,
> + elfcpp::Elf_types<32>::Elf_Addr got_address)
> + {
> + memcpy(pov, first_plt_entry, plt_entry_size);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 2, got_address + 4);
> + elfcpp::Swap<32, false>::writeval(pov + 8, got_address + 8);
> + }
> +
> + virtual unsigned int
> + fill_plt_entry(unsigned char* pov,
> + elfcpp::Elf_types<32>::Elf_Addr got_address,
> + unsigned int got_offset,
> + unsigned int plt_offset,
> + unsigned int plt_rel_offset)
> + {
> + memcpy(pov, plt_entry, plt_entry_size);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 2,
> + got_address + got_offset);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 7, plt_rel_offset);
> + elfcpp::Swap<32, false>::writeval(pov + 12, - (plt_offset + 12 + 4));
> + return 6;
> + }
Move these two functions out of the class definition.
> + virtual unsigned int
> + fill_plt_entry(unsigned char* pov,
> + elfcpp::Elf_types<32>::Elf_Addr,
> + unsigned int got_offset,
> + unsigned int plt_offset,
> + unsigned int plt_rel_offset)
> + {
> + memcpy(pov, plt_entry, plt_entry_size);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 2, got_offset);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 7, plt_rel_offset);
> + elfcpp::Swap<32, false>::writeval(pov + 12, - (plt_offset + 12 + 4));
> + return 6;
> + }
Move this function out of the class definition.
> @@ -693,10 +839,12 @@ const Target::Target_info Target_i386::i386_info =
> true, // is_default_stack_executable
> true, // can_icf_inline_merge_sections
> '\0', // wrap_char
> - "/usr/lib/libc.so.1", // dynamic_linker
> + "/lib/ld-nacl-x86-32.so.1", // dynamic_linker
This change looks wrong. Please revert.
> @@ -861,7 +1006,7 @@ Output_data_plt_i386::add_entry(Symbol_table* symtab, Layout* layout,
> if (gsym->type() == elfcpp::STT_GNU_IFUNC
> && gsym->can_use_relative_reloc(false))
> {
> - gsym->set_plt_offset(this->irelative_count_ * plt_entry_size);
> + gsym->set_plt_offset(this->irelative_count_ * get_plt_entry_size());
Add explicit this->.
> @@ -878,7 +1023,7 @@ Output_data_plt_i386::add_entry(Symbol_table* symtab, Layout* layout,
> {
> // When setting the PLT offset we skip the initial reserved PLT
> // entry.
> - gsym->set_plt_offset((this->count_ + 1) * plt_entry_size);
> + gsym->set_plt_offset((this->count_ + 1) * get_plt_entry_size());
Add explicit this->.
> @@ -909,7 +1054,7 @@ Output_data_plt_i386::add_local_ifunc_entry(
> Sized_relobj_file<32, false>* relobj,
> unsigned int local_sym_index)
> {
> - unsigned int plt_offset = this->irelative_count_ * plt_entry_size;
> + unsigned int plt_offset = this->irelative_count_ * get_plt_entry_size();
Add explicit this->.
> @@ -998,7 +1143,7 @@ Output_data_plt_i386::address_for_global(const Symbol* gsym)
> uint64_t offset = 0;
> if (gsym->type() == elfcpp::STT_GNU_IFUNC
> && gsym->can_use_relative_reloc(false))
> - offset = (this->count_ + 1) * plt_entry_size;
> + offset = (this->count_ + 1) * get_plt_entry_size();
Add explicit this->.
> @@ -1008,12 +1153,12 @@ Output_data_plt_i386::address_for_global(const Symbol* gsym)
> uint64_t
> Output_data_plt_i386::address_for_local(const Relobj*, unsigned int)
> {
> - return this->address() + (this->count_ + 1) * plt_entry_size;
> + return this->address() + (this->count_ + 1) * get_plt_entry_size();
Add explicit this->.
> @@ -1130,15 +1275,8 @@ Output_data_plt_i386::do_write(Output_file* of)
> elfcpp::Elf_types<32>::Elf_Addr plt_address = this->address();
> elfcpp::Elf_types<32>::Elf_Addr got_address = this->got_plt_->address();
>
> - if (parameters->options().output_is_position_independent())
> - memcpy(pov, dyn_first_plt_entry, plt_entry_size);
> - else
> - {
> - memcpy(pov, exec_first_plt_entry, plt_entry_size);
> - elfcpp::Swap_unaligned<32, false>::writeval(pov + 2, got_address + 4);
> - elfcpp::Swap<32, false>::writeval(pov + 8, got_address + 8);
> - }
> - pov += plt_entry_size;
> + this->fill_first_plt_entry(pov, got_address);
> + pov += get_plt_entry_size();
Add explicit this->.
> @@ -1155,40 +1293,29 @@ Output_data_plt_i386::do_write(Output_file* of)
>
> const int rel_size = elfcpp::Elf_sizes<32>::rel_size;
>
> - unsigned int plt_offset = plt_entry_size;
> + unsigned int plt_offset = get_plt_entry_size();
Add explicit this->.
> unsigned int plt_rel_offset = 0;
> unsigned int got_offset = 12;
> const unsigned int count = this->count_ + this->irelative_count_;
> for (unsigned int i = 0;
> i < count;
> ++i,
> - pov += plt_entry_size,
> + pov += get_plt_entry_size(),
Add explicit this->.
> got_pov += 4,
> - plt_offset += plt_entry_size,
> + plt_offset += get_plt_entry_size(),
Add explicit this->.
> + protected:
> + virtual void
> + fill_first_plt_entry(unsigned char* pov,
> + elfcpp::Elf_types<32>::Elf_Addr got_address)
> + {
> + memcpy(pov, exec_first_plt_entry, plt_entry_size);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 2, got_address + 4);
> + elfcpp::Swap<32, false>::writeval(pov + 8, got_address + 8);
> + }
> +
> + virtual unsigned int
> + fill_plt_entry(unsigned char* pov,
> + elfcpp::Elf_types<32>::Elf_Addr got_address,
> + unsigned int got_offset,
> + unsigned int plt_offset,
> + unsigned int plt_rel_offset)
> + {
> + memcpy(pov, exec_plt_entry, plt_entry_size);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 2,
> + got_address + got_offset);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 33, plt_rel_offset);
> + elfcpp::Swap<32, false>::writeval(pov + 38, - (plt_offset + 38 + 4));
> + return 32;
> + }
Move these functions out of the class definition.
> + virtual unsigned int
> + fill_plt_entry(unsigned char* pov,
> + elfcpp::Elf_types<32>::Elf_Addr,
> + unsigned int got_offset,
> + unsigned int plt_offset,
> + unsigned int plt_rel_offset)
> + {
> + memcpy(pov, dyn_plt_entry, plt_entry_size);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 2, got_offset);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 33, plt_rel_offset);
> + elfcpp::Swap<32, false>::writeval(pov + 38, - (plt_offset + 38 + 4));
> + return 32;
> + }
move this function out of the class definition.
> diff --git a/gold/layout.cc b/gold/layout.cc
> index 65d1432..774b779 100644
> --- a/gold/layout.cc
> +++ b/gold/layout.cc
> @@ -1,6 +1,6 @@
> // layout.cc -- lay out output file sections for gold
>
> -// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
> +// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
Keep the spaces between the years, break the line after 2012.
> diff --git a/gold/layout.h b/gold/layout.h
> index f81ea3b..ea84548 100644
> --- a/gold/layout.h
> +++ b/gold/layout.h
> @@ -1,6 +1,6 @@
> // layout.h -- lay out output file sections for gold -*- C++ -*-
>
> -// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
> +// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
Keep the spaces between the years, break the line after 2012.
> + inline off_t offset() const
> + { return offset_; }
Add explicit this->.
> + inline section_size_type size() const
> + { return size_; }
Add explicit this->.
> + const unsigned char* data()
> + { return data_; }
Add explicit this->.
> + View view(off_t file_offset, off_t data_size)
> + {
> + return View(file_, file_offset, data_size);
Add explicit this->.
> + return View(file_, loc.offset(), loc.size());
Add explicit this->.
> + virtual Target*
> + do_instantiate_target()
> + {
> + if (is_nacl_)
Add explicit this->.
> + virtual Target*
> + do_recognize(Input_file* file, int machine, int osabi, int abiversion)
> + {
> + this->is_nacl_ = recognize_nacl_file(file);
Add explicit this->.
It looks in some cases FILE can be NULL here. You should probably skip
calling this->recognize_nacl_file in that case.
> + virtual const char*
> + do_target_bfd_name(const Target* target)
> + {
> + return (!this->is_our_target(target) ? NULL
> + : this->is_nacl_ ? this->bfd_name_
> + : base_selector::do_target_bfd_name(target));
Format this as
return (!this->is_our_target(target)
? NULL
: (this->is_nacl_
? this->bfd_name_
: base_selector::do_target_bfd_name(target)));
> + private:
> + bool
> + recognize_nacl_file(Input_file* input_file)
> + {
> + if (this->is_big_endian())
> + {
> +#if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
> +# ifdef HAVE_TARGET_32_BIG
> + if (this->get_size() == 32)
> + return do_recognize_nacl_file<32, true>(input_file);
> +# endif
> +# ifdef HAVE_TARGET_64_BIG
> + if (this->get_size() == 64)
> + return do_recognize_nacl_file<64, true>(input_file);
> +# endif
> +#endif
> + gold_unreachable();
> + }
> + else
> + {
> +#if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_64_LITTLE)
> +# ifdef HAVE_TARGET_32_LITTLE
> + if (this->get_size() == 32)
> + return do_recognize_nacl_file<32, false>(input_file);
> +# endif
> +# ifdef HAVE_TARGET_64_LITTLE
> + if (this->get_size() == 64)
> + return do_recognize_nacl_file<64, false>(input_file);
> +# endif
> +#endif
> + gold_unreachable();
> + }
> + }
> +
> + template<int size, bool big_endian>
> + bool
> + do_recognize_nacl_file(Input_file* input_file)
> + {
> + Sniff_file file(input_file);
> + elfcpp::Elf_file<size, big_endian, Sniff_file> elf_file(&file);
> + const unsigned int shnum = elf_file.shnum();
> + for (unsigned int shndx = 1; shndx < shnum; ++shndx)
> + {
> + if (elf_file.section_type(shndx) == elfcpp::SHT_NOTE)
> + {
> + Sniff_file::Location loc = elf_file.section_contents(shndx);
> + if (loc.size() < (3 * 4
> + + align_address(sizeof "NaCl", 4)
> + + align_address(nacl_abi_name_.size() + 1, 4)))
> + continue;
> + Sniff_file::View view(file.view(loc));
> + const unsigned char* note_data = view.data();
> + if ((elfcpp::Swap<32, big_endian>::readval(note_data + 0)
> + == sizeof "NaCl")
> + && (elfcpp::Swap<32, big_endian>::readval(note_data + 4)
> + == nacl_abi_name_.size() + 1)
> + && (elfcpp::Swap<32, big_endian>::readval(note_data + 8)
> + == elfcpp::NT_VERSION))
> + {
> + const unsigned char* name = note_data + 12;
> + const unsigned char* desc = (name
> + + align_address(sizeof "NaCl", 4));
> + if (!memcmp(name, "NaCl", sizeof "NaCl")
> + && !memcmp(desc, nacl_abi_name_.c_str(),
> + nacl_abi_name_.size() + 1))
Don't write !memcmp(). Write memcmp() == 0.
> + return true;
> + }
> + }
> + }
> + return false;
> + }
Move these big functions out of the class definition.
> diff --git a/gold/object.cc b/gold/object.cc
> index 15e5d05..8c1359c 100644
> --- a/gold/object.cc
> +++ b/gold/object.cc
> @@ -1,6 +1,6 @@
> // object.cc -- support for an object file for linking in gold
>
> -// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
> +// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
Keep the spaces between the years, break the line after 2012.
> diff --git a/gold/parameters.cc b/gold/parameters.cc
> index 7fc5730..a73899e 100644
> --- a/gold/parameters.cc
> +++ b/gold/parameters.cc
> @@ -1,6 +1,6 @@
> // parameters.cc -- general parameters for a link using gold
>
> -// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
> +// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
Keep the spaces between the years, break the line after 2012.
> - Target* do_recognize(int machine, int, int)
> + virtual Target*
> + do_recognize(Input_file*, int machine, int, int)
> {
> switch (size)
> {
> @@ -2153,7 +2162,8 @@ public:
> return this->instantiate_target();
> }
>
> - Target* do_instantiate_target()
> + virtual Target*
> + do_instantiate_target()
> { return new Target_powerpc<size, big_endian>(); }
> };
Thanks for correcting the formatting here. Note that it's not necessary
to use virtual here, and I tend to omit it for functions that are
intended to be final.
> diff --git a/gold/target-select.cc b/gold/target-select.cc
> index 9370a87..90211d8 100644
> --- a/gold/target-select.cc
> +++ b/gold/target-select.cc
> @@ -1,6 +1,6 @@
> // target-select.cc -- select a target for an object file
>
> -// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
> +// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
Keep the spaces between the years, break the line after 2012.
> diff --git a/gold/target-select.h b/gold/target-select.h
> index 310c0b9..7706077 100644
> --- a/gold/target-select.h
> +++ b/gold/target-select.h
> @@ -1,6 +1,6 @@
> // target-select.h -- select a target for an object file -*- C++ -*-
>
> -// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
> +// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
Keep the spaces between the years, break the line after 2012.
> diff --git a/gold/target.h b/gold/target.h
> index ff97aba..0a50d06 100644
> --- a/gold/target.h
> +++ b/gold/target.h
> @@ -1,6 +1,6 @@
> // target.h -- target support for gold -*- C++ -*-
>
> -// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
> +// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
Keep the spaces between the years, break the line after 2012.
> @@ -118,7 +125,7 @@ class Output_data_plt_x86_64 : public Output_section_data
> // Return the offset of the reserved TLSDESC_PLT entry.
> unsigned int
> get_tlsdesc_plt_offset() const
> - { return (this->count_ + this->irelative_count_ + 1) * plt_entry_size; }
> + { return (this->count_ + this->irelative_count_ + 1) * get_plt_entry_size(); }
Add explicit this->.
> void
> reserve_slot(unsigned int plt_index)
> {
> - this->free_list_.remove((plt_index + 1) * plt_entry_size,
> - (plt_index + 2) * plt_entry_size);
> + this->free_list_.remove((plt_index + 1) * get_plt_entry_size(),
> + (plt_index + 2) * get_plt_entry_size());
Add explicit this->.
> template<int size>
> void
> Output_data_plt_x86_64<size>::do_adjust_output_section(Output_section* os)
> {
> - os->set_entsize(plt_entry_size);
> + os->set_entsize(get_plt_entry_size());
Add explicit this->.
> @@ -1040,7 +1195,7 @@ Output_data_plt_x86_64<size>::add_entry(Symbol_table* symtab, Layout* layout,
> // Note that when setting the PLT offset for a non-IRELATIVE
> // entry we skip the initial reserved PLT entry.
> plt_index = *pcount + offset;
> - plt_offset = plt_index * plt_entry_size;
> + plt_offset = plt_index * get_plt_entry_size();
Add explicit this->.
> @@ -1057,7 +1212,8 @@ Output_data_plt_x86_64<size>::add_entry(Symbol_table* symtab, Layout* layout,
> // FIXME: This is probably not correct for IRELATIVE relocs.
>
> // For incremental updates, find an available slot.
> - plt_offset = this->free_list_.allocate(plt_entry_size, plt_entry_size, 0);
> + plt_offset = this->free_list_.allocate(get_plt_entry_size(),
> + get_plt_entry_size(), 0);
Add explicit this->.
> @@ -1065,7 +1221,7 @@ Output_data_plt_x86_64<size>::add_entry(Symbol_table* symtab, Layout* layout,
> // The GOT and PLT entries have a 1-1 correspondance, so the GOT offset
> // can be calculated from the PLT index, adjusting for the three
> // reserved entries at the beginning of the GOT.
> - plt_index = plt_offset / plt_entry_size - 1;
> + plt_index = plt_offset / get_plt_entry_size() - 1;
Add explicit this->.
> @@ -1090,7 +1246,7 @@ Output_data_plt_x86_64<size>::add_local_ifunc_entry(
> Sized_relobj_file<size, false>* relobj,
> unsigned int local_sym_index)
> {
> - unsigned int plt_offset = this->irelative_count_ * plt_entry_size;
> + unsigned int plt_offset = this->irelative_count_ * get_plt_entry_size();
Add explicit this->.
> @@ -1202,7 +1358,7 @@ Output_data_plt_x86_64<size>::address_for_global(const Symbol* gsym)
> uint64_t offset = 0;
> if (gsym->type() == elfcpp::STT_GNU_IFUNC
> && gsym->can_use_relative_reloc(false))
> - offset = (this->count_ + 1) * plt_entry_size;
> + offset = (this->count_ + 1) * get_plt_entry_size();
Add explicit this->.
> @@ -1213,7 +1369,7 @@ template<int size>
> uint64_t
> Output_data_plt_x86_64<size>::address_for_local(const Relobj*, unsigned int)
> {
> - return this->address() + (this->count_ + 1) * plt_entry_size;
> + return this->address() + (this->count_ + 1) * get_plt_entry_size();
Add explicit this->.
> @@ -1224,14 +1380,14 @@ Output_data_plt_x86_64<size>::set_final_data_size()
> unsigned int count = this->count_ + this->irelative_count_;
> if (this->has_tlsdesc_entry())
> ++count;
> - this->set_data_size((count + 1) * plt_entry_size);
> + this->set_data_size((count + 1) * get_plt_entry_size());
Add explicit this->.
> + this->fill_first_plt_entry(pov, got_address, plt_address);
> + pov += get_plt_entry_size();
Add explicit this->.
> @@ -1379,47 +1528,35 @@ Output_data_plt_x86_64<size>::do_write(Output_file* of)
> memset(got_pov, 0, 16);
> got_pov += 16;
>
> - unsigned int plt_offset = plt_entry_size;
> + unsigned int plt_offset = get_plt_entry_size();
Add explicit this->.
> unsigned int got_offset = 24;
> const unsigned int count = this->count_ + this->irelative_count_;
> for (unsigned int plt_index = 0;
> plt_index < count;
> ++plt_index,
> - pov += plt_entry_size,
> + pov += get_plt_entry_size(),
Add explicit this->.
> got_pov += 8,
> - plt_offset += plt_entry_size,
> + plt_offset += get_plt_entry_size(),
Add explicit this->.
> + protected:
> + virtual void
> + fill_first_plt_entry(unsigned char* pov,
> + typename elfcpp::Elf_types<size>::Elf_Addr got_address,
> + typename elfcpp::Elf_types<size>::Elf_Addr plt_address)
> + {
> + memcpy(pov, first_plt_entry, plt_entry_size);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 2,
> + (got_address + 8
> + - (plt_address + 2 + 4)));
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 9,
> + (got_address + 16
> + - (plt_address + 9 + 4)));
> + }
> +
> + virtual unsigned int
> + fill_plt_entry(unsigned char* pov,
> + typename elfcpp::Elf_types<size>::Elf_Addr got_address,
> + typename elfcpp::Elf_types<size>::Elf_Addr plt_address,
> + unsigned int got_offset,
> + unsigned int plt_offset,
> + unsigned int plt_index)
> + {
> + memcpy(pov, plt_entry, plt_entry_size);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 3,
> + (got_address + got_offset
> + - (plt_address + plt_offset
> + + 3 + 4)));
> +
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 33, plt_index);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 38,
> + - (plt_offset + 38 + 4));
> +
> + return 32;
> + }
> +
> + virtual void
> + fill_tlsdesc_entry(unsigned char* pov,
> + typename elfcpp::Elf_types<size>::Elf_Addr got_address,
> + typename elfcpp::Elf_types<size>::Elf_Addr plt_address,
> + typename elfcpp::Elf_types<size>::Elf_Addr got_base,
> + unsigned int tlsdesc_got_offset,
> + unsigned int plt_offset)
> + {
> + memcpy(pov, tlsdesc_plt_entry, plt_entry_size);
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 2,
> + (got_address + 8
> + - (plt_address + plt_offset
> + + 2 + 4)));
> + elfcpp::Swap_unaligned<32, false>::writeval(pov + 9,
> + (got_base
> + + tlsdesc_got_offset
> + - (plt_address + plt_offset
> + + 9 + 4)));
> + }
Move these functions out of the class definition.
> +
> + private:
> + // The size of an entry in the PLT.
> + static const int plt_entry_size = 64;
> +
> + // The first entry in the PLT.
> + // From the AMD64 ABI: "Unlike Intel386 ABI, this ABI uses the same
> + // procedure linkage table for both programs and shared objects."
I'm not sure the comment from the AMD64 ABI is relevant here.
This patch is OK with those changes.
Thanks.
Ian
More information about the Binutils
mailing list