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: Nick Clifton <nickc at redhat dot com>
- Cc: binutils at sourceware dot org
- Date: Thu, 05 May 2016 11:06:12 +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> <32442566 dot GSo4DN4Amp at e108577-lin> <57038DDD dot 9090401 at redhat dot com>
On Tuesday 05 April 2016 11:05:17 Nick Clifton wrote:
> Hi Thomas,
>
> >>>> -#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))
> >>
> >> It occurs to me that an out-of-range value for the TYPE parameter to the
> >> ARM_SET_SYM_BRANCH_TYPE macro ought to be flagged as an error, and not
> >> silently truncated. You could probably let the compiler's enum checking
> >> do
> >> this...
> >
> > Any advice on how to catch any growth in terms of enum size? The point is
> > for later commit [1] to start using currently unused bits but we need to
> > make sure these new macros are updated if branch type enum increase in
> > size.
> >
> > [1] https://sourceware.org/ml/binutils/2016-03/msg00379.html
>
> How about this ? It is a bit complicated because I could not find a way to
> use the compiler's enum checking to verify the bitfield size, so I had to
> use assertions instead. Still I think that it should work, and be
> relatively simple to extend as necessary.
>
> Cheers
> Nick
>
>
> enum arm_st_branch_type
> {
> ST_BRANCH_TO_ARM = 0,
> ST_BRANCH_TO_THUMB,
> ST_BRANCH_LONG,
> ST_BRANCH_UNKNOWN,
> ST_BRANCH_ENUM_SIZE
> };
>
> #define NUM_ENUM_ARM_ST_BRANCH_TYPE_BITS 3
> #define CMSE_BITFIELD (NUM_ENUM_ARM_ST_BRANCH_TYPE_BITS
> + 1) #define ENUM_ARM_ST_BRANCH_BITMASK ((1 <<
> NUM_ENUM_ARM_ST_BRANCH_TYPE_BITS) - 1) #define ARM_CMSE_SPCL_BITMASK
> (1 << CMSE_BITFIELD)
>
> #define ARM_SYM_BRANCH_TYPE(SYM) \
> ARM_GET_SYM_BRANCH_TYPE ((SYM)->st_target_internal)
>
> #define ARM_GET_SYM_BRANCH_TYPE(STI) \
> ((enum arm_st_branch_type) (STI & ENUM_ARM_ST_BRANCH_BITMASK))
>
> #ifdef BFD_ASSERT
> #define ARM_SET_SYM_BRANCH_TYPE(STI, TYPE) \
> do \
> { \
> unsigned typ = (TYPE); \
> BFD_ASSERT (typ <= ST_BRANCH_ENUM_SIZE); \
> BFD_ASSERT ((1 << NUM_ENUM_ARM_ST_BRANCH_TYPE_BITS) >=
> ST_BRANCH_ENUM_SIZE); \ (STI) &= ~ ENUM_ARM_ST_BRANCH_BITMASK; \
> (STI) |= typ & ENUM_ARM_ST_BRANCH_BITMASK; \
> } \
> while (0)
> #else
> #define ARM_SET_SYM_BRANCH_TYPE(STI, TYPE) \
> do \
> { \
> unsigned typ = (TYPE); \
> (STI) &= ~ ENUM_ARM_ST_BRANCH_BITMASK; \
> (STI) |= typ & ENUM_ARM_ST_BRANCH_BITMASK; \
> } \
> while (0)
> #endif
>
> /* Get or set whether a symbol is a special symbol of an entry function of
> CMSE secure code. */
> #define ARM_GET_SYM_CMSE_SPCL(STI) \
> (((STI) & ARM_CMSE_SPCL_BITMASK) ? 1 : 0)
>
> #define ARM_SET_SYM_CMSE_SPCL(STI) \
> do \
> { \
> (STI) |= ARM_CMSE_SPCL_BITMASK; \
> } \
> while (0)
Quite verbose and dynamic checking only but I could not find anything better
indeed. I just removed the use of a variable in the setters.
Updated 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 ***
2016-04-12 Thomas Preud'homme <thomas.preudhomme@arm.com>
Nick Clifton <nickc@redhat.com>
* arm.h (enum arm_st_branch_type): Add new ST_BRANCH_ENUM_SIZE
enumerator.
(NUM_ENUM_ARM_ST_BRANCH_TYPE_BITS): New macro.
(ENUM_ARM_ST_BRANCH_TYPE_BITMASK): Likewise.
(ARM_SYM_BRANCH_TYPE): Replace by ...
(ARM_GET_SYM_BRANCH_TYPE): This and ...
(ARM_SET_SYM_BRANCH_TYPE): This in two versions depending on whether
BFD_ASSERT is defined or not.
*** 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
052497ff3ddbf2c85112969409e11ec937745f4e..71d60222cab2cd0634ff61362ee9eb4d84a85f14
100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -5459,7 +5459,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,
@@ -5534,7 +5535,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;
}
@@ -6626,7 +6628,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;
@@ -11484,28 +11487,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
@@ -14246,7 +14255,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
@@ -14394,7 +14403,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;
@@ -14414,12 +14423,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;
}
@@ -15074,7 +15083,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
@@ -15358,7 +15367,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);
@@ -17526,6 +17537,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. */
@@ -17535,20 +17547,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;
}
@@ -17568,7 +17581,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
7483212ce1e513c59c79b5a6b4cafc1748dc61f8..4f79c1a93161210e2a0da1dd11997614dff04173
100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -24319,8 +24319,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
5a2d490df77905d6cf6a82041c7304b57581a82d..d02bdb27b99d7fc4d179e8e51c1a421c7c101da8
100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8467,7 +8467,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
bd9fd8bcf540933cf99957e1c138b479a83958ca..bafc03c52ee686671e847eff272ab5cc8a79398c
100644
--- a/include/elf/arm.h
+++ b/include/elf/arm.h
@@ -359,10 +359,29 @@ enum arm_st_branch_type {
ST_BRANCH_TO_ARM,
ST_BRANCH_TO_THUMB,
ST_BRANCH_LONG,
- ST_BRANCH_UNKNOWN
+ ST_BRANCH_UNKNOWN,
+ ST_BRANCH_ENUM_SIZE
};
-#define ARM_SYM_BRANCH_TYPE(SYM) \
- ((enum arm_st_branch_type) (SYM)->st_target_internal)
+#define NUM_ENUM_ARM_ST_BRANCH_TYPE_BITS 2
+#define ENUM_ARM_ST_BRANCH_TYPE_BITMASK \
+ ((1 << NUM_ENUM_ARM_ST_BRANCH_TYPE_BITS) - 1)
+
+#define ARM_GET_SYM_BRANCH_TYPE(STI) \
+ ((enum arm_st_branch_type) ((STI) & ENUM_ARM_ST_BRANCH_TYPE_BITMASK))
+#ifdef BFD_ASSERT
+#define ARM_SET_SYM_BRANCH_TYPE(STI, TYPE) \
+ do { \
+ BFD_ASSERT (TYPE <= ST_BRANCH_ENUM_SIZE); \
+ BFD_ASSERT ((1 << NUM_ENUM_ARM_ST_BRANCH_TYPE_BITS) \
+ >= ST_BRANCH_ENUM_SIZE); \
+ (STI) = (((STI) & ~ENUM_ARM_ST_BRANCH_TYPE_BITMASK) \
+ | ((TYPE) & ENUM_ARM_ST_BRANCH_TYPE_BITMASK)); \
+ } while (0)
+#else
+#define ARM_SET_SYM_BRANCH_TYPE(STI, TYPE) \
+ (STI) = (((STI) & ~ENUM_ARM_ST_BRANCH_TYPE_BITMASK) \
+ | ((TYPE) & ENUM_ARM_ST_BRANCH_TYPE_BITMASK))
+#endif
#endif /* _ELF_ARM_H */
diff --git a/ld/emultempl/armelf.em b/ld/emultempl/armelf.em
index
bfb7ee498a8d4206e89ed127efcc6aa4c5bf836b..caa2fbfff3633f2a7f69a7b197574eed8279d6c4
100644
--- a/ld/emultempl/armelf.em
+++ b/ld/emultempl/armelf.em
@@ -447,7 +447,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
c31cb2dfc26ac093a947da1b85d45b79cf0bb048..538574f139135e6edb2ee4c8f51ff5afa4cbccf6
100644
--- a/opcodes/arm-dis.c
+++ b/opcodes/arm-dis.c
@@ -6112,7 +6112,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;
@@ -6479,9 +6480,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)
Best regards,
Thomas