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: [RFC PATCH, binutils, ARM 4/11, ping] Use getters/setters to access ARM branch type


Ping?

On Wednesday 23 December 2015 15:57:36 Thomas Preud'homme wrote:
> Hi,
> 
> [Posting patch series as RFC]
> 
> This patch is part of a patch series to add support for ARMv8-M security
> extension[1] to GNU ld. This specific patch changes all accesses to the
> branch type stored in the st_target_internal field of a Elf_Internal_Sym
> structure or in the target_internal field of a elf_link_hash_entry
> structure by getter and setters. This is required by subsequent patches to
> also store in these fields whether a symbol is a special symbol indicating
> a secure entry function.
> 
> 
> [1] Software requirements for ARMv8-M security extension are described in
> document ARM-ECM-0359818 [2] [2] Available on http://infocenter.arm.com in
> Developer guides and articles > Software development > ARMÂv8-M Security
> Extensions: Requirements on Development Tools
> 
> ChangeLog entries are as follow:
> 
> 
> *** bfd/ChangeLog ***
> 
> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * elf32-arm.c (elf32_arm_size_stubs): Use new macros
>         ARM_GET_SYM_BRANCH_TYPE and ARM_SET_SYM_BRANCH_TYPE to respectively
> get and set branch type of a symbol.
>         (bfd_elf32_arm_process_before_allocation): Likewise.
>         (elf32_arm_relocate_section): Likewise and fix identation along the
>         way.
>         (allocate_dynrelocs_for_symbol): Likewise.
>         (elf32_arm_finish_dynamic_symbol): Likewise.
>         (elf32_arm_swap_symbol_in): Likewise.
>         (elf32_arm_swap_symbol_out): Likewise.
> 
> 
> *** gas/ChangeLog ***
> 
> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * config/tc-arm.c (arm_adjust_symtab): Use ARM_SET_SYM_BRANCH_TYPE
> to set branch type of a symbol.
> 
> 
> *** gdb/ChangeLog ***
> 
> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * arm-tdep.c (arm_elf_make_msymbol_special): Use
>         ARM_GET_SYM_BRANCH_TYPE to get branch type of a symbol.
> 
> 
> *** include/elf/ChangeLog ***
> 
> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * arm.h (ARM_SYM_BRANCH_TYPE): Replace by ...
>         (ARM_GET_SYM_BRANCH_TYPE): ... this ...
>         (ARM_SET_SYM_BRANCH_TYPE): ... and this.
> 
> 
> *** ld/ChangeLog ***
> 
> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         (gld${EMULATION_NAME}_finish): Use ARM_GET_SYM_BRANCH_TYPE to get
>         branch type of a symbol.
> 
> 
> *** opcodes/ChangeLog ***
> 
> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * arm-dis.c (get_sym_code_type): Use ARM_GET_SYM_BRANCH_TYPE to get
>         branch type of a symbol.
>         (print_insn): Likewise.
> 
> 
> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
> index
> 8450733016a496ec34533e8aad4900be16dc5849..6c8060a2c559ff5bd8a8e7f414f0a471d
> fa32cc8 100644 --- a/bfd/elf32-arm.c
> +++ b/bfd/elf32-arm.c
> @@ -5383,7 +5383,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
>  				     + sym_sec->output_offset
>  				     + sym_sec->output_section->vma);
>  		      st_type = ELF_ST_TYPE (sym->st_info);
> -		      branch_type = ARM_SYM_BRANCH_TYPE (sym);
> +		      branch_type =
> +			ARM_GET_SYM_BRANCH_TYPE (sym->st_target_internal);
>  		      sym_name
>  			= bfd_elf_string_from_elf_section (input_bfd,
>  							   symtab_hdr->sh_link,
> @@ -5458,7 +5459,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
>  			  goto error_ret_free_internal;
>  			}
>  		      st_type = hash->root.type;
> -		      branch_type = hash->root.target_internal;
> +		      branch_type =
> +			ARM_GET_SYM_BRANCH_TYPE (hash->root.target_internal);
>  		      sym_name = hash->root.root.root.string;
>  		    }
> 
> @@ -6551,7 +6553,8 @@ bfd_elf32_arm_process_before_allocation (bfd *abfd,
>  	      /* This one is a call from arm code.  We need to look up
>  		 the target of the call.  If it is a thumb target, we
>  		 insert glue.  */
> -	      if (h->target_internal == ST_BRANCH_TO_THUMB)
> +	      if (ARM_GET_SYM_BRANCH_TYPE (h->target_internal)
> +		  == ST_BRANCH_TO_THUMB)
>  		record_arm_to_thumb_glue (link_info, h);
>  	      break;
> 
> @@ -11382,28 +11385,34 @@ elf32_arm_relocate_section (bfd *                 
> output_bfd, and we won't let anybody mess with it. Also, we have to do
>  	 addend adjustments in case of a R_ARM_TLS_GOTDESC relocation
>  	 both in relaxed and non-relaxed cases.  */
> -     if ((elf32_arm_tls_transition (info, r_type, h) != (unsigned)r_type)
> -	 || (IS_ARM_TLS_GNU_RELOC (r_type)
> -	     && !((h ? elf32_arm_hash_entry (h)->tls_type :
> -		   elf32_arm_local_got_tls_type (input_bfd)[r_symndx])
> -		  & GOT_TLS_GDESC)))
> -       {
> -	 r = elf32_arm_tls_relax (globals, input_bfd, input_section,
> -				  contents, rel, h == NULL);
> -	 /* This may have been marked unresolved because it came from
> -	    a shared library.  But we've just dealt with that.  */
> -	 unresolved_reloc = 0;
> -       }
> -     else
> -       r = bfd_reloc_continue;
> +      if ((elf32_arm_tls_transition (info, r_type, h) != (unsigned)r_type)
> +	  || (IS_ARM_TLS_GNU_RELOC (r_type)
> +	      && !((h ? elf32_arm_hash_entry (h)->tls_type :
> +		    elf32_arm_local_got_tls_type (input_bfd)[r_symndx])
> +		   & GOT_TLS_GDESC)))
> +	{
> +	  r = elf32_arm_tls_relax (globals, input_bfd, input_section,
> +				   contents, rel, h == NULL);
> +	  /* This may have been marked unresolved because it came from
> +	     a shared library.  But we've just dealt with that.  */
> +	  unresolved_reloc = 0;
> +	}
> +      else
> +	r = bfd_reloc_continue;
> 
> -     if (r == bfd_reloc_continue)
> -       r = elf32_arm_final_link_relocate (howto, input_bfd, output_bfd,
> -					  input_section, contents, rel,
> -					  relocation, info, sec, name, sym_type,
> -					  (h ? h->target_internal
> -					   : ARM_SYM_BRANCH_TYPE (sym)), h,
> -					  &unresolved_reloc, &error_message);
> +      if (r == bfd_reloc_continue)
> +	{
> +	  unsigned char branch_type =
> +	    h ? ARM_GET_SYM_BRANCH_TYPE (h->target_internal)
> +	      : ARM_GET_SYM_BRANCH_TYPE (sym->st_target_internal);
> +
> +	  r = elf32_arm_final_link_relocate (howto, input_bfd, output_bfd,
> +					     input_section, contents, rel,
> +					     relocation, info, sec, name,
> +					     sym_type, branch_type, h,
> +					     &unresolved_reloc,
> +					     &error_message);
> +	}
> 
>        /* Dynamic relocs are not propagated for SEC_DEBUGGING sections
>  	 because such sections are not SEC_ALLOC and thus ld.so will
> @@ -14125,7 +14134,7 @@ allocate_dynrelocs_for_symbol (struct
> elf_link_hash_entry *h, void * inf) /* Make sure the function is not marked
> as Thumb, in case
>  		 it is the target of an ABS32 relocation, which will
>  		 point to the PLT entry.  */
> -	      h->target_internal = ST_BRANCH_TO_ARM;
> +	      ARM_SET_SYM_BRANCH_TYPE (h->target_internal, ST_BRANCH_TO_ARM);
>  	    }
> 
>  	  /* VxWorks executables have a second set of relocations for
> @@ -14273,7 +14282,7 @@ allocate_dynrelocs_for_symbol (struct
> elf_link_hash_entry *h, void * inf) /* Allocate stubs for exported Thumb
> functions on v4t.  */
>    if (!htab->use_blx && h->dynindx != -1
>        && h->def_regular
> -      && h->target_internal == ST_BRANCH_TO_THUMB
> +      && ARM_GET_SYM_BRANCH_TYPE (h->target_internal) == ST_BRANCH_TO_THUMB
> && ELF_ST_VISIBILITY (h->other) == STV_DEFAULT)
>      {
>        struct elf_link_hash_entry * th;
> @@ -14293,12 +14302,12 @@ allocate_dynrelocs_for_symbol (struct
> elf_link_hash_entry *h, void * inf) myh = (struct elf_link_hash_entry *)
> bh;
>        myh->type = ELF_ST_INFO (STB_LOCAL, STT_FUNC);
>        myh->forced_local = 1;
> -      myh->target_internal = ST_BRANCH_TO_THUMB;
> +      ARM_SET_SYM_BRANCH_TYPE (myh->target_internal, ST_BRANCH_TO_THUMB);
>        eh->export_glue = myh;
>        th = record_arm_to_thumb_glue (info, h);
>        /* Point the symbol at the stub.  */
>        h->type = ELF_ST_INFO (ELF_ST_BIND (h->type), STT_FUNC);
> -      h->target_internal = ST_BRANCH_TO_ARM;
> +      ARM_SET_SYM_BRANCH_TYPE (h->target_internal, ST_BRANCH_TO_ARM);
>        h->root.u.def.section = th->root.u.def.section;
>        h->root.u.def.value = th->root.u.def.value & ~1;
>      }
> @@ -14953,7 +14962,7 @@ elf32_arm_finish_dynamic_symbol (bfd * output_bfd,
>  	  /* At least one non-call relocation references this .iplt entry,
>  	     so the .iplt entry is the function's canonical address.  */
>  	  sym->st_info = ELF_ST_INFO (ELF_ST_BIND (sym->st_info), STT_FUNC);
> -	  sym->st_target_internal = ST_BRANCH_TO_ARM;
> +	  ARM_SET_SYM_BRANCH_TYPE (sym->st_target_internal, ST_BRANCH_TO_ARM);
>  	  sym->st_shndx = (_bfd_elf_section_from_bfd_section
>  			   (output_bfd, htab->root.iplt->output_section));
>  	  sym->st_value = (h->plt.offset
> @@ -15237,7 +15246,9 @@ elf32_arm_finish_dynamic_sections (bfd * output_bfd,
> struct bfd_link_info * info
> 
>  		  eh = elf_link_hash_lookup (elf_hash_table (info), name,
>  					     FALSE, FALSE, TRUE);
> -		  if (eh != NULL && eh->target_internal == ST_BRANCH_TO_THUMB)
> +		  if (eh != NULL
> +		      && ARM_GET_SYM_BRANCH_TYPE (eh->target_internal)
> +			 == ST_BRANCH_TO_THUMB)
>  		    {
>  		      dyn.d_un.d_val |= 1;
>  		      bfd_elf32_swap_dyn_out (output_bfd, &dyn, dyncon);
> @@ -17325,6 +17336,7 @@ elf32_arm_swap_symbol_in (bfd * abfd,
>  {
>    if (!bfd_elf32_swap_symbol_in (abfd, psrc, pshn, dst))
>      return FALSE;
> +  dst->st_target_internal = 0;
> 
>    /* New EABI objects mark thumb function symbols by setting the low bit of
> the address.  */
> @@ -17334,20 +17346,21 @@ elf32_arm_swap_symbol_in (bfd * abfd,
>        if (dst->st_value & 1)
>  	{
>  	  dst->st_value &= ~(bfd_vma) 1;
> -	  dst->st_target_internal = ST_BRANCH_TO_THUMB;
> +	  ARM_SET_SYM_BRANCH_TYPE (dst->st_target_internal,
> +				   ST_BRANCH_TO_THUMB);
>  	}
>        else
> -	dst->st_target_internal = ST_BRANCH_TO_ARM;
> +	ARM_SET_SYM_BRANCH_TYPE (dst->st_target_internal, ST_BRANCH_TO_ARM);
>      }
>    else if (ELF_ST_TYPE (dst->st_info) == STT_ARM_TFUNC)
>      {
>        dst->st_info = ELF_ST_INFO (ELF_ST_BIND (dst->st_info), STT_FUNC);
> -      dst->st_target_internal = ST_BRANCH_TO_THUMB;
> +      ARM_SET_SYM_BRANCH_TYPE (dst->st_target_internal,
> ST_BRANCH_TO_THUMB); }
>    else if (ELF_ST_TYPE (dst->st_info) == STT_SECTION)
> -    dst->st_target_internal = ST_BRANCH_LONG;
> +    ARM_SET_SYM_BRANCH_TYPE (dst->st_target_internal, ST_BRANCH_LONG);
>    else
> -    dst->st_target_internal = ST_BRANCH_UNKNOWN;
> +    ARM_SET_SYM_BRANCH_TYPE (dst->st_target_internal, ST_BRANCH_UNKNOWN);
> 
>    return TRUE;
>  }
> @@ -17367,7 +17380,7 @@ elf32_arm_swap_symbol_out (bfd *abfd,
>       of the address set, as per the new EABI.  We do this unconditionally
>       because objcopy does not set the elf header flags until after
>       it writes out the symbol table.  */
> -  if (src->st_target_internal == ST_BRANCH_TO_THUMB)
> +  if (ARM_GET_SYM_BRANCH_TYPE (src->st_target_internal) ==
> ST_BRANCH_TO_THUMB) {
>        newsym = *src;
>        if (ELF_ST_TYPE (src->st_info) != STT_GNU_IFUNC)
> diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
> index
> 70ce3020a09e329c87b636a85d82f8be17e35094..3721be4a1dc8d85e4d0e5c28c0b2972c7
> 5ca6cc2 100644 --- a/gas/config/tc-arm.c
> +++ b/gas/config/tc-arm.c
> @@ -24176,8 +24176,8 @@ arm_adjust_symtab (void)
>  	      /* If it's a .thumb_func, declare it as so,
>  		 otherwise tag label as .code 16.  */
>  	      if (THUMB_IS_FUNC (sym))
> -		elf_sym->internal_elf_sym.st_target_internal
> -		  = ST_BRANCH_TO_THUMB;
> +		ARM_SET_SYM_BRANCH_TYPE (elf_sym-
>internal_elf_sym.st_target_internal,
> +					 ST_BRANCH_TO_THUMB);
>  	      else if (EF_ARM_EABI_VERSION (meabi_flags) < EF_ARM_EABI_VER4)
>  		elf_sym->internal_elf_sym.st_info =
>  		  ELF_ST_INFO (bind, STT_ARM_16BIT);
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index
> 6ce6f09c37ff164d327281df1cdbe5a3d4f4136f..3e8c4b599a2cc1b3762e21c40c88cdf49
> f4e2101 100644 --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -9628,7 +9628,8 @@ coff_sym_is_thumb (int val)
>  static void
>  arm_elf_make_msymbol_special(asymbol *sym, struct minimal_symbol *msym)
>  {
> -  if (ARM_SYM_BRANCH_TYPE (&((elf_symbol_type *)sym)->internal_elf_sym)
> +  elf_symbol_type *elfsym = (elf_symbol_type *) sym;
> +  if (ARM_GET_SYM_BRANCH_TYPE (elfsym->internal_elf_sym.st_target_internal)
> == ST_BRANCH_TO_THUMB)
>      MSYMBOL_SET_SPECIAL (msym);
>  }
> diff --git a/include/elf/arm.h b/include/elf/arm.h
> index
> a477f17dd8c6f05e736e46bc6402778f0ad7f19b..1a7fe1cf947bfb890884f77b3b30c0350
> 46b3fd5 100644 --- a/include/elf/arm.h
> +++ b/include/elf/arm.h
> @@ -356,7 +356,9 @@ enum arm_st_branch_type {
>    ST_BRANCH_UNKNOWN
>  };
> 
> -#define ARM_SYM_BRANCH_TYPE(SYM) \
> -  ((enum arm_st_branch_type) (SYM)->st_target_internal)
> +#define ARM_GET_SYM_BRANCH_TYPE(SYM_TARGET_INTERNAL) \
> +  ((enum arm_st_branch_type) ((SYM_TARGET_INTERNAL) & 3))
> +#define ARM_SET_SYM_BRANCH_TYPE(SYM_TARGET_INTERNAL,TYPE) \
> +  ((SYM_TARGET_INTERNAL) = ((SYM_TARGET_INTERNAL) & ~3) | ((TYPE) & 3))
> 
>  #endif /* _ELF_ARM_H */
> diff --git a/ld/emultempl/armelf.em b/ld/emultempl/armelf.em
> index
> a3b52e685fcd2d415737f3ee9ed461978a2419f6..f20820733e37b78e45eb6c5f466f6ad2c
> cea9a14 100644 --- a/ld/emultempl/armelf.em
> +++ b/ld/emultempl/armelf.em
> @@ -450,7 +450,8 @@ gld${EMULATION_NAME}_finish (void)
>        h = bfd_link_hash_lookup (link_info.hash, entry_symbol.name,
>  				FALSE, FALSE, TRUE);
>        eh = (struct elf_link_hash_entry *)h;
> -      if (!h || eh->target_internal != ST_BRANCH_TO_THUMB)
> +      if (!h || ARM_GET_SYM_BRANCH_TYPE (eh->target_internal)
> +		!= ST_BRANCH_TO_THUMB)
>  	return;
>      }
> 
> diff --git a/opcodes/arm-dis.c b/opcodes/arm-dis.c
> index
> 50b8649a9477d641e7ee88a130f35404dd68436b..ee8476b61982b7d0da03b1b7458b4f3c0
> acf8c55 100644 --- a/opcodes/arm-dis.c
> +++ b/opcodes/arm-dis.c
> @@ -6086,7 +6086,8 @@ get_sym_code_type (struct disassemble_info *info,
>    /* If the symbol has function type then use that.  */
>    if (type == STT_FUNC || type == STT_GNU_IFUNC)
>      {
> -      if (ARM_SYM_BRANCH_TYPE (&es->internal_elf_sym) ==
> ST_BRANCH_TO_THUMB) +      if (ARM_GET_SYM_BRANCH_TYPE
> (es->internal_elf_sym.st_target_internal) +	  == ST_BRANCH_TO_THUMB)
>  	*map_type = MAP_THUMB;
>        else
>  	*map_type = MAP_ARM;
> @@ -6388,9 +6389,9 @@ print_insn (bfd_vma pc, struct disassemble_info *info,
> bfd_boolean little) es = *(elf_symbol_type **)(info->symbols);
>  	  type = ELF_ST_TYPE (es->internal_elf_sym.st_info);
> 
> -	  is_thumb = ((ARM_SYM_BRANCH_TYPE (&es->internal_elf_sym)
> -		       == ST_BRANCH_TO_THUMB)
> -		      || type == STT_ARM_16BIT);
> +	  is_thumb =
> +	    ((ARM_GET_SYM_BRANCH_TYPE (es->internal_elf_sym.st_target_internal)
> +	      == ST_BRANCH_TO_THUMB) || type == STT_ARM_16BIT);
>  	}
>        else if (bfd_asymbol_flavour (*info->symbols)
>  	       == bfd_target_mach_o_flavour)
> 
> 
> The patch doesn't show any regression when running the binutils-gdb
> testsuite for the arm-none-eabi target.
> 
> Any comments?
> 
> Best regards,
> 
> Thomas


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