public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [RFC PATCH, binutils, ARM 2/9] Factor our stub creation in ARM backend
@ 2015-12-23  7:55 Thomas Preud'homme
  2016-03-29 14:34 ` [RFC PATCH, binutils, ARM 2/11, ping] " Thomas Preudhomme
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Preud'homme @ 2015-12-23  7:55 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 factors out the code to create a stub in ARM backend to avoid code duplication in subsequent patches adding new code for creating 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 entries is as follows:


*** bfd/ChangeLog ***

2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * elf32-arm.c (elf32_arm_create_stub): New function.
        (elf32_arm_size_stubs): Use elf32_arm_create_stub for stub creation.


diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index 639fe0f125688ccf218b449467c746686841386f..e6afb99512524ac26c0e3842e8dbe293c58bacad 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -5031,6 +5031,103 @@ cortex_a8_erratum_scan (bfd *input_bfd,
   return FALSE;
 }
 
+/* Create or update a stub entry depending on whether the stub can already be
+   found in HTAB.  The stub is identified by:
+   - its type STUB_TYPE
+   - its source branch (note that several can share the same stub) whose
+     section and relocation (if any) are given by SECTION and IRELA
+     respectively
+   - its target symbol whose input section, hash, name, value and branch type
+     are given in SYM_SEC, HASH, SYM_NAME, SYM_VALUE and BRANCH_TYPE
+     respectively
+
+   If found, the value of the stub's target symbol is updated from SYM_VALUE
+   and *NEW_STUB is set to FALSE.  Otherwise, *NEW_STUB is set to
+   TRUE and the stub entry is initialized.
+
+   Returns whether the stub could be successfully created or updated, or FALSE
+   if an error occured.  */
+
+static bfd_boolean
+elf32_arm_create_stub (struct elf32_arm_link_hash_table *htab,
+		       enum elf32_arm_stub_type stub_type, asection *section,
+		       Elf_Internal_Rela *irela, asection *sym_sec,
+		       struct elf32_arm_link_hash_entry *hash, char *sym_name,
+		       bfd_vma sym_value, enum arm_st_branch_type branch_type,
+		       bfd_boolean *new_stub)
+{
+  const asection *id_sec;
+  char *stub_name;
+  struct elf32_arm_stub_hash_entry *stub_entry;
+  unsigned int r_type;
+
+  BFD_ASSERT (stub_type != arm_stub_none);
+  *new_stub = FALSE;
+
+  BFD_ASSERT (irela);
+  BFD_ASSERT (section);
+
+  /* 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;
+
+  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);
+      stub_entry->target_value = sym_value;
+      return TRUE;
+    }
+
+  stub_entry = elf32_arm_add_stub (stub_name, section, htab);
+  if (stub_entry == NULL)
+    {
+      free (stub_name);
+      return FALSE;
+    }
+
+  stub_entry->target_value = sym_value;
+  stub_entry->target_section = sym_sec;
+  stub_entry->stub_type = stub_type;
+  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)
+    {
+      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);
+
+  *new_stub = TRUE;
+  return TRUE;
+}
+
 /* Determine and set the size of the stub section for a final link.
 
    The basic idea here is to examine all the relocations looking for
@@ -5174,14 +5271,11 @@ elf32_arm_size_stubs (bfd *output_bfd,
 		{
 		  unsigned int r_type, r_indx;
 		  enum elf32_arm_stub_type stub_type;
-		  struct elf32_arm_stub_hash_entry *stub_entry;
 		  asection *sym_sec;
 		  bfd_vma sym_value;
 		  bfd_vma destination;
 		  struct elf32_arm_link_hash_entry *hash;
 		  const char *sym_name;
-		  char *stub_name;
-		  const asection *id_sec;
 		  unsigned char st_type;
 		  enum arm_st_branch_type branch_type;
 		  bfd_boolean created_stub = FALSE;
@@ -5364,6 +5458,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
 
 		  do
 		    {
+		      bfd_boolean new_stub;
+
 		      /* Determine what (if any) linker stub is needed.  */
 		      stub_type = arm_type_of_stub (info, section, irela,
 						    st_type, &branch_type,
@@ -5372,74 +5468,21 @@ elf32_arm_size_stubs (bfd *output_bfd,
 		      if (stub_type == arm_stub_none)
 			break;
 
-		      /* 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)
-			goto error_ret_free_internal;
-
 		      /* We've either created a stub for this reloc already,
 			 or we are about to.  */
-		      created_stub = TRUE;
-
-		      stub_entry = arm_stub_hash_lookup
-				     (&htab->stub_hash_table, stub_name,
-				      FALSE, FALSE);
-		      if (stub_entry != NULL)
-			{
-			  /* The proper stub has already been created.  */
-			  free (stub_name);
-			  stub_entry->target_value = sym_value;
-			  break;
-			}
-
-		      stub_entry = elf32_arm_add_stub (stub_name, section,
-						       htab);
-		      if (stub_entry == NULL)
-			{
-			  free (stub_name);
-			  goto error_ret_free_internal;
-			}
+		      created_stub =
+			elf32_arm_create_stub (htab, stub_type, section, irela,
+					       sym_sec, hash,
+					       (char *) sym_name, sym_value,
+					       branch_type, &new_stub);
 
-		      stub_entry->target_value = sym_value;
-		      stub_entry->target_section = sym_sec;
-		      stub_entry->stub_type = stub_type;
-		      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 (!created_stub || !new_stub)
 			{
-			  free (stub_name);
-			  goto error_ret_free_internal;
+			  if (!created_stub)
+			    goto error_ret_free_internal;
 			}
-
-		      /* For historical reasons, use the existing names for
-			 ARM-to-Thumb and Thumb-to-ARM stubs.  */
-		      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);
-
-		      stub_changed = TRUE;
+			stub_changed = TRUE;
 		    }
 		  while (0);


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] 6+ messages in thread

* Re: [RFC PATCH, binutils, ARM 2/11, ping] Factor our stub creation in ARM backend
  2015-12-23  7:55 [RFC PATCH, binutils, ARM 2/9] Factor our stub creation in ARM backend Thomas Preud'homme
@ 2016-03-29 14:34 ` Thomas Preudhomme
  2016-03-30  9:11   ` Christophe Lyon
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Preudhomme @ 2016-03-29 14:34 UTC (permalink / raw)
  To: binutils

Ping?

On Wednesday 23 December 2015 15:55:26 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 factors out the code to create
> a stub in ARM backend to avoid code duplication in subsequent patches
> adding new code for creating 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 entries is as follows:
> 
> 
> *** bfd/ChangeLog ***
> 
> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * elf32-arm.c (elf32_arm_create_stub): New function.
>         (elf32_arm_size_stubs): Use elf32_arm_create_stub for stub creation.
> 
> 
> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
> index
> 639fe0f125688ccf218b449467c746686841386f..e6afb99512524ac26c0e3842e8dbe293c
> 58bacad 100644 --- a/bfd/elf32-arm.c
> +++ b/bfd/elf32-arm.c
> @@ -5031,6 +5031,103 @@ cortex_a8_erratum_scan (bfd *input_bfd,
>    return FALSE;
>  }
> 
> +/* Create or update a stub entry depending on whether the stub can already
> be +   found in HTAB.  The stub is identified by:
> +   - its type STUB_TYPE
> +   - its source branch (note that several can share the same stub) whose
> +     section and relocation (if any) are given by SECTION and IRELA
> +     respectively
> +   - its target symbol whose input section, hash, name, value and branch
> type +     are given in SYM_SEC, HASH, SYM_NAME, SYM_VALUE and BRANCH_TYPE
> +     respectively
> +
> +   If found, the value of the stub's target symbol is updated from
> SYM_VALUE +   and *NEW_STUB is set to FALSE.  Otherwise, *NEW_STUB is set
> to +   TRUE and the stub entry is initialized.
> +
> +   Returns whether the stub could be successfully created or updated, or
> FALSE +   if an error occured.  */
> +
> +static bfd_boolean
> +elf32_arm_create_stub (struct elf32_arm_link_hash_table *htab,
> +		       enum elf32_arm_stub_type stub_type, asection *section,
> +		       Elf_Internal_Rela *irela, asection *sym_sec,
> +		       struct elf32_arm_link_hash_entry *hash, char *sym_name,
> +		       bfd_vma sym_value, enum arm_st_branch_type branch_type,
> +		       bfd_boolean *new_stub)
> +{
> +  const asection *id_sec;
> +  char *stub_name;
> +  struct elf32_arm_stub_hash_entry *stub_entry;
> +  unsigned int r_type;
> +
> +  BFD_ASSERT (stub_type != arm_stub_none);
> +  *new_stub = FALSE;
> +
> +  BFD_ASSERT (irela);
> +  BFD_ASSERT (section);
> +
> +  /* 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;
> +
> +  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);
> +      stub_entry->target_value = sym_value;
> +      return TRUE;
> +    }
> +
> +  stub_entry = elf32_arm_add_stub (stub_name, section, htab);
> +  if (stub_entry == NULL)
> +    {
> +      free (stub_name);
> +      return FALSE;
> +    }
> +
> +  stub_entry->target_value = sym_value;
> +  stub_entry->target_section = sym_sec;
> +  stub_entry->stub_type = stub_type;
> +  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)
> +    {
> +      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);
> +
> +  *new_stub = TRUE;
> +  return TRUE;
> +}
> +
>  /* Determine and set the size of the stub section for a final link.
> 
>     The basic idea here is to examine all the relocations looking for
> @@ -5174,14 +5271,11 @@ elf32_arm_size_stubs (bfd *output_bfd,
>  		{
>  		  unsigned int r_type, r_indx;
>  		  enum elf32_arm_stub_type stub_type;
> -		  struct elf32_arm_stub_hash_entry *stub_entry;
>  		  asection *sym_sec;
>  		  bfd_vma sym_value;
>  		  bfd_vma destination;
>  		  struct elf32_arm_link_hash_entry *hash;
>  		  const char *sym_name;
> -		  char *stub_name;
> -		  const asection *id_sec;
>  		  unsigned char st_type;
>  		  enum arm_st_branch_type branch_type;
>  		  bfd_boolean created_stub = FALSE;
> @@ -5364,6 +5458,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
> 
>  		  do
>  		    {
> +		      bfd_boolean new_stub;
> +
>  		      /* Determine what (if any) linker stub is needed.  */
>  		      stub_type = arm_type_of_stub (info, section, irela,
>  						    st_type, &branch_type,
> @@ -5372,74 +5468,21 @@ elf32_arm_size_stubs (bfd *output_bfd,
>  		      if (stub_type == arm_stub_none)
>  			break;
> 
> -		      /* 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)
> -			goto error_ret_free_internal;
> -
>  		      /* We've either created a stub for this reloc already,
>  			 or we are about to.  */
> -		      created_stub = TRUE;
> -
> -		      stub_entry = arm_stub_hash_lookup
> -				     (&htab->stub_hash_table, stub_name,
> -				      FALSE, FALSE);
> -		      if (stub_entry != NULL)
> -			{
> -			  /* The proper stub has already been created.  */
> -			  free (stub_name);
> -			  stub_entry->target_value = sym_value;
> -			  break;
> -			}
> -
> -		      stub_entry = elf32_arm_add_stub (stub_name, section,
> -						       htab);
> -		      if (stub_entry == NULL)
> -			{
> -			  free (stub_name);
> -			  goto error_ret_free_internal;
> -			}
> +		      created_stub =
> +			elf32_arm_create_stub (htab, stub_type, section, irela,
> +					       sym_sec, hash,
> +					       (char *) sym_name, sym_value,
> +					       branch_type, &new_stub);
> 
> -		      stub_entry->target_value = sym_value;
> -		      stub_entry->target_section = sym_sec;
> -		      stub_entry->stub_type = stub_type;
> -		      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 (!created_stub || !new_stub)
>  			{
> -			  free (stub_name);
> -			  goto error_ret_free_internal;
> +			  if (!created_stub)
> +			    goto error_ret_free_internal;
>  			}
> -
> -		      /* For historical reasons, use the existing names for
> -			 ARM-to-Thumb and Thumb-to-ARM stubs.  */
> -		      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);
> -
> -		      stub_changed = TRUE;
> +			stub_changed = TRUE;
>  		    }
>  		  while (0);
> 
> 
> 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] 6+ messages in thread

* Re: [RFC PATCH, binutils, ARM 2/11, ping] Factor our stub creation in ARM backend
  2016-03-29 14:34 ` [RFC PATCH, binutils, ARM 2/11, ping] " Thomas Preudhomme
@ 2016-03-30  9:11   ` Christophe Lyon
  2016-04-04 14:01     ` Thomas Preudhomme
  2016-05-05 10:04     ` Thomas Preudhomme
  0 siblings, 2 replies; 6+ messages in thread
From: Christophe Lyon @ 2016-03-30  9:11 UTC (permalink / raw)
  To: Thomas Preudhomme; +Cc: binutils

On 29 March 2016 at 16:33, Thomas Preudhomme
<thomas.preudhomme@foss.arm.com> wrote:
> Ping?
>
> On Wednesday 23 December 2015 15:55:26 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 factors out the code to create
>> a stub in ARM backend to avoid code duplication in subsequent patches
>> adding new code for creating 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 entries is as follows:
>>
>>
>> *** bfd/ChangeLog ***
>>
>> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>         * elf32-arm.c (elf32_arm_create_stub): New function.
>>         (elf32_arm_size_stubs): Use elf32_arm_create_stub for stub creation.
>>
>>
>> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
>> index
>> 639fe0f125688ccf218b449467c746686841386f..e6afb99512524ac26c0e3842e8dbe293c
>> 58bacad 100644 --- a/bfd/elf32-arm.c
>> +++ b/bfd/elf32-arm.c
>> @@ -5031,6 +5031,103 @@ cortex_a8_erratum_scan (bfd *input_bfd,
>>    return FALSE;
>>  }
>>
>> +/* Create or update a stub entry depending on whether the stub can already
>> be +   found in HTAB.  The stub is identified by:
>> +   - its type STUB_TYPE
>> +   - its source branch (note that several can share the same stub) whose
>> +     section and relocation (if any) are given by SECTION and IRELA
>> +     respectively
>> +   - its target symbol whose input section, hash, name, value and branch
>> type +     are given in SYM_SEC, HASH, SYM_NAME, SYM_VALUE and BRANCH_TYPE
>> +     respectively
>> +
>> +   If found, the value of the stub's target symbol is updated from
>> SYM_VALUE +   and *NEW_STUB is set to FALSE.  Otherwise, *NEW_STUB is set
>> to +   TRUE and the stub entry is initialized.
>> +
>> +   Returns whether the stub could be successfully created or updated, or
>> FALSE +   if an error occured.  */
>> +
>> +static bfd_boolean
>> +elf32_arm_create_stub (struct elf32_arm_link_hash_table *htab,
>> +                    enum elf32_arm_stub_type stub_type, asection *section,
>> +                    Elf_Internal_Rela *irela, asection *sym_sec,
>> +                    struct elf32_arm_link_hash_entry *hash, char *sym_name,
>> +                    bfd_vma sym_value, enum arm_st_branch_type branch_type,
>> +                    bfd_boolean *new_stub)
>> +{
>> +  const asection *id_sec;
>> +  char *stub_name;
>> +  struct elf32_arm_stub_hash_entry *stub_entry;
>> +  unsigned int r_type;
>> +
>> +  BFD_ASSERT (stub_type != arm_stub_none);
>> +  *new_stub = FALSE;
>> +
>> +  BFD_ASSERT (irela);
>> +  BFD_ASSERT (section);
>> +
>> +  /* 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;
>> +
>> +  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);
>> +      stub_entry->target_value = sym_value;
>> +      return TRUE;
>> +    }
>> +
>> +  stub_entry = elf32_arm_add_stub (stub_name, section, htab);
>> +  if (stub_entry == NULL)
>> +    {
>> +      free (stub_name);
>> +      return FALSE;
>> +    }
>> +
>> +  stub_entry->target_value = sym_value;
>> +  stub_entry->target_section = sym_sec;
>> +  stub_entry->stub_type = stub_type;
>> +  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)
>> +    {
>> +      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);
>> +
>> +  *new_stub = TRUE;
>> +  return TRUE;
>> +}
>> +
>>  /* Determine and set the size of the stub section for a final link.
>>
>>     The basic idea here is to examine all the relocations looking for
>> @@ -5174,14 +5271,11 @@ elf32_arm_size_stubs (bfd *output_bfd,
>>               {
>>                 unsigned int r_type, r_indx;
>>                 enum elf32_arm_stub_type stub_type;
>> -               struct elf32_arm_stub_hash_entry *stub_entry;
>>                 asection *sym_sec;
>>                 bfd_vma sym_value;
>>                 bfd_vma destination;
>>                 struct elf32_arm_link_hash_entry *hash;
>>                 const char *sym_name;
>> -               char *stub_name;
>> -               const asection *id_sec;
>>                 unsigned char st_type;
>>                 enum arm_st_branch_type branch_type;
>>                 bfd_boolean created_stub = FALSE;
>> @@ -5364,6 +5458,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
>>
>>                 do
>>                   {
>> +                   bfd_boolean new_stub;
>> +
>>                     /* Determine what (if any) linker stub is needed.  */
>>                     stub_type = arm_type_of_stub (info, section, irela,
>>                                                   st_type, &branch_type,
>> @@ -5372,74 +5468,21 @@ elf32_arm_size_stubs (bfd *output_bfd,
>>                     if (stub_type == arm_stub_none)
>>                       break;
>>
>> -                   /* 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)
>> -                     goto error_ret_free_internal;
>> -
>>                     /* We've either created a stub for this reloc already,
>>                        or we are about to.  */
>> -                   created_stub = TRUE;
>> -
>> -                   stub_entry = arm_stub_hash_lookup
>> -                                  (&htab->stub_hash_table, stub_name,
>> -                                   FALSE, FALSE);
>> -                   if (stub_entry != NULL)
>> -                     {
>> -                       /* The proper stub has already been created.  */
>> -                       free (stub_name);
>> -                       stub_entry->target_value = sym_value;
>> -                       break;
>> -                     }
>> -
>> -                   stub_entry = elf32_arm_add_stub (stub_name, section,
>> -                                                    htab);
>> -                   if (stub_entry == NULL)
>> -                     {
>> -                       free (stub_name);
>> -                       goto error_ret_free_internal;
>> -                     }
>> +                   created_stub =
>> +                     elf32_arm_create_stub (htab, stub_type, section, irela,
>> +                                            sym_sec, hash,
>> +                                            (char *) sym_name, sym_value,
>> +                                            branch_type, &new_stub);
>>
>> -                   stub_entry->target_value = sym_value;
>> -                   stub_entry->target_section = sym_sec;
>> -                   stub_entry->stub_type = stub_type;
>> -                   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 (!created_stub || !new_stub)
I'm not sure to understand why you check both?

>>                       {
>> -                       free (stub_name);
>> -                       goto error_ret_free_internal;
>> +                       if (!created_stub)
>> +                         goto error_ret_free_internal;
>>                       }
>> -
>> -                   /* For historical reasons, use the existing names for
>> -                      ARM-to-Thumb and Thumb-to-ARM stubs.  */
>> -                   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);
>> -
>> -                   stub_changed = TRUE;
>> +                     stub_changed = TRUE;
>>                   }
>>                 while (0);
>>
>>
>> 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] 6+ messages in thread

* Re: [RFC PATCH, binutils, ARM 2/11, ping] Factor our stub creation in ARM backend
  2016-03-30  9:11   ` Christophe Lyon
@ 2016-04-04 14:01     ` Thomas Preudhomme
  2016-05-05 10:04     ` Thomas Preudhomme
  1 sibling, 0 replies; 6+ messages in thread
From: Thomas Preudhomme @ 2016-04-04 14:01 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: binutils

On Wednesday 30 March 2016 11:11:01 Christophe Lyon wrote:
> On 29 March 2016 at 16:33, Thomas Preudhomme
> 
> <thomas.preudhomme@foss.arm.com> wrote:
> > Ping?
> > 
> > On Wednesday 23 December 2015 15:55:26 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 factors out the code to
> >> create
> >> a stub in ARM backend to avoid code duplication in subsequent patches
> >> adding new code for creating 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 entries is as follows:
> >> 
> >> 
> >> *** bfd/ChangeLog ***
> >> 
> >> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> >> 
> >>         * elf32-arm.c (elf32_arm_create_stub): New function.
> >>         (elf32_arm_size_stubs): Use elf32_arm_create_stub for stub
> >>         creation.
> >> 
> >> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
> >> index
> >> 639fe0f125688ccf218b449467c746686841386f..e6afb99512524ac26c0e3842e8dbe29
> >> 3c
> >> 58bacad 100644 --- a/bfd/elf32-arm.c
> >> +++ b/bfd/elf32-arm.c
> >> @@ -5031,6 +5031,103 @@ cortex_a8_erratum_scan (bfd *input_bfd,
> >> 
> >>    return FALSE;
> >>  
> >>  }
> >> 
> >> +/* Create or update a stub entry depending on whether the stub can
> >> already
> >> be +   found in HTAB.  The stub is identified by:
> >> +   - its type STUB_TYPE
> >> +   - its source branch (note that several can share the same stub) whose
> >> +     section and relocation (if any) are given by SECTION and IRELA
> >> +     respectively
> >> +   - its target symbol whose input section, hash, name, value and branch
> >> type +     are given in SYM_SEC, HASH, SYM_NAME, SYM_VALUE and
> >> BRANCH_TYPE
> >> +     respectively
> >> +
> >> +   If found, the value of the stub's target symbol is updated from
> >> SYM_VALUE +   and *NEW_STUB is set to FALSE.  Otherwise, *NEW_STUB is set
> >> to +   TRUE and the stub entry is initialized.
> >> +
> >> +   Returns whether the stub could be successfully created or updated, or
> >> FALSE +   if an error occured.  */
> >> +
> >> +static bfd_boolean
> >> +elf32_arm_create_stub (struct elf32_arm_link_hash_table *htab,
> >> +                    enum elf32_arm_stub_type stub_type, asection
> >> *section,
> >> +                    Elf_Internal_Rela *irela, asection *sym_sec,
> >> +                    struct elf32_arm_link_hash_entry *hash, char
> >> *sym_name, +                    bfd_vma sym_value, enum
> >> arm_st_branch_type branch_type, +                    bfd_boolean
> >> *new_stub)
> >> +{
> >> +  const asection *id_sec;
> >> +  char *stub_name;
> >> +  struct elf32_arm_stub_hash_entry *stub_entry;
> >> +  unsigned int r_type;
> >> +
> >> +  BFD_ASSERT (stub_type != arm_stub_none);
> >> +  *new_stub = FALSE;
> >> +
> >> +  BFD_ASSERT (irela);
> >> +  BFD_ASSERT (section);
> >> +
> >> +  /* 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;
> >> +
> >> +  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);
> >> +      stub_entry->target_value = sym_value;
> >> +      return TRUE;
> >> +    }
> >> +
> >> +  stub_entry = elf32_arm_add_stub (stub_name, section, htab);
> >> +  if (stub_entry == NULL)
> >> +    {
> >> +      free (stub_name);
> >> +      return FALSE;
> >> +    }
> >> +
> >> +  stub_entry->target_value = sym_value;
> >> +  stub_entry->target_section = sym_sec;
> >> +  stub_entry->stub_type = stub_type;
> >> +  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)
> >> +    {
> >> +      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);
> >> +
> >> +  *new_stub = TRUE;
> >> +  return TRUE;
> >> +}
> >> +
> >> 
> >>  /* Determine and set the size of the stub section for a final link.
> >>  
> >>     The basic idea here is to examine all the relocations looking for
> >> 
> >> @@ -5174,14 +5271,11 @@ elf32_arm_size_stubs (bfd *output_bfd,
> >> 
> >>               {
> >>               
> >>                 unsigned int r_type, r_indx;
> >>                 enum elf32_arm_stub_type stub_type;
> >> 
> >> -               struct elf32_arm_stub_hash_entry *stub_entry;
> >> 
> >>                 asection *sym_sec;
> >>                 bfd_vma sym_value;
> >>                 bfd_vma destination;
> >>                 struct elf32_arm_link_hash_entry *hash;
> >>                 const char *sym_name;
> >> 
> >> -               char *stub_name;
> >> -               const asection *id_sec;
> >> 
> >>                 unsigned char st_type;
> >>                 enum arm_st_branch_type branch_type;
> >>                 bfd_boolean created_stub = FALSE;
> >> 
> >> @@ -5364,6 +5458,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
> >> 
> >>                 do
> >>                 
> >>                   {
> >> 
> >> +                   bfd_boolean new_stub;
> >> +
> >> 
> >>                     /* Determine what (if any) linker stub is needed.  */
> >>                     stub_type = arm_type_of_stub (info, section, irela,
> >>                     
> >>                                                   st_type, &branch_type,
> >> 
> >> @@ -5372,74 +5468,21 @@ elf32_arm_size_stubs (bfd *output_bfd,
> >> 
> >>                     if (stub_type == arm_stub_none)
> >>                     
> >>                       break;
> >> 
> >> -                   /* 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)
> >> -                     goto error_ret_free_internal;
> >> -
> >> 
> >>                     /* We've either created a stub for this reloc
> >>                     already,
> >>                     
> >>                        or we are about to.  */
> >> 
> >> -                   created_stub = TRUE;
> >> -
> >> -                   stub_entry = arm_stub_hash_lookup
> >> -                                  (&htab->stub_hash_table, stub_name,
> >> -                                   FALSE, FALSE);
> >> -                   if (stub_entry != NULL)
> >> -                     {
> >> -                       /* The proper stub has already been created.  */
> >> -                       free (stub_name);
> >> -                       stub_entry->target_value = sym_value;
> >> -                       break;
> >> -                     }

Please take note of this lookup + conditional block.

> >> -
> >> -                   stub_entry = elf32_arm_add_stub (stub_name, section,
> >> -                                                    htab);
> >> -                   if (stub_entry == NULL)
> >> -                     {
> >> -                       free (stub_name);
> >> -                       goto error_ret_free_internal;
> >> -                     }
> >> +                   created_stub =
> >> +                     elf32_arm_create_stub (htab, stub_type, section,
> >> irela, +                                            sym_sec, hash,
> >> +                                            (char *) sym_name,
> >> sym_value,
> >> +                                            branch_type, &new_stub);
> >> 
> >> -                   stub_entry->target_value = sym_value;
> >> -                   stub_entry->target_section = sym_sec;
> >> -                   stub_entry->stub_type = stub_type;
> >> -                   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 (!created_stub || !new_stub)
> 
> I'm not sure to understand why you check both?

Good catch! That's a result of the iterative process that for the create_stub 
function. It should be a break for the !new_stub case, as in the code I 
hilighted above.

I'll respin the patch.

Best regards,

Thomas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH, binutils, ARM 2/11, ping] Factor our stub creation in ARM backend
  2016-03-30  9:11   ` Christophe Lyon
  2016-04-04 14:01     ` Thomas Preudhomme
@ 2016-05-05 10:04     ` Thomas Preudhomme
  2016-05-06 16:47       ` Nick Clifton
  1 sibling, 1 reply; 6+ messages in thread
From: Thomas Preudhomme @ 2016-05-05 10:04 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: binutils

On Monday 04 April 2016 15:01:00 Thomas Preudhomme wrote:
> > >> -                   created_stub = TRUE;
> > >> -
> > >> -                   stub_entry = arm_stub_hash_lookup
> > >> -                                  (&htab->stub_hash_table, stub_name,
> > >> -                                   FALSE, FALSE);
> > >> -                   if (stub_entry != NULL)
> > >> -                     {
> > >> -                       /* The proper stub has already been created. 
> > >> */
> > >> -                       free (stub_name);
> > >> -                       stub_entry->target_value = sym_value;
> > >> -                       break;
> > >> -                     }
> 
> Please take note of this lookup + conditional block.
> 
> > >> -
> > >> -                   stub_entry = elf32_arm_add_stub (stub_name,
> > >> section,
> > >> -                                                    htab);
> > >> -                   if (stub_entry == NULL)
> > >> -                     {
> > >> -                       free (stub_name);
> > >> -                       goto error_ret_free_internal;
> > >> -                     }
> > >> +                   created_stub =
> > >> +                     elf32_arm_create_stub (htab, stub_type, section,
> > >> irela, +                                            sym_sec, hash,
> > >> +                                            (char *) sym_name,
> > >> sym_value,
> > >> +                                            branch_type, &new_stub);
> > >> 
> > >> -                   stub_entry->target_value = sym_value;
> > >> -                   stub_entry->target_section = sym_sec;
> > >> -                   stub_entry->stub_type = stub_type;
> > >> -                   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 (!created_stub || !new_stub)
> > 
> > I'm not sure to understand why you check both?
> 
> Good catch! That's a result of the iterative process that for the
> create_stub function. It should be a break for the !new_stub case, as in
> the code I hilighted above.

Please find below an updated patch addressing this issue.

ChangeLog entry is unchanged:

*** bfd/ChangeLog ***

2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * elf32-arm.c (elf32_arm_create_stub): New function.
        (elf32_arm_size_stubs): Use elf32_arm_create_stub for stub creation.


diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index 
2f9edee2aff6d944d5d593c350a54ec57e152b10..b9e0a8b9276e1e229c3229f5e8ba90af7d9dd115 
100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -5107,6 +5107,103 @@ cortex_a8_erratum_scan (bfd *input_bfd,
   return FALSE;
 }
 
+/* Create or update a stub entry depending on whether the stub can already be
+   found in HTAB.  The stub is identified by:
+   - its type STUB_TYPE
+   - its source branch (note that several can share the same stub) whose
+     section and relocation (if any) are given by SECTION and IRELA
+     respectively
+   - its target symbol whose input section, hash, name, value and branch type
+     are given in SYM_SEC, HASH, SYM_NAME, SYM_VALUE and BRANCH_TYPE
+     respectively
+
+   If found, the value of the stub's target symbol is updated from SYM_VALUE
+   and *NEW_STUB is set to FALSE.  Otherwise, *NEW_STUB is set to
+   TRUE and the stub entry is initialized.
+
+   Returns whether the stub could be successfully created or updated, or 
FALSE
+   if an error occured.  */
+
+static bfd_boolean
+elf32_arm_create_stub (struct elf32_arm_link_hash_table *htab,
+		       enum elf32_arm_stub_type stub_type, asection *section,
+		       Elf_Internal_Rela *irela, asection *sym_sec,
+		       struct elf32_arm_link_hash_entry *hash, char *sym_name,
+		       bfd_vma sym_value, enum arm_st_branch_type branch_type,
+		       bfd_boolean *new_stub)
+{
+  const asection *id_sec;
+  char *stub_name;
+  struct elf32_arm_stub_hash_entry *stub_entry;
+  unsigned int r_type;
+
+  BFD_ASSERT (stub_type != arm_stub_none);
+  *new_stub = FALSE;
+
+  BFD_ASSERT (irela);
+  BFD_ASSERT (section);
+
+  /* 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;
+
+  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);
+      stub_entry->target_value = sym_value;
+      return TRUE;
+    }
+
+  stub_entry = elf32_arm_add_stub (stub_name, section, htab);
+  if (stub_entry == NULL)
+    {
+      free (stub_name);
+      return FALSE;
+    }
+
+  stub_entry->target_value = sym_value;
+  stub_entry->target_section = sym_sec;
+  stub_entry->stub_type = stub_type;
+  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)
+    {
+      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);
+
+  *new_stub = TRUE;
+  return TRUE;
+}
+
 /* Determine and set the size of the stub section for a final link.
 
    The basic idea here is to examine all the relocations looking for
@@ -5250,14 +5347,11 @@ elf32_arm_size_stubs (bfd *output_bfd,
 		{
 		  unsigned int r_type, r_indx;
 		  enum elf32_arm_stub_type stub_type;
-		  struct elf32_arm_stub_hash_entry *stub_entry;
 		  asection *sym_sec;
 		  bfd_vma sym_value;
 		  bfd_vma destination;
 		  struct elf32_arm_link_hash_entry *hash;
 		  const char *sym_name;
-		  char *stub_name;
-		  const asection *id_sec;
 		  unsigned char st_type;
 		  enum arm_st_branch_type branch_type;
 		  bfd_boolean created_stub = FALSE;
@@ -5440,6 +5534,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
 
 		  do
 		    {
+		      bfd_boolean new_stub;
+
 		      /* Determine what (if any) linker stub is needed.  */
 		      stub_type = arm_type_of_stub (info, section, irela,
 						    st_type, &branch_type,
@@ -5448,74 +5544,20 @@ elf32_arm_size_stubs (bfd *output_bfd,
 		      if (stub_type == arm_stub_none)
 			break;
 
-		      /* 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)
-			goto error_ret_free_internal;
-
 		      /* We've either created a stub for this reloc already,
 			 or we are about to.  */
-		      created_stub = TRUE;
-
-		      stub_entry = arm_stub_hash_lookup
-				     (&htab->stub_hash_table, stub_name,
-				      FALSE, FALSE);
-		      if (stub_entry != NULL)
-			{
-			  /* The proper stub has already been created.  */
-			  free (stub_name);
-			  stub_entry->target_value = sym_value;
-			  break;
-			}
+		      created_stub =
+			elf32_arm_create_stub (htab, stub_type, section, irela,
+					       sym_sec, hash,
+					       (char *) sym_name, sym_value,
+					       branch_type, &new_stub);
 
-		      stub_entry = elf32_arm_add_stub (stub_name, section,
-						       htab);
-		      if (stub_entry == NULL)
-			{
-			  free (stub_name);
-			  goto error_ret_free_internal;
-			}
-
-		      stub_entry->target_value = sym_value;
-		      stub_entry->target_section = sym_sec;
-		      stub_entry->stub_type = stub_type;
-		      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)
-			{
-			  free (stub_name);
-			  goto error_ret_free_internal;
-			}
-
-		      /* For historical reasons, use the existing names for
-			 ARM-to-Thumb and Thumb-to-ARM stubs.  */
-		      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);
+		      if (!created_stub)
+			goto error_ret_free_internal;
+		      else if (!new_stub)
+			break;
 		      else
-			sprintf (stub_entry->output_name, STUB_ENTRY_NAME,
-				 sym_name);
-
-		      stub_changed = TRUE;
+			stub_changed = TRUE;
 		    }
 		  while (0);
 


Best regards,

Thomas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH, binutils, ARM 2/11, ping] Factor our stub creation in ARM backend
  2016-05-05 10:04     ` Thomas Preudhomme
@ 2016-05-06 16:47       ` Nick Clifton
  0 siblings, 0 replies; 6+ messages in thread
From: Nick Clifton @ 2016-05-06 16:47 UTC (permalink / raw)
  To: Thomas Preudhomme, Christophe Lyon; +Cc: binutils

Hi Thomas,
 
> *** bfd/ChangeLog ***
> 
> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * elf32-arm.c (elf32_arm_create_stub): New function.
>         (elf32_arm_size_stubs): Use elf32_arm_create_stub for stub creation.

Approved, please apply.

Cheers
  Nick

 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-05-06 16:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-23  7:55 [RFC PATCH, binutils, ARM 2/9] Factor our stub creation in ARM backend Thomas Preud'homme
2016-03-29 14:34 ` [RFC PATCH, binutils, ARM 2/11, ping] " Thomas Preudhomme
2016-03-30  9:11   ` Christophe Lyon
2016-04-04 14:01     ` Thomas Preudhomme
2016-05-05 10:04     ` Thomas Preudhomme
2016-05-06 16:47       ` 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).