This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [RFC PATCH, binutils, ARM 4/11, ping] Use getters/setters to access ARM branch type
- From: Thomas Preudhomme <thomas dot preudhomme at foss dot arm dot com>
- To: binutils at sourceware dot org
- Date: Tue, 29 Mar 2016 15:35:23 +0100
- Subject: Re: [RFC PATCH, binutils, ARM 4/11, ping] Use getters/setters to access ARM branch type
- Authentication-results: sourceware.org; auth=none
- References: <004c01d13d57$975ae2a0$c610a7e0$ at foss dot arm dot com>
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