* [RFC PATCH, binutils, ARM 5/9] Allow veneers to claim veneered symbols
@ 2015-12-23 7:58 Thomas Preud'homme
2016-03-29 14:37 ` Thomas Preudhomme
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Preud'homme @ 2015-12-23 7:58 UTC (permalink / raw)
To: binutils
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 add infrastructure to allow a veneer to take over the symbol of the code it is veneering, as required for Secure Gateway veneers.
[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 entry is as follows:
*** bfd/ChangeLog ***
2015-12-16 Thomas Preud'homme <thomas.preudhomme@arm.com>
* elf32-arm.c (arm_stub_sym_claimed): New function.
(elf32_arm_create_stub): Use veneered symbol name and section if
veneer needs to claim its symbol, and keep logic unchanged otherwise.
(arm_claim_sym_one_stub): New function.
(arm_map_one_stub): Call arm_claim_sym_one_stub if veneer needs to
claim veneered symbol, otherwise create local symbol as before.
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index 6c8060a2c559ff5bd8a8e7f414f0a471dfa32cc8..11e6137732b420b49166567ce755fe413dab76cc 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -4259,6 +4259,41 @@ arm_stub_required_alignment (enum elf32_arm_stub_type stub_type)
}
}
+/* Returns whether stubs of type STUB_TYPE take over the symbol they are
+ veneering (TRUE) or have their own symbol (FALSE). */
+
+static bfd_boolean
+arm_stub_sym_claimed (enum elf32_arm_stub_type stub_type)
+{
+ switch (stub_type)
+ {
+ case arm_stub_a8_veneer_b_cond:
+ case arm_stub_a8_veneer_b:
+ case arm_stub_a8_veneer_bl:
+ case arm_stub_long_branch_any_any:
+ case arm_stub_long_branch_v4t_arm_thumb:
+ case arm_stub_long_branch_thumb_only:
+ case arm_stub_long_branch_v4t_thumb_thumb:
+ case arm_stub_long_branch_v4t_thumb_arm:
+ case arm_stub_short_branch_v4t_thumb_arm:
+ case arm_stub_long_branch_any_arm_pic:
+ case arm_stub_long_branch_any_thumb_pic:
+ case arm_stub_long_branch_v4t_thumb_thumb_pic:
+ case arm_stub_long_branch_v4t_arm_thumb_pic:
+ case arm_stub_long_branch_v4t_thumb_arm_pic:
+ case arm_stub_long_branch_thumb_only_pic:
+ case arm_stub_long_branch_any_tls_pic:
+ case arm_stub_long_branch_v4t_thumb_tls_pic:
+ case arm_stub_a8_veneer_blx:
+ case arm_stub_long_branch_arm_nacl:
+ case arm_stub_long_branch_arm_nacl_pic:
+ return FALSE;
+
+ default:
+ abort (); /* Should be unreachable. */
+ }
+}
+
static bfd_boolean
arm_build_one_stub (struct bfd_hash_entry *gen_entry,
void * in_arg)
@@ -5069,23 +5104,30 @@ elf32_arm_create_stub (struct elf32_arm_link_hash_table *htab,
BFD_ASSERT (stub_type != arm_stub_none);
*new_stub = FALSE;
- BFD_ASSERT (irela);
- BFD_ASSERT (section);
+ if (arm_stub_sym_claimed (stub_type))
+ stub_name = sym_name;
+ else
+ {
+ BFD_ASSERT (irela);
+ BFD_ASSERT (section);
- /* Support for grouping stub sections. */
- id_sec = htab->stub_group[section->id].link_sec;
+ /* Support for grouping stub sections. */
+ id_sec = htab->stub_group[section->id].link_sec;
- /* Get the name of this stub. */
- stub_name = elf32_arm_stub_name (id_sec, sym_sec, hash, irela, stub_type);
- if (!stub_name)
- return FALSE;
+ /* Get the name of this stub. */
+ stub_name = elf32_arm_stub_name (id_sec, sym_sec, hash, irela,
+ stub_type);
+ if (!stub_name)
+ return FALSE;
+ }
stub_entry = arm_stub_hash_lookup (&htab->stub_hash_table, stub_name, FALSE,
FALSE);
/* The proper stub has already been created, just update its value. */
if (stub_entry != NULL)
{
- free (stub_name);
+ if (!arm_stub_sym_claimed (stub_type))
+ free (stub_name);
stub_entry->target_value = sym_value;
return TRUE;
}
@@ -5093,7 +5135,8 @@ elf32_arm_create_stub (struct elf32_arm_link_hash_table *htab,
stub_entry = elf32_arm_add_stub (stub_name, section, htab);
if (stub_entry == NULL)
{
- free (stub_name);
+ if (!arm_stub_sym_claimed (stub_type))
+ free (stub_name);
return FALSE;
}
@@ -5103,31 +5146,36 @@ elf32_arm_create_stub (struct elf32_arm_link_hash_table *htab,
stub_entry->h = hash;
stub_entry->branch_type = branch_type;
- if (sym_name == NULL)
- sym_name = "unnamed";
- stub_entry->output_name = (char *)
- bfd_alloc (htab->stub_bfd, sizeof (THUMB2ARM_GLUE_ENTRY_NAME)
- + strlen (sym_name));
- if (stub_entry->output_name == NULL)
+ if (arm_stub_sym_claimed (stub_type))
+ stub_entry->output_name = sym_name;
+ else
{
- free (stub_name);
- return FALSE;
- }
+ if (sym_name == NULL)
+ sym_name = "unnamed";
+ stub_entry->output_name = (char *)
+ bfd_alloc (htab->stub_bfd, sizeof (THUMB2ARM_GLUE_ENTRY_NAME)
+ + strlen (sym_name));
+ if (stub_entry->output_name == NULL)
+ {
+ free (stub_name);
+ return FALSE;
+ }
- /* For historical reasons, use the existing names for ARM-to-Thumb and
- Thumb-to-ARM stubs. */
- r_type = ELF32_R_TYPE (irela->r_info);
- if ((r_type == (unsigned int) R_ARM_THM_CALL
- || r_type == (unsigned int) R_ARM_THM_JUMP24
- || r_type == (unsigned int) R_ARM_THM_JUMP19)
- && branch_type == ST_BRANCH_TO_ARM)
- sprintf (stub_entry->output_name, THUMB2ARM_GLUE_ENTRY_NAME, sym_name);
- else if ((r_type == (unsigned int) R_ARM_CALL
- || r_type == (unsigned int) R_ARM_JUMP24)
- && branch_type == ST_BRANCH_TO_THUMB)
- sprintf (stub_entry->output_name, ARM2THUMB_GLUE_ENTRY_NAME, sym_name);
- else
- sprintf (stub_entry->output_name, STUB_ENTRY_NAME, sym_name);
+ /* For historical reasons, use the existing names for ARM-to-Thumb and
+ Thumb-to-ARM stubs. */
+ r_type = ELF32_R_TYPE (irela->r_info);
+ if ((r_type == (unsigned int) R_ARM_THM_CALL
+ || r_type == (unsigned int) R_ARM_THM_JUMP24
+ || r_type == (unsigned int) R_ARM_THM_JUMP19)
+ && branch_type == ST_BRANCH_TO_ARM)
+ sprintf (stub_entry->output_name, THUMB2ARM_GLUE_ENTRY_NAME, sym_name);
+ else if ((r_type == (unsigned int) R_ARM_CALL
+ || r_type == (unsigned int) R_ARM_JUMP24)
+ && branch_type == ST_BRANCH_TO_THUMB)
+ sprintf (stub_entry->output_name, ARM2THUMB_GLUE_ENTRY_NAME, sym_name);
+ else
+ sprintf (stub_entry->output_name, STUB_ENTRY_NAME, sym_name);
+ }
*new_stub = TRUE;
return TRUE;
@@ -15705,6 +15753,22 @@ elf32_arm_output_plt_map (struct elf_link_hash_entry *h, void *inf)
&h->plt, &eh->plt);
}
+/* Bind a veneered symbol to its veneer identified by its hash entry
+ STUB_ENTRY. The veneered location thus loose its symbol. */
+
+static bfd_boolean
+arm_claim_sym_one_stub (struct elf32_arm_stub_hash_entry *stub_entry)
+{
+ struct elf32_arm_link_hash_entry *hash = stub_entry->h;
+
+ BFD_ASSERT (hash);
+ hash->root.root.u.def.section = stub_entry->stub_sec;
+ hash->root.root.u.def.value = stub_entry->stub_offset;
+ hash->root.size = stub_entry->stub_size;
+
+ return TRUE;
+}
+
/* Output a single local symbol for a generated stub. */
static bfd_boolean
@@ -15751,24 +15815,30 @@ arm_map_one_stub (struct bfd_hash_entry * gen_entry,
return TRUE;
addr = (bfd_vma) stub_entry->stub_offset;
- stub_name = stub_entry->output_name;
-
template_sequence = stub_entry->stub_template;
- switch (template_sequence[0].type)
+
+ if (arm_stub_sym_claimed (stub_entry->stub_type))
+ arm_claim_sym_one_stub (stub_entry);
+ else
{
- case ARM_TYPE:
- if (!elf32_arm_output_stub_sym (osi, stub_name, addr, stub_entry->stub_size))
- return FALSE;
- break;
- case THUMB16_TYPE:
- case THUMB32_TYPE:
- if (!elf32_arm_output_stub_sym (osi, stub_name, addr | 1,
- stub_entry->stub_size))
- return FALSE;
- break;
- default:
- BFD_FAIL ();
- return 0;
+ stub_name = stub_entry->output_name;
+ switch (template_sequence[0].type)
+ {
+ case ARM_TYPE:
+ if (!elf32_arm_output_stub_sym (osi, stub_name, addr,
+ stub_entry->stub_size))
+ return FALSE;
+ break;
+ case THUMB16_TYPE:
+ case THUMB32_TYPE:
+ if (!elf32_arm_output_stub_sym (osi, stub_name, addr | 1,
+ stub_entry->stub_size))
+ return FALSE;
+ break;
+ default:
+ BFD_FAIL ();
+ return 0;
+ }
}
prev_type = DATA_TYPE;
The patch doesn't show any regression when running the binutils-gdb testsuite for the arm-none-eabi target.
Any comments?
Best regards,
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH, binutils, ARM 5/9] Allow veneers to claim veneered symbols
2015-12-23 7:58 [RFC PATCH, binutils, ARM 5/9] Allow veneers to claim veneered symbols Thomas Preud'homme
@ 2016-03-29 14:37 ` Thomas Preudhomme
2016-03-30 15:49 ` Nick Clifton
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Preudhomme @ 2016-03-29 14:37 UTC (permalink / raw)
To: binutils
On Wednesday 23 December 2015 15:58:34 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 add infrastructure to allow a
> veneer to take over the symbol of the code it is veneering, as required for
> Secure Gateway veneers.
>
>
> [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
Please find an updated patch below.
*** bfd/ChangeLog ***
2016-03-23 Thomas Preud'homme <thomas.preudhomme@arm.com>
* elf32-arm.c (enum elf32_arm_stub_type): New max_stub_type
enumerator.
(arm_stub_sym_claimed): New function.
(elf32_arm_create_stub): Use veneered symbol name and section if
veneer needs to claim its symbol, and keep logic unchanged otherwise.
(arm_stub_claim_sym): New function.
(arm_map_one_stub): Call arm_stub_claim_sym if veneer needs to claim
veneered symbol, otherwise create local symbol as before.
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index
99bf10277ca80eb14459a9fa8e14cce35ec828de..7beac0a3b81239a1b87db41518232e74b2531c46
100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -2632,8 +2632,9 @@ enum elf32_arm_stub_type
{
arm_stub_none,
DEF_STUBS
+ max_stub_type,
/* Note the first a8_veneer type. */
- arm_stub_a8_veneer_lwm = arm_stub_a8_veneer_b_cond
+ arm_stub_a8_veneer_lwm = arm_stub_a8_veneer_b_cond,
};
#undef DEF_STUB
@@ -4334,6 +4335,24 @@ arm_stub_required_alignment (enum elf32_arm_stub_type
stub_type)
}
}
+/* Returns whether stubs of type STUB_TYPE take over the symbol they are
+ veneering (TRUE) or have their own symbol (FALSE). */
+
+static bfd_boolean
+arm_stub_sym_claimed (enum elf32_arm_stub_type stub_type)
+{
+ if (stub_type >= max_stub_type)
+ abort (); /* Should be unreachable. */
+
+ switch (stub_type)
+ {
+ default:
+ return FALSE;
+ }
+
+ abort (); /* Should be unreachable. */
+}
+
static bfd_boolean
arm_build_one_stub (struct bfd_hash_entry *gen_entry,
void * in_arg)
@@ -5140,27 +5159,35 @@ elf32_arm_create_stub (struct
elf32_arm_link_hash_table *htab,
char *stub_name;
struct elf32_arm_stub_hash_entry *stub_entry;
unsigned int r_type;
+ bfd_boolean sym_claimed = arm_stub_sym_claimed (stub_type);
BFD_ASSERT (stub_type != arm_stub_none);
*new_stub = FALSE;
- BFD_ASSERT (irela);
- BFD_ASSERT (section);
+ if (sym_claimed)
+ stub_name = sym_name;
+ else
+ {
+ BFD_ASSERT (irela);
+ BFD_ASSERT (section);
- /* Support for grouping stub sections. */
- id_sec = htab->stub_group[section->id].link_sec;
+ /* Support for grouping stub sections. */
+ id_sec = htab->stub_group[section->id].link_sec;
- /* Get the name of this stub. */
- stub_name = elf32_arm_stub_name (id_sec, sym_sec, hash, irela, stub_type);
- if (!stub_name)
- return FALSE;
+ /* Get the name of this stub. */
+ stub_name = elf32_arm_stub_name (id_sec, sym_sec, hash, irela,
+ stub_type);
+ if (!stub_name)
+ return FALSE;
+ }
stub_entry = arm_stub_hash_lookup (&htab->stub_hash_table, stub_name,
FALSE,
FALSE);
/* The proper stub has already been created, just update its value. */
if (stub_entry != NULL)
{
- free (stub_name);
+ if (!sym_claimed)
+ free (stub_name);
stub_entry->target_value = sym_value;
return TRUE;
}
@@ -5168,7 +5195,8 @@ elf32_arm_create_stub (struct elf32_arm_link_hash_table
*htab,
stub_entry = elf32_arm_add_stub (stub_name, section, htab);
if (stub_entry == NULL)
{
- free (stub_name);
+ if (!sym_claimed)
+ free (stub_name);
return FALSE;
}
@@ -5178,31 +5206,36 @@ elf32_arm_create_stub (struct
elf32_arm_link_hash_table *htab,
stub_entry->h = hash;
stub_entry->branch_type = branch_type;
- if (sym_name == NULL)
- sym_name = "unnamed";
- stub_entry->output_name = (char *)
- bfd_alloc (htab->stub_bfd, sizeof (THUMB2ARM_GLUE_ENTRY_NAME)
- + strlen (sym_name));
- if (stub_entry->output_name == NULL)
+ if (sym_claimed)
+ stub_entry->output_name = sym_name;
+ else
{
- free (stub_name);
- return FALSE;
- }
+ if (sym_name == NULL)
+ sym_name = "unnamed";
+ stub_entry->output_name = (char *)
+ bfd_alloc (htab->stub_bfd, sizeof (THUMB2ARM_GLUE_ENTRY_NAME)
+ + strlen (sym_name));
+ if (stub_entry->output_name == NULL)
+ {
+ free (stub_name);
+ return FALSE;
+ }
- /* For historical reasons, use the existing names for ARM-to-Thumb and
- Thumb-to-ARM stubs. */
- r_type = ELF32_R_TYPE (irela->r_info);
- if ((r_type == (unsigned int) R_ARM_THM_CALL
- || r_type == (unsigned int) R_ARM_THM_JUMP24
- || r_type == (unsigned int) R_ARM_THM_JUMP19)
- && branch_type == ST_BRANCH_TO_ARM)
- sprintf (stub_entry->output_name, THUMB2ARM_GLUE_ENTRY_NAME, sym_name);
- else if ((r_type == (unsigned int) R_ARM_CALL
- || r_type == (unsigned int) R_ARM_JUMP24)
- && branch_type == ST_BRANCH_TO_THUMB)
- sprintf (stub_entry->output_name, ARM2THUMB_GLUE_ENTRY_NAME, sym_name);
- else
- sprintf (stub_entry->output_name, STUB_ENTRY_NAME, sym_name);
+ /* For historical reasons, use the existing names for ARM-to-Thumb and
+ Thumb-to-ARM stubs. */
+ r_type = ELF32_R_TYPE (irela->r_info);
+ if ((r_type == (unsigned int) R_ARM_THM_CALL
+ || r_type == (unsigned int) R_ARM_THM_JUMP24
+ || r_type == (unsigned int) R_ARM_THM_JUMP19)
+ && branch_type == ST_BRANCH_TO_ARM)
+ sprintf (stub_entry->output_name, THUMB2ARM_GLUE_ENTRY_NAME, sym_name);
+ else if ((r_type == (unsigned int) R_ARM_CALL
+ || r_type == (unsigned int) R_ARM_JUMP24)
+ && branch_type == ST_BRANCH_TO_THUMB)
+ sprintf (stub_entry->output_name, ARM2THUMB_GLUE_ENTRY_NAME, sym_name);
+ else
+ sprintf (stub_entry->output_name, STUB_ENTRY_NAME, sym_name);
+ }
*new_stub = TRUE;
return TRUE;
@@ -15853,6 +15886,20 @@ elf32_arm_output_plt_map (struct elf_link_hash_entry
*h, void *inf)
&h->plt, &eh->plt);
}
+/* Bind a veneered symbol to its veneer identified by its hash entry
+ STUB_ENTRY. The veneered location thus loose its symbol. */
+
+static void
+arm_stub_claim_sym (struct elf32_arm_stub_hash_entry *stub_entry)
+{
+ struct elf32_arm_link_hash_entry *hash = stub_entry->h;
+
+ BFD_ASSERT (hash);
+ hash->root.root.u.def.section = stub_entry->stub_sec;
+ hash->root.root.u.def.value = stub_entry->stub_offset;
+ hash->root.size = stub_entry->stub_size;
+}
+
/* Output a single local symbol for a generated stub. */
static bfd_boolean
@@ -15899,24 +15946,30 @@ arm_map_one_stub (struct bfd_hash_entry * gen_entry,
return TRUE;
addr = (bfd_vma) stub_entry->stub_offset;
- stub_name = stub_entry->output_name;
-
template_sequence = stub_entry->stub_template;
- switch (template_sequence[0].type)
+
+ if (arm_stub_sym_claimed (stub_entry->stub_type))
+ arm_stub_claim_sym (stub_entry);
+ else
{
- case ARM_TYPE:
- if (!elf32_arm_output_stub_sym (osi, stub_name, addr, stub_entry-
>stub_size))
- return FALSE;
- break;
- case THUMB16_TYPE:
- case THUMB32_TYPE:
- if (!elf32_arm_output_stub_sym (osi, stub_name, addr | 1,
- stub_entry->stub_size))
- return FALSE;
- break;
- default:
- BFD_FAIL ();
- return 0;
+ stub_name = stub_entry->output_name;
+ switch (template_sequence[0].type)
+ {
+ case ARM_TYPE:
+ if (!elf32_arm_output_stub_sym (osi, stub_name, addr,
+ stub_entry->stub_size))
+ return FALSE;
+ break;
+ case THUMB16_TYPE:
+ case THUMB32_TYPE:
+ if (!elf32_arm_output_stub_sym (osi, stub_name, addr | 1,
+ stub_entry->stub_size))
+ return FALSE;
+ break;
+ default:
+ BFD_FAIL ();
+ return 0;
+ }
}
prev_type = DATA_TYPE;
Is this ok for master branch?
Best regards,
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH, binutils, ARM 5/9] Allow veneers to claim veneered symbols
2016-03-29 14:37 ` Thomas Preudhomme
@ 2016-03-30 15:49 ` Nick Clifton
2016-04-04 14:19 ` Thomas Preudhomme
2016-05-05 10:08 ` [RFC PATCH, binutils, ARM 5/11] " Thomas Preudhomme
0 siblings, 2 replies; 7+ messages in thread
From: Nick Clifton @ 2016-03-30 15:49 UTC (permalink / raw)
To: Thomas Preudhomme, binutils
Hi Thomas.
> @@ -2632,8 +2632,9 @@ enum elf32_arm_stub_type
> {
> arm_stub_none,
> DEF_STUBS
> + max_stub_type,
> /* Note the first a8_veneer type. */
> - arm_stub_a8_veneer_lwm = arm_stub_a8_veneer_b_cond
> + arm_stub_a8_veneer_lwm = arm_stub_a8_veneer_b_cond,
> };
Why do you need the extra comma at the end of the enum ?
I don't like that "max_stub_type" is not actually the maximum possible
value for an elf32_arn_stub_type enum. The name is misleading. Maybe
you could use something like "last_non_veneer_stub".
> + switch (stub_type)
> + {
> + default:
> + return FALSE;
> + }
> +
> + abort (); /* Should be unreachable. */
> +}
I presume that your intention is to extend the switch table later on,
but there is no need for that now. Just use:
return FALSE;
Cheers
Nick
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH, binutils, ARM 5/9] Allow veneers to claim veneered symbols
2016-03-30 15:49 ` Nick Clifton
@ 2016-04-04 14:19 ` Thomas Preudhomme
2016-04-05 10:06 ` Nick Clifton
2016-05-05 10:08 ` [RFC PATCH, binutils, ARM 5/11] " Thomas Preudhomme
1 sibling, 1 reply; 7+ messages in thread
From: Thomas Preudhomme @ 2016-04-04 14:19 UTC (permalink / raw)
To: Nick Clifton; +Cc: binutils
On Wednesday 30 March 2016 16:49:03 Nick Clifton wrote:
> Hi Thomas.
>
> > @@ -2632,8 +2632,9 @@ enum elf32_arm_stub_type
> >
> > {
> >
> > arm_stub_none,
> > DEF_STUBS
> >
> > + max_stub_type,
> >
> > /* Note the first a8_veneer type. */
> >
> > - arm_stub_a8_veneer_lwm = arm_stub_a8_veneer_b_cond
> > + arm_stub_a8_veneer_lwm = arm_stub_a8_veneer_b_cond,
> >
> > };
>
> Why do you need the extra comma at the end of the enum ?
There's no need, it's just an artifact from experiments I tried.
>
> I don't like that "max_stub_type" is not actually the maximum possible
> value for an elf32_arn_stub_type enum. The name is misleading. Maybe
> you could use something like "last_non_veneer_stub".
Last is confusing because it's last + 1. To be fair, I don't think this should
be a stub type, rather a const global variable. I can include that in the
Cortex-A8 refactoring patch, what do you think?
>
> > + switch (stub_type)
> > + {
> > + default:
> > + return FALSE;
> > + }
> > +
> > + abort (); /* Should be unreachable. */
> > +}
>
> I presume that your intention is to extend the switch table later on,
> but there is no need for that now. Just use:
>
> return FALSE;
Ok, will respin all patches following that same approach.
Best regards,
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH, binutils, ARM 5/9] Allow veneers to claim veneered symbols
2016-04-04 14:19 ` Thomas Preudhomme
@ 2016-04-05 10:06 ` Nick Clifton
0 siblings, 0 replies; 7+ messages in thread
From: Nick Clifton @ 2016-04-05 10:06 UTC (permalink / raw)
To: Thomas Preudhomme; +Cc: binutils
Hi Thomas,
>> I don't like that "max_stub_type" is not actually the maximum possible
>> value for an elf32_arn_stub_type enum. The name is misleading. Maybe
>> you could use something like "last_non_veneer_stub".
>
> Last is confusing because it's last + 1. To be fair, I don't think this should
> be a stub type, rather a const global variable. I can include that in the
> Cortex-A8 refactoring patch, what do you think?
I think that would be much cleaner, so yes please.
Cheers
Nick
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH, binutils, ARM 5/11] Allow veneers to claim veneered symbols
2016-03-30 15:49 ` Nick Clifton
2016-04-04 14:19 ` Thomas Preudhomme
@ 2016-05-05 10:08 ` Thomas Preudhomme
2016-05-09 11:58 ` Nick Clifton
1 sibling, 1 reply; 7+ messages in thread
From: Thomas Preudhomme @ 2016-05-05 10:08 UTC (permalink / raw)
To: Nick Clifton; +Cc: binutils
On Tuesday 05 April 2016 11:06:30 Nick Clifton wrote:
> Hi Thomas,
Hi Nick,
>
> >> I don't like that "max_stub_type" is not actually the maximum possible
> >> value for an elf32_arn_stub_type enum. The name is misleading. Maybe
> >> you could use something like "last_non_veneer_stub".
> >
> > Last is confusing because it's last + 1. To be fair, I don't think this
> > should be a stub type, rather a const global variable. I can include that
> > in the Cortex-A8 refactoring patch, what do you think?
>
> I think that would be much cleaner, so yes please.
Please find an updated patch with all your comments addressed. ChangeLog entry
is unchanged:
*** bfd/ChangeLog ***
2016-03-23 Thomas Preud'homme <thomas.preudhomme@arm.com>
* elf32-arm.c (enum elf32_arm_stub_type): New max_stub_type
enumerator.
(arm_stub_sym_claimed): New function.
(elf32_arm_create_stub): Use veneered symbol name and section if
veneer needs to claim its symbol, and keep logic unchanged otherwise.
(arm_stub_claim_sym): New function.
(arm_map_one_stub): Call arm_stub_claim_sym if veneer needs to claim
veneered symbol, otherwise create local symbol as before.
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index
71d60222cab2cd0634ff61362ee9eb4d84a85f14..02977358ac69deab379f2d826c44bfc5efa49e1a
100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -2632,6 +2632,7 @@ enum elf32_arm_stub_type
{
arm_stub_none,
DEF_STUBS
+ max_stub_type
};
#undef DEF_STUB
@@ -4335,6 +4336,18 @@ arm_stub_required_alignment (enum elf32_arm_stub_type
stub_type)
}
}
+/* Returns whether stubs of type STUB_TYPE take over the symbol they are
+ veneering (TRUE) or have their own symbol (FALSE). */
+
+static bfd_boolean
+arm_stub_sym_claimed (enum elf32_arm_stub_type stub_type)
+{
+ if (stub_type >= max_stub_type)
+ abort (); /* Should be unreachable. */
+
+ return FALSE;
+}
+
static bfd_boolean
arm_build_one_stub (struct bfd_hash_entry *gen_entry,
void * in_arg)
@@ -5141,27 +5154,35 @@ elf32_arm_create_stub (struct
elf32_arm_link_hash_table *htab,
char *stub_name;
struct elf32_arm_stub_hash_entry *stub_entry;
unsigned int r_type;
+ bfd_boolean sym_claimed = arm_stub_sym_claimed (stub_type);
BFD_ASSERT (stub_type != arm_stub_none);
*new_stub = FALSE;
- BFD_ASSERT (irela);
- BFD_ASSERT (section);
+ if (sym_claimed)
+ stub_name = sym_name;
+ else
+ {
+ BFD_ASSERT (irela);
+ BFD_ASSERT (section);
- /* Support for grouping stub sections. */
- id_sec = htab->stub_group[section->id].link_sec;
+ /* Support for grouping stub sections. */
+ id_sec = htab->stub_group[section->id].link_sec;
- /* Get the name of this stub. */
- stub_name = elf32_arm_stub_name (id_sec, sym_sec, hash, irela, stub_type);
- if (!stub_name)
- return FALSE;
+ /* Get the name of this stub. */
+ stub_name = elf32_arm_stub_name (id_sec, sym_sec, hash, irela,
+ stub_type);
+ if (!stub_name)
+ return FALSE;
+ }
stub_entry = arm_stub_hash_lookup (&htab->stub_hash_table, stub_name,
FALSE,
FALSE);
/* The proper stub has already been created, just update its value. */
if (stub_entry != NULL)
{
- free (stub_name);
+ if (!sym_claimed)
+ free (stub_name);
stub_entry->target_value = sym_value;
return TRUE;
}
@@ -5169,7 +5190,8 @@ elf32_arm_create_stub (struct elf32_arm_link_hash_table
*htab,
stub_entry = elf32_arm_add_stub (stub_name, section, htab);
if (stub_entry == NULL)
{
- free (stub_name);
+ if (!sym_claimed)
+ free (stub_name);
return FALSE;
}
@@ -5179,31 +5201,36 @@ elf32_arm_create_stub (struct
elf32_arm_link_hash_table *htab,
stub_entry->h = hash;
stub_entry->branch_type = branch_type;
- if (sym_name == NULL)
- sym_name = "unnamed";
- stub_entry->output_name = (char *)
- bfd_alloc (htab->stub_bfd, sizeof (THUMB2ARM_GLUE_ENTRY_NAME)
- + strlen (sym_name));
- if (stub_entry->output_name == NULL)
+ if (sym_claimed)
+ stub_entry->output_name = sym_name;
+ else
{
- free (stub_name);
- return FALSE;
- }
+ if (sym_name == NULL)
+ sym_name = "unnamed";
+ stub_entry->output_name = (char *)
+ bfd_alloc (htab->stub_bfd, sizeof (THUMB2ARM_GLUE_ENTRY_NAME)
+ + strlen (sym_name));
+ if (stub_entry->output_name == NULL)
+ {
+ free (stub_name);
+ return FALSE;
+ }
- /* For historical reasons, use the existing names for ARM-to-Thumb and
- Thumb-to-ARM stubs. */
- r_type = ELF32_R_TYPE (irela->r_info);
- if ((r_type == (unsigned int) R_ARM_THM_CALL
- || r_type == (unsigned int) R_ARM_THM_JUMP24
- || r_type == (unsigned int) R_ARM_THM_JUMP19)
- && branch_type == ST_BRANCH_TO_ARM)
- sprintf (stub_entry->output_name, THUMB2ARM_GLUE_ENTRY_NAME, sym_name);
- else if ((r_type == (unsigned int) R_ARM_CALL
- || r_type == (unsigned int) R_ARM_JUMP24)
- && branch_type == ST_BRANCH_TO_THUMB)
- sprintf (stub_entry->output_name, ARM2THUMB_GLUE_ENTRY_NAME, sym_name);
- else
- sprintf (stub_entry->output_name, STUB_ENTRY_NAME, sym_name);
+ /* For historical reasons, use the existing names for ARM-to-Thumb and
+ Thumb-to-ARM stubs. */
+ r_type = ELF32_R_TYPE (irela->r_info);
+ if ((r_type == (unsigned int) R_ARM_THM_CALL
+ || r_type == (unsigned int) R_ARM_THM_JUMP24
+ || r_type == (unsigned int) R_ARM_THM_JUMP19)
+ && branch_type == ST_BRANCH_TO_ARM)
+ sprintf (stub_entry->output_name, THUMB2ARM_GLUE_ENTRY_NAME, sym_name);
+ else if ((r_type == (unsigned int) R_ARM_CALL
+ || r_type == (unsigned int) R_ARM_JUMP24)
+ && branch_type == ST_BRANCH_TO_THUMB)
+ sprintf (stub_entry->output_name, ARM2THUMB_GLUE_ENTRY_NAME, sym_name);
+ else
+ sprintf (stub_entry->output_name, STUB_ENTRY_NAME, sym_name);
+ }
*new_stub = TRUE;
return TRUE;
@@ -15853,6 +15880,20 @@ elf32_arm_output_plt_map (struct elf_link_hash_entry
*h, void *inf)
&h->plt, &eh->plt);
}
+/* Bind a veneered symbol to its veneer identified by its hash entry
+ STUB_ENTRY. The veneered location thus loose its symbol. */
+
+static void
+arm_stub_claim_sym (struct elf32_arm_stub_hash_entry *stub_entry)
+{
+ struct elf32_arm_link_hash_entry *hash = stub_entry->h;
+
+ BFD_ASSERT (hash);
+ hash->root.root.u.def.section = stub_entry->stub_sec;
+ hash->root.root.u.def.value = stub_entry->stub_offset;
+ hash->root.size = stub_entry->stub_size;
+}
+
/* Output a single local symbol for a generated stub. */
static bfd_boolean
@@ -15899,24 +15940,30 @@ arm_map_one_stub (struct bfd_hash_entry * gen_entry,
return TRUE;
addr = (bfd_vma) stub_entry->stub_offset;
- stub_name = stub_entry->output_name;
-
template_sequence = stub_entry->stub_template;
- switch (template_sequence[0].type)
+
+ if (arm_stub_sym_claimed (stub_entry->stub_type))
+ arm_stub_claim_sym (stub_entry);
+ else
{
- case ARM_TYPE:
- if (!elf32_arm_output_stub_sym (osi, stub_name, addr, stub_entry-
>stub_size))
- return FALSE;
- break;
- case THUMB16_TYPE:
- case THUMB32_TYPE:
- if (!elf32_arm_output_stub_sym (osi, stub_name, addr | 1,
- stub_entry->stub_size))
- return FALSE;
- break;
- default:
- BFD_FAIL ();
- return 0;
+ stub_name = stub_entry->output_name;
+ switch (template_sequence[0].type)
+ {
+ case ARM_TYPE:
+ if (!elf32_arm_output_stub_sym (osi, stub_name, addr,
+ stub_entry->stub_size))
+ return FALSE;
+ break;
+ case THUMB16_TYPE:
+ case THUMB32_TYPE:
+ if (!elf32_arm_output_stub_sym (osi, stub_name, addr | 1,
+ stub_entry->stub_size))
+ return FALSE;
+ break;
+ default:
+ BFD_FAIL ();
+ return 0;
+ }
}
prev_type = DATA_TYPE;
Best regards,
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH, binutils, ARM 5/11] Allow veneers to claim veneered symbols
2016-05-05 10:08 ` [RFC PATCH, binutils, ARM 5/11] " Thomas Preudhomme
@ 2016-05-09 11:58 ` Nick Clifton
0 siblings, 0 replies; 7+ messages in thread
From: Nick Clifton @ 2016-05-09 11:58 UTC (permalink / raw)
To: Thomas Preudhomme; +Cc: binutils
Hi Thomas,
> *** bfd/ChangeLog ***
> 2016-03-23 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> * elf32-arm.c (enum elf32_arm_stub_type): New max_stub_type
> enumerator.
> (arm_stub_sym_claimed): New function.
> (elf32_arm_create_stub): Use veneered symbol name and section if
> veneer needs to claim its symbol, and keep logic unchanged otherwise.
> (arm_stub_claim_sym): New function.
> (arm_map_one_stub): Call arm_stub_claim_sym if veneer needs to claim
> veneered symbol, otherwise create local symbol as before.
Approved - please apply.
Cheers
Nick
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-05-09 11:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-23 7:58 [RFC PATCH, binutils, ARM 5/9] Allow veneers to claim veneered symbols Thomas Preud'homme
2016-03-29 14:37 ` Thomas Preudhomme
2016-03-30 15:49 ` Nick Clifton
2016-04-04 14:19 ` Thomas Preudhomme
2016-04-05 10:06 ` Nick Clifton
2016-05-05 10:08 ` [RFC PATCH, binutils, ARM 5/11] " Thomas Preudhomme
2016-05-09 11:58 ` Nick Clifton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).