This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [GOLD][PATCH] Set ARM ABI information in ELF file header.


"Doug Kwan (éæå)" <dougkwan@google.com> writes:

> 2009-10-29  Doug Kwan  <dougkwan@google.com>
>
> elfcpp/ChangeLog:
>         * arm.h (EF_ARM_BE8, EF_ARM_EABIMASK, EF_ARM_EABI_UNKNOWN,
>         EF_ARM_EABI_VER1, EF_ARM_EABI_VER2, EF_ARM_EABI_VER3,
>         EF_ARM_EABI_VER4, EF_ARM_EABI_VER5): New enums for processor-specific
>         flags.
>         (arm_eabi_version): New inline function.
>         * elfcpp.h: Add a comment about DT_ENCODING.
>
> gold/ChangeLog:
>         * arm.cc (Arm_relobj::processor_specific_flags): New method
>         definition.
>         (Arm_relobj::do_read_symbols): New method declaration.
>         (Arm_relobj::processor_specific_flags_): New data member declaration.
>         (Arm_dynobj): New class definition.
>         (Target_arm::do_finalize_sections): Add input_objects parameter.
>         (Target_arm::do_adjust_elf_header): New method declaration.
>         (Target_arm::are_eabi_versions_compatible,
>         (Target_arm::merge_processor_specific_flags): New method declaration.
>         (Target_arm::do_make_elf_object): New overloaded method definitions
>         and declaration.
>         (Arm_relobj::do_read_symbols): New method definition.
>         (Arm_dynobj::do_read_symbols): Ditto.
>         (Target_arm::do_finalize_sections): Add input_objects parameters.
>         Merge processor-specific flags from all input objects.
>         (Target_arm::are_eabi_versions_compatible,
>         Target_arm::merge_processor_specific_flags,
>         Target_arm::do_adjust_elf_header, Target_arm::do_make_elf_object):
>         New method definitions.
>         * i386.cc (Target_i386::do_finalize_sections): Add unnamed
>         Input_objects pointer type parameter.
>         * layout.cc (Layout::finalize): Pass input objects to target's.
>         finalize_sections function.
>         * powerpc.cc (Target_powerpc::do_finalize_sections): Add unnamed
>         Input_objects pointer type parameter.
>         * sparc.cc (Target_sparc::do_finalize_sections): Same.
>         * target.h (Input_objects): New forward class declaration.
>         (Target::processor_specific_flags,
>         Target::are_processor_specific_flags_sect): New method definitions.
>         (Target::finalize_sections): Add input_objects parameter.
>         (Target::Target): Initialize processor_specific_flags_ and
>         are_processor_specific_flags_set_.
>         (Target::do_finalize_sections): Add unnamed Input_objects pointer type
>         parameter.
>         (Target::set_processor_specific_flags): New method definition.
>         (Target::processor_specific_flags_,
>         Target::are_processor_specific_flags_set_): New data member
>         declarations.
>         * x86_64.cc (Target_x86_64::do_finalize_sections): Add unnamed
>         Input_objects pointer type parameter.


> +// Read the symbol information.
> +
> +template<bool big_endian>
> +void
> +Arm_relobj<big_endian>::do_read_symbols(Read_symbols_data* sd)
> +{
> +  // Call parent class to read symbol information.
> +  Sized_relobj<32, big_endian>::do_read_symbols(sd);
> +
> +  // Read processor-specific flags in ELF file header.
> +  const unsigned char* pehdr = this->get_view(elfcpp::file_header_offset,
> +					      elfcpp::Elf_sizes<32>::ehdr_size,
> +					      true, true);

I think you should pass the last parameter to get_view as false here.
I think it is unlikely that anything subsequent pass will need that
view.


> +template<bool big_endian>
> +void
> +Arm_dynobj<big_endian>::do_read_symbols(Read_symbols_data* sd)
> +{
> +  // Call parent class to read symbol information.
> +  Sized_dynobj<32, big_endian>::do_read_symbols(sd);
> +
> +  // Read processor-specific flags in ELF file header.
> +  const unsigned char* pehdr = this->get_view(elfcpp::file_header_offset,
> +					      elfcpp::Elf_sizes<32>::ehdr_size,
> +					      true, true);

Same here.

> +      // Complain about various flag mismatches.
> +      const char* name_as_c_string = name.c_str();
> +      elfcpp::Elf_Word version1 = elfcpp::arm_eabi_version(flags);
> +      elfcpp::Elf_Word version2 = elfcpp::arm_eabi_version(out_flags);
> +      if (!this->are_eabi_versions_compatible(version1, version2))
> +	gold_error(_("Source object %s has EABI version %d but output has "
> +		     "EABI version %d."),
> +		   name_as_c_string,
> +		   (flags & elfcpp::EF_ARM_EABIMASK) >> 24,
> +		   (out_flags & elfcpp::EF_ARM_EABIMASK) >> 24);

Not sure name_as_c_string buys you anything here, just use
name.c_str() in the call to gold_error.

In gold, error messages always start with a lower case letter, and
never end with a period.  I would s/Source/input/.

> +    }
> +  else
> +    {
> +      // This is the first time, just copy the flags.
> +      // We only copy the EABI version for now.
> +      this->set_processor_specific_flags(flags & elfcpp::EF_ARM_EABIMASK);
> +    }
> +}

This looks like you are going to set the processor specific flags for
the output even if the input doesn't have any.  That doesn't seem
right.  If input doesn't have any flags, don't set the output flags.


> +// Adjust ELF file header.
> +template<bool big_endian>
> +void
> +Target_arm<big_endian>::do_adjust_elf_header(
> +    unsigned char* view,
> +    int len) const
> +{
> +  gold_assert(len == elfcpp::Elf_sizes<32>::ehdr_size);
> +
> +  elfcpp::Ehdr<32, big_endian> ehdr(view);
> +  unsigned char e_ident[elfcpp::EI_NIDENT];
> +  memcpy(e_ident, ehdr.get_e_ident(), elfcpp::EI_NIDENT);
> +
> +  if (elfcpp::arm_eabi_version(this->processor_specific_flags())
> +      == elfcpp::EF_ARM_EABI_UNKNOWN)
> +    e_ident[elfcpp::EI_OSABI] = elfcpp::ELFOSABI_ARM;
> +  else
> +    e_ident[elfcpp::EI_OSABI] = 0;
> +  e_ident[elfcpp::EI_ABIVERSION] = 0;
> +
> +  // FIXME: Do EF_ARM_BE8 adjustment.
> +
> +  elfcpp::Ehdr_write<32, big_endian> oehdr(view);
> +  oehdr.put_e_ident(e_ident);
> +}

Aren't you supposed to call set_e_flags here?

Oh, I see, that should be done in output.cc, but you didn't include
output.cc in this version of the patch.

This patch is OK with the above changes if output.cc just calls the
target to get the value to store in e_flags.

Thanks.

Ian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]