GOLD: RFA: Add support for RX target
Ian Lance Taylor
ian@airs.com
Mon Dec 13 18:45:00 GMT 2010
Nick Clifton <nickc@redhat.com> writes:
> ./ChangeLog
> 2010-12-13 Nick Clifton <nickc@redhat.com>
>
> * configure.ac: Add RX to list of targets for which
> --enable-gold is valid.
> * configure: Regenerate.
>
> elfcpp/ChangeLog
> 2010-12-13 Nick Clifton <nickc@redhat.com>
>
> * rx.h: New file. Contains ELF definitions specific to the
> Renesas RX architecture.
> * elfcpp.h (enum EM): Add EM_RX.
>
> gold/ChangeLog
> 2010-12-13 Nick Clifton <nickc@redhat.com>
>
> * rx.cc: New file. Adds support for Renesas RX architecture.
> * Makefile.am (TARGETSOURCES): Add rx.cc.
> (ALL_TARGETOBJS): Add rx.$(OBJEXT)
> * Makefile.in: Regenerate.
> * configure.ac: Add RX as a default target.
> * configure: Regenerate.
> * configure.tgt: Add rx-*-* as a supported target.
> + #include <cstdio>
> + #include <cstring>
> + #include <limits>
> + #include <cstdio>
> + #include <string>
> + #include <algorithm>
> + #include <map>
> + #include <utility>
> + #include <set>
You only have to #include <cstdio> once.
> + inline bool
> + Target_rx::Relocate::relocate(
> + const Relocate_info<32, false>* relinfo,
> + Target_rx* target,
> + Output_section * /* output_section */,
> + size_t relnum,
> + const elfcpp::Rela<32, false>& rela,
> + unsigned int r_type,
> + const Sized_symbol<32>* /* gsym */,
> + const Symbol_value<32>* psymval,
> + unsigned char* view,
> + RX_address address,
> + section_size_type /* view_size */)
> + {
> + typedef RX_relocation_functions Reloc;
> + elfcpp::Elf_Xword addend = rela.get_r_addend();
> +
> + if (target->uses_bigendian_data ())
s/ ()/()/
> + void
> + Target_rx::relocate_section(
> + const Relocate_info<32, false>* relinfo,
> + unsigned int sh_type,
> + const unsigned char* prelocs,
> + size_t reloc_count,
> + Output_section* output_section,
> + bool needs_special_offset_handling,
> + unsigned char* view,
> + RX_address address,
> + section_size_type view_size,
> + const Reloc_symbol_changes* reloc_symbol_changes)
> + {
> + typedef Target_rx::Relocate Rx_relocate;
> +
> + gold_assert(sh_type == elfcpp::SHT_RELA);
> +
> + // See if we are relocating a relaxed input section. If so, the view
> + // covers the whole output section and we need to adjust accordingly.
> + if (needs_special_offset_handling)
> + {
> + const Output_relaxed_input_section* poris =
> + output_section->find_relaxed_input_section(relinfo->object,
> + relinfo->data_shndx);
> + if (poris != NULL)
> + {
> + RX_address section_address = poris->address();
> + section_size_type section_size = poris->data_size();
> +
> + gold_assert((section_address >= address)
> + && ((section_address + section_size)
> + <= (address + view_size)));
> +
> + off_t offset = section_address - address;
> + view += offset;
> + address += offset;
> + view_size = section_size;
> + }
> + }
> +
> + if (uses_bigendian_data ())
This line should be
if (this->uses_bigendian_data())
> + {
> + // This is a copy of the relocate_section() function in target-reloc.h,
> + // except with the additional calls to convert_host to fix up the
> + // extracted information.
> + // FIXME: There ought to be a better way to do this.
Yes, there ought to be a better way to do this. If your code is
correct, then it looks like the problem is that you have a little-endian
object but the reloc fields are written with big-endian data. The
comment at the top of the file suggests that the symbol table is also
written with big-endian data, in which case I think this code is
insufficient.
Anyhow, if the reloc fields are consistenly written big-endian, then the
right approach is not to call convert_host, but instead to change
> + typedef Reloc_types<elfcpp::SHT_RELA, 32, false>::Reloc Reltype;
to
> + typedef Reloc_types<elfcpp::SHT_RELA, 32, true>::Reloc Reltype;
That will cause the reloc code to fetch values big-endian rather than
little-endian.
Now, if you need to have a little-endian Object with a big-endian
Reltype, then I think the right approach is to split relocate_section in
target-reloc.h so that it calls another template which takes, as a
parameter, the reloc type. The normal case would instantiate this
template with Reloc_types<sh_type, size, big_endian>::Reloc. The RX
case would call the new template with Reloc_types<sh_type, size,
true>::Reloc.
However, I'm not sure this is the right approach, because I'm not sure
it will handle the symbol table correctly. It seems to me that you are
going to need use a big-endian Object rather than a little-endian
Object. And if you do that, you might as well templatize the whole
Target, as Target_arm does. Then instead of worrying about
relocate_section, you would handle the required conversions entirely in
the Relocate class.
> + // Process relocations for gc.
> +
> + void
> + Target_rx::gc_process_relocs(Symbol_table* /*symtab*/,
> + Layout* /*layout*/,
> + Sized_relobj<32, false>* /*object*/,
> + unsigned int /*data_shndx*/,
> + unsigned int ,
> + const unsigned char* /*prelocs*/,
> + size_t /*reloc_count*/,
> + Output_section* /*output_section*/,
> + bool /*needs_special_offset_handling*/,
> + size_t /*local_symbol_count*/,
> + const unsigned char* /*plocal_symbols*/)
> + {
> + #if 0
> + typedef Target_rx RX;
> + typedef Target_rx::Scan Scan;
> +
> + gold::gc_process_relocs<32, false, RX, elfcpp::SHT_RELA, Scan>
> + (symtab, layout, this, object, data_shndx, prelocs,
> + reloc_count, output_section, needs_special_offset_handling,
> + local_symbol_count, plocal_symbols);
> + #endif
> + }
Should probably give an error here for now.
> + static const char *
> + flag_meaning(elfcpp::Elf_Word flags, elfcpp::Elf_Word other_flags)
I would prefer that this be a static function in Target_rx rather than a
file static function. It amounts to the same thing in the end.
> + {
> + switch (flags)
> + {
> + case elfcpp::E_FLAG_RX_64BIT_DOUBLES:
> + return "uses 64-bit doubles";
> + case elfcpp::E_FLAG_RX_DSP:
> + return "uses DSP instructions";
> + case elfcpp::E_FLAG_RX_64BIT_DOUBLES | elfcpp::E_FLAG_RX_DSP:
> + switch (other_flags)
> + {
> + case 0:
> + return "uses 64-bit doubles & DSP instructions";
> + case elfcpp::E_FLAG_RX_64BIT_DOUBLES:
> + return "uses DSP instructions";
> + case elfcpp::E_FLAG_RX_DSP:
> + return "uses 64-bit doubles";
> + default:
> + gold_unreachable();
If I understand this correctly, this should not be gold_unreachable,
because the object file could contain any random data. It should print
an error about unrecognized flags, or something like that, rather than
crash the linker.
> + }
> + case 0:
> + switch (other_flags)
> + {
> + case elfcpp::E_FLAG_RX_64BIT_DOUBLES | elfcpp::E_FLAG_RX_DSP:
> + case elfcpp::E_FLAG_RX_64BIT_DOUBLES:
> + return "uses 32-bit doubles";
> + case elfcpp::E_FLAG_RX_DSP:
> + return "synthesies DSP instructions";
> + default:
> + gold_unreachable();
> + }
> + default:
> + gold_unreachable();
Same for these two calls to gold_unreachable.
> + bool
> + Target_rx::check_type_and_flags (int type, elfcpp::Elf_Word flags, const char * name)
s/ (/(/
s/char */char*/
Also the line might be too long.
> + {
> + static const char * saved_name = NULL;
s/char */char*/
However, don't use a static variable here. It should be a member of
Target_rx instead.
> + if (are_processor_specific_flags_set())
Write as
if (this->are_processor_specific_flags_set())
> + {
> + static elfcpp::Elf_Word previous_flags = -1;
Don't use a static variable here. It should be a member of Target_rx
instead.
> + elfcpp::Elf_Word set_flags = processor_specific_flags();
Use "this->".
> + if (set_flags != flags
> + && previous_flags != flags)
> + {
> + // Make sure that we do not complain about this particular
> + // discrepancy more than once in a row.
> + previous_flags = flags;
> +
> + gold_error(_("%s: %s whereas %s does not"),
> + name, flag_meaning(flags, set_flags), saved_name);
This looks kind of hard to internationalize correctly.
> + else if (flags)
> + {
> + set_processor_specific_flags(flags);
Use "this->".
> + // Use do_make_elf_object to override the same function in the base class.
> + // We need to checks the ELF header flags before allowing the object to be
> + // made.
> + Object*
> + Target_rx::do_make_elf_object(const std::string& name,
> + Input_file* input_file,
> + off_t offset,
> + const elfcpp::Ehdr<32, false>& ehdr)
> + {
> + if (! check_type_and_flags (ehdr.get_e_type(),
> + ehdr.get_e_flags(),
> + name.c_str ()))
> + return NULL;
s/! check_type_and_flags/!this->check_type_and_flags/
> +
> + Sized_relobj<32, false>* obj =
> + new Sized_relobj<32, false>(name, input_file, offset, ehdr);
> + obj->setup();
> + return obj;
> + }
> +
> + Object*
> + Target_rx::do_make_elf_object(const std::string& name,
> + Input_file* input_file,
> + off_t offset,
> + const elfcpp::Ehdr<32, true>& ehdr)
> + {
> + if (! check_type_and_flags (ehdr.get_e_type(),
> + ehdr.get_e_flags(),
> + name.c_str ()))
> + return NULL;
s/! check_type_and_flags/!this->check_type_and_flags/
> +
> + Sized_relobj<32, true>* obj =
> + new Sized_relobj<32, true>(name, input_file, offset, ehdr);
> + obj->setup();
> + return obj;
> + }
Hmmm, I see, so you do have a big-endian Sized_relobj. But
Target_rx::relocate_section takes a little-endian Relocate_info. I
guess I don't see how you are avoiding a type error when
Sized_relobj<size, big_endian>::do_relocate_sections calls
target->relocate_section.
Ian
More information about the Binutils
mailing list