This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [Patch][Gold] BE8 for ARM
On 15/08/16 19:05, Bharathi Seshadri (bseshadr) wrote:
> Hi,
>
> Please find attached the BE8 patch that includes support for thumb as well.
> I tested it on a hello world and bzip2 example for both arm and thumb besides a large arm application (size ~120MB).
> No regressions with the gold test-suite for arm.
>
Thanks for adding that. For the record I don't know the Gold code well
enough to review this further, so I'll leave that to Cary.
R.
> gold/ChangeLog
> 2016-08-15 Bharathi Seshadri <bseshadr@cisco.com>
>
> * options.h (General_options): Add --be8 option.
> * arm.cc (Arm_relobj::do_relocate_sections): Add code to swap for be8.
> (Output_data_plt_arm_standard::do_fill_first_plt_entry): Likewise.
> (Output_data_plt_arm_short::do_fill_plt_entry): Likewise.
> (Output_data_plt_arm_long::do_fill_plt_entry): Likewise.
> (Target_arm::do_adjust_elf_header): Do EF_ARM_BE8 adjustment.
>
> Thanks,
> Bharathi
>
> -----Original Message-----
> From: Cary Coutant [mailto:ccoutant@gmail.com]
> Sent: Friday, August 12, 2016 9:35 AM
> To: Bharathi Seshadri (bseshadr)
> Cc: Richard Earnshaw (lists); binutils@sourceware.org
> Subject: Re: [Patch][Gold] BE8 for ARM
>
>> My application is arm only and non-thumb, so I did not include the thumb patch in the initial implementation. I'm now extending it to cover thumb. I'll post the patch when I'm done with testing.
>
> OK, thanks! Here are my comments on the formatting...
>
> + gold_assert(section_address >= view_address
> + && ((section_address + section_size)
> + <= (view_address + view_size)));
>
> The indentation isn't right here. The "<=" should line up with the second '(' in the line above.
>
> 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]);
> + if (parameters->options().user_set_be8())
> + elfcpp::Swap<32, false>::writeval(pov + i * 4, first_plt_entry[i]);
> + else
> + elfcpp::Swap<32, big_endian>::writeval(pov + i * 4,
> + first_plt_entry[i]);
>
> (Two places.) I'd prefer to see braces around the body of the for loop here, since it's a more complex statement.
>
> + if (parameters->options().user_set_be8())
> + elfcpp::Swap<32, false>::writeval(pov + 16,
> + got_address - (plt_address +
> + 16)); else
> + elfcpp::Swap<32, big_endian>::writeval(pov + 16,
> got_address - (plt_address + 16));
>
> (Two places.) The indentation is off here, too. In each case, "got_address" should line up under "pov".
>
> + if (parameters->options().user_set_be8())
>
> Instead of "user_set_be8", you should probably be using just "be8".
> The difference is that if --be8 is ever turned on by default,
> user_set_be8() will still be true only if the option is explicitly specified on the command line. (I do see a few other places in arm.cc where the same argument applies; those should probably get cleaned up
> someday.)
>
> + DEFINE_bool(be8,options::TWO_DASHES, '\0', false,
> + N_("(ARM only) be8 -byte invariant addressing."), NULL);
>
> The hyphen in front of "byte" seems wrong. (For comparison, ld's help text is simply "Output BE8 format image", but I find that fairly
> unhelpful.)
>
> -cary
>
>
> be8_patch.txt
>
>
> diff --git a/gold/arm.cc b/gold/arm.cc
> index c47b002..2159dda 100644
> --- a/gold/arm.cc
> +++ b/gold/arm.cc
> @@ -6639,6 +6639,79 @@ Arm_relobj<big_endian>::do_relocate_sections(
> section_address,
> section_size);
> }
> + // BE8 swapping
> + if (parameters->options().be8())
> + {
> + section_size_type span_start, span_end;
> + elfcpp::Shdr<32, big_endian>
> + shdr(pshdrs + i * elfcpp::Elf_sizes<32>::shdr_size);
> + Mapping_symbol_position section_start(i, 0);
> + typename Mapping_symbols_info::const_iterator p =
> + this->mapping_symbols_info_.lower_bound(section_start);
> + unsigned char* view = (*pviews)[i].view;
> + Arm_address view_address = (*pviews)[i].address;
> + section_size_type view_size = (*pviews)[i].view_size;
> + while (p != this->mapping_symbols_info_.end()
> + && p->first.first == i)
> + {
> + typename Mapping_symbols_info::const_iterator next =
> + this->mapping_symbols_info_.upper_bound(p->first);
> +
> + // Only swap arm or thumb code.
> + if ((p->second == 'a') || (p->second == 't'))
> + {
> + Output_section* os = this->output_section(i);
> + gold_assert(os != NULL);
> + Arm_address section_address =
> + this->simple_input_section_output_address(i, os);
> + span_start = convert_to_section_size_type(p->first.second);
> + if (next != this->mapping_symbols_info_.end()
> + && next->first.first == i)
> + span_end =
> + convert_to_section_size_type(next->first.second);
> + else
> + span_end =
> + convert_to_section_size_type(shdr.get_sh_size());
> + unsigned char* section_view =
> + view + (section_address - view_address);
> + uint64_t section_size = this->section_size(i);
> +
> + gold_assert(section_address >= view_address
> + && ((section_address + section_size)
> + <= (view_address + view_size)));
> +
> + // Set Output view for swapping
> + unsigned char *oview = section_view + span_start;
> + unsigned int index = 0;
> + if (p->second == 'a')
> + {
> + while (index + 3 < (span_end - span_start))
> + {
> + typedef typename elfcpp::Swap<32, big_endian>
> + ::Valtype Valtype;
> + Valtype* wv =
> + reinterpret_cast<Valtype*>(oview+index);
> + uint32_t val = elfcpp::Swap<32, false>::readval(wv);
> + elfcpp::Swap<32, true>::writeval(wv, val);
> + index += 4;
> + }
> + }
> + else if (p->second == 't')
> + {
> + while (index + 1 < (span_end - span_start))
> + {
> + typedef typename elfcpp::Swap<16, big_endian>
> + ::Valtype Valtype;
> + Valtype* wv = reinterpret_cast<Valtype*>(oview+index);
> + uint16_t val = elfcpp::Swap<16, false>::readval(wv);
> + elfcpp::Swap<16, true>::writeval(wv, val);
> + index += 2;
> + }
> + }
> + }
> + p = next;
> + }
> + }
> }
> }
>
> @@ -7785,7 +7858,18 @@ Output_data_plt_arm_standard<big_endian>::do_fill_first_plt_entry(
> const size_t num_first_plt_words = (sizeof(first_plt_entry)
> / sizeof(first_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]);
> + {
> + if (parameters->options().be8())
> + {
> + elfcpp::Swap<32, false>::writeval(pov + i * 4,
> + first_plt_entry[i]);
> + }
> + else
> + {
> + 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));
> @@ -7846,11 +7930,21 @@ Output_data_plt_arm_short<big_endian>::do_fill_plt_entry(
> gold_error(_("PLT offset too large, try linking with --long-plt"));
>
> 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);
> +
> + if (parameters->options().be8())
> + {
> + elfcpp::Swap<32, false>::writeval(pov, plt_insn0);
> + elfcpp::Swap<32, false>::writeval(pov + 4, plt_insn1);
> + elfcpp::Swap<32, false>::writeval(pov + 8, plt_insn2);
> + }
> + else
> + {
> + elfcpp::Swap<32, big_endian>::writeval(pov, plt_insn0);
> + elfcpp::Swap<32, big_endian>::writeval(pov + 4, plt_insn1);
> + elfcpp::Swap<32, big_endian>::writeval(pov + 8, plt_insn2);
> + }
> }
>
> // This class generates long (16-byte) entries, for arbitrary displacements.
> @@ -7906,13 +8000,24 @@ Output_data_plt_arm_long<big_endian>::do_fill_plt_entry(
> - (plt_address + plt_offset + 8));
>
> uint32_t plt_insn0 = plt_entry[0] | (offset >> 28);
> - elfcpp::Swap<32, big_endian>::writeval(pov, plt_insn0);
> uint32_t plt_insn1 = plt_entry[1] | ((offset >> 20) & 0xff);
> - elfcpp::Swap<32, big_endian>::writeval(pov + 4, plt_insn1);
> uint32_t plt_insn2 = plt_entry[2] | ((offset >> 12) & 0xff);
> - elfcpp::Swap<32, big_endian>::writeval(pov + 8, plt_insn2);
> uint32_t plt_insn3 = plt_entry[3] | (offset & 0xfff);
> - elfcpp::Swap<32, big_endian>::writeval(pov + 12, plt_insn3);
> +
> + if (parameters->options().be8())
> + {
> + elfcpp::Swap<32, false>::writeval(pov, plt_insn0);
> + elfcpp::Swap<32, false>::writeval(pov + 4, plt_insn1);
> + elfcpp::Swap<32, false>::writeval(pov + 8, plt_insn2);
> + elfcpp::Swap<32, false>::writeval(pov + 12, plt_insn3);
> + }
> + else
> + {
> + elfcpp::Swap<32, big_endian>::writeval(pov, plt_insn0);
> + elfcpp::Swap<32, big_endian>::writeval(pov + 4, plt_insn1);
> + elfcpp::Swap<32, big_endian>::writeval(pov + 8, plt_insn2);
> + elfcpp::Swap<32, big_endian>::writeval(pov + 12, plt_insn3);
> + }
> }
>
> // Write out the PLT. This uses the hand-coded instructions above,
> @@ -10683,7 +10788,14 @@ Target_arm<big_endian>::do_adjust_elf_header(
> e_ident[elfcpp::EI_OSABI] = 0;
> e_ident[elfcpp::EI_ABIVERSION] = 0;
>
> - // FIXME: Do EF_ARM_BE8 adjustment.
> + // Do EF_ARM_BE8 adjustment.
> + if (parameters->options().be8() && !big_endian)
> + gold_error("BE8 images only valid in big-endian mode.");
> + if (parameters->options().be8())
> + {
> + flags |= elfcpp::EF_ARM_BE8;
> + this->set_processor_specific_flags(flags);
> + }
>
> // If we're working in EABI_VER5, set the hard/soft float ABI flags
> // as appropriate.
> diff --git a/gold/options.h b/gold/options.h
> index 23c9658..8680635 100644
> --- a/gold/options.h
> +++ b/gold/options.h
> @@ -674,6 +674,9 @@ class General_options
> DEFINE_bool_alias(dn, Bdynamic, options::ONE_DASH, '\0',
> N_("alias for -Bstatic"), NULL, true);
>
> + DEFINE_bool(be8,options::TWO_DASHES, '\0', false,
> + N_("Output BE8 format image"), NULL);
> +
> DEFINE_bool(Bgroup, options::ONE_DASH, '\0', false,
> N_("Use group name lookup rules for shared library"), NULL);
>
>