Re: [PATCH 02/19] PRU BFD support

[Alan Modra <amodra at gmail dot com>]

On Mon, Dec 05, 2016 at 10:42:17PM +0200, Dimitar Dimitrov wrote:
> 2016-10-24  Dimitar Dimitrov <dimitar@dinux.eu>
> 
> 	* bfd/Makefile.am: Add PRU target.
> 	* bfd/Makefile.in: Ditto.
> 	* bfd/config.bfd: Ditto.
> 	* bfd/archures.c: Ditto.
> 	* bfd/configure.ac: Ditto.
> 	* bfd/elf-bfd.h: Ditto.
> 	* bfd/targets.c: Ditto.
> 	* bfd/bfd-in2.h: Add PRU relocations.
> 	* bfd/reloc.c: Ditto.
> 	* bfd/libbfd.h: Ditto.
> 	* bfd/cpu-pru.c: New file.
> 	* bfd/elf32-pru.c: New file.

Normally you don't post patches for generated files like Makefile.in,
configure, and libbfd.h.  Just write the ChangeLog for these files
like:
	* Makefile.in: Regenerate.

Also, no bfd/ in the entries.  The paths should be relative to the
ChangeLog file.

> +/* Use RELA relocations.  */
> +#ifndef USE_RELA
> +#define USE_RELA
> +#endif
> +
> +#ifdef USE_REL
> +#undef USE_REL
> +#endif

No need to define/undef any of the above.

> +  /* 8-bit unsigned immediate relocation.  */
> +  HOWTO (R_PRU_U8,		/* type */
> +	 0,			/* rightshift */
> +	 2,			/* size (0 = byte, 1 = short, 2 = long) */
> +	 8,			/* bitsize */
> +	 FALSE,			/* pc_relative */
> +	 16,			/* bitpos */
> +	 complain_overflow_unsigned,/* complain on overflow */
> +	 bfd_elf_generic_reloc,	/* special function */
> +	 "R_PRU_U8",		/* name */
> +	 FALSE,			/* partial_inplace */
> +	 0x00ff0000,		/* src_mask */
> +	 0x00ff0000,		/* dest_mask */

Please read the comment in reloc.c (or bfd-in2.h) about src_mask, and
fix here and elsewhere.

> +/* PRU ELF linker hash entry.  */
> +
> +struct elf32_pru_link_hash_entry
> +{
> +  struct elf_link_hash_entry root;
> +};
> +
> +#define elf32_pru_hash_entry(ent) \
> +  ((struct elf32_pru_link_hash_entry *) (ent))
> +
> +/* Get the PRU elf linker hash table from a link_info structure.  */
> +#define elf32_pru_hash_table(info) \
> +  ((struct elf32_pru_link_hash_table *) ((info)->hash))
> +
> +/* PRU ELF linker hash table.  */
> +struct elf32_pru_link_hash_table
> +  {
> +    /* The main hash table.  */
> +    struct elf_link_hash_table root;
> +  };

I'd rather see these all left undefined too, until you actually need
to extend elf_link_hash_entry and elf_link_hash_table.

> +/* HOWTO handlers for relocations that require special handling.  */
> +
> +static bfd_reloc_status_type
> +pru_elf32_hi16_relocate (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
> +			 void *data, asection *input_section,
> +			 bfd *output_bfd,
> +			 char **error_message)
> +{
> +  /* If this is a relocatable link (output_bfd test tells us), just
> +     call the generic function.  Any adjustment will be done at final
> +     link time.  */
> +  if (output_bfd != NULL)
> +    return bfd_elf_generic_reloc (abfd, reloc_entry, symbol, data,
> +				  input_section, output_bfd, error_message);
> +
> +  return pru_elf32_do_hi16_relocate (abfd, reloc_entry->howto,
> +				     input_section,
> +				     data, reloc_entry->address,
> +				     (symbol->value
> +				      + symbol->section->output_section->vma
> +				      + symbol->section->output_offset),
> +				     reloc_entry->addend);
> +}

This function is unnecessary.  Simply set rightshift to 16 in the
howtos instead.

> +static bfd_reloc_status_type
> +pru_elf32_lo16_relocate (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
> +			   void *data, asection *input_section,
> +			   bfd *output_bfd,
> +			   char **error_message)
> +{
> +  /* If this is a relocatable link (output_bfd test tells us), just
> +     call the generic function.  Any adjustment will be done at final
> +     link time.  */
> +  if (output_bfd != NULL)
> +    return bfd_elf_generic_reloc (abfd, reloc_entry, symbol, data,
> +				  input_section, output_bfd, error_message);
> +
> +  return pru_elf32_do_lo16_relocate (abfd, reloc_entry->howto,
> +				     input_section,
> +				     data, reloc_entry->address,
> +				     (symbol->value
> +				      + symbol->section->output_section->vma
> +				      + symbol->section->output_offset),
> +				     reloc_entry->addend);
> +}

This one is even more so.  Please remove it.

> +#define elf_backend_can_gc_sections	1
> +#define elf_backend_can_refcount	1

I don't see any support for the above two features.  Leave them
undefined.

-- 
Alan Modra
Australia Development Lab, IBM