Re: [PATCH, binutils, ARM 6/11, ping1] Support for dedicated output section for some veneer types

[Thomas Preudhomme <thomas dot preudhomme at foss dot arm dot com>]

[CCing Richard Earnshaw and Nick Clifton who reviewed previous ARMv8-M 
patches]

Ping?

On Thursday 05 May 2016 11:26:31 Thomas Preudhomme wrote:
> On Tuesday 29 March 2016 15:39:25 Thomas Preudhomme wrote:
> > Hi,
> > 
> > This patch is part of a patch series to add support for ARMv8-M Security
> > Extensions[1] to GNU ld. This specific patch adds support for having
> > veneers of a given type in a dedicated output section.
> > 
> > ARM v8-M Security Extensions require [3] secure gateway veneers to be
> > generated for (secure) entry function in order for code to transition from
> > non-secure state to secure state when calling these entry function. One
> > requirement for these veneers [4] is to be able to keep their addresses
> > when the secure code is relinked in order to avoid relinking the non
> > secure code with the secure code. This patch adds support for generating
> > these veneers in a dedicated output section and expect the user to
> > specify the address of that section (either via --section-start parameter
> > or in the linker script). This ensure that they are placed in a memory
> > area with sufficient space for more veneers to be added. Ensuring that
> > existing veneers keep their addresses when the secure executable is
> > relinked is handled by a subsequent patch of this patch series.
> > 
> > 
> > [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
> > [3] See section 3.4.3 and requirement 44 of ARM-ECM-0359818 [2]
> > [4] requirement 14 and following comment of ARM-ECM-0359818 [2]
> 
> Please find an updated version without the switch case for the
> arm_dedicated_stub_* functions.
> 
> ChangeLog entries are unchanged:
> 
> *** bfd/ChangeLog ***
> 
> 2016-02-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * bfd-in.h (bfd_elf32_arm_keep_private_stub_output_sections):
> Declare bfd hook.
>         * bfd-in2.h: Regenerate.
>         * elf32-arm.c (arm_dedicated_stub_output_section_required): New
>         function.
>         (arm_dedicated_stub_output_section_required_alignment): Likewise.
>         (arm_dedicated_stub_output_section_name): Likewise.
>         (arm_dedicated_stub_input_section_ptr): Likewise.
>         (elf32_arm_create_or_find_stub_sec): Add stub type parameter and
>         function description comment. Add support for dedicated output stub
>         section to given stub types.
>         (elf32_arm_add_stub): Add a stub type parameter and pass it down to
>         elf32_arm_create_or_find_stub_sec.
>         (elf32_arm_create_stub): Pass stub type down to elf32_arm_add_stub.
>         (elf32_arm_size_stubs): Pass stub type when calling
>         elf32_arm_create_or_find_stub_sec for Cortex-A8 erratum veneers.
>         (bfd_elf32_arm_keep_private_stub_output_sections): New function.
> 
> *** ld/ChangeLog ***
> 
> 2016-01-04  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * emultempl/armelf.em (arm_elf_before_allocation): Call
>         bfd_elf32_arm_keep_private_stub_output_sections before generic
>         before_allocation function.
> 
> 
> diff --git a/bfd/bfd-in.h b/bfd/bfd-in.h
> index
> 571a63672bfda3b4310adf791fb28cf23ae09ae0..c66ce3bba1ad1d1ce670f7404e465834cb
> f43fb1 100644
> --- a/bfd/bfd-in.h
> +++ b/bfd/bfd-in.h
> @@ -903,6 +903,9 @@ extern bfd_boolean
> bfd_elf32_arm_get_bfd_for_interworking extern bfd_boolean
> bfd_elf32_arm_add_glue_sections_to_bfd
>    (bfd *, struct bfd_link_info *);
> 
> +extern void bfd_elf32_arm_keep_private_stub_output_sections
> +  (struct bfd_link_info *);
> +
>  /* ELF ARM mapping symbol support.  */
>  #define BFD_ARM_SPECIAL_SYM_TYPE_MAP	(1 << 0)
>  #define BFD_ARM_SPECIAL_SYM_TYPE_TAG	(1 << 1)
> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> index
> 294da5e1c163bd443c73a99778880a986a08566b..afaca4ef19e49cf9899952a84a90c60fcd
> 3fc8dd 100644
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -910,6 +910,9 @@ extern bfd_boolean
> bfd_elf32_arm_get_bfd_for_interworking extern bfd_boolean
> bfd_elf32_arm_add_glue_sections_to_bfd
>    (bfd *, struct bfd_link_info *);
> 
> +extern void bfd_elf32_arm_keep_private_stub_output_sections
> +  (struct bfd_link_info *);
> +
>  /* ELF ARM mapping symbol support.  */
>  #define BFD_ARM_SPECIAL_SYM_TYPE_MAP	(1 << 0)
>  #define BFD_ARM_SPECIAL_SYM_TYPE_TAG	(1 << 1)
> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
> index
> 02977358ac69deab379f2d826c44bfc5efa49e1a..ab878b20905c907b3496317c7ed1c9c698
> d850c9 100644
> --- a/bfd/elf32-arm.c
> +++ b/bfd/elf32-arm.c
> @@ -4136,68 +4136,155 @@ elf32_arm_get_stub_entry (const asection
> *input_section,
>    return stub_entry;
>  }
> 
> -/* Find or create a stub section.  Returns a pointer to the stub section,
> and -   the section to which the stub section will be attached (in
> *LINK_SEC_P). +/* Whether veneers of type STUB_TYPE require to be in a
> dedicated output +   section.  */
> +
> +static bfd_boolean
> +arm_dedicated_stub_output_section_required (enum elf32_arm_stub_type
> stub_type)
> +{
> +  if (stub_type >= max_stub_type)
> +    abort ();  /* Should be unreachable.  */
> +
> +  return FALSE;
> +}
> +
> +/* Required alignment (as a power of 2) for the dedicated section holding
> +   veneers of type STUB_TYPE, or 0 if veneers of this type are interspersed
> +   with input sections.  */
> +
> +static int
> +arm_dedicated_stub_output_section_required_alignment
> +  (enum elf32_arm_stub_type stub_type)
> +{
> +  if (stub_type >= max_stub_type)
> +    abort ();  /* Should be unreachable.  */
> +
> +  BFD_ASSERT (!arm_dedicated_stub_output_section_required (stub_type));
> +  return 0;
> +}
> +
> +/* Name of the dedicated output section to put veneers of type STUB_TYPE,
> or +   NULL if veneers of this type are interspersed with input sections. 
> */ +
> +static const char *
> +arm_dedicated_stub_output_section_name (enum elf32_arm_stub_type stub_type)
> +{
> +  if (stub_type >= max_stub_type)
> +    abort ();  /* Should be unreachable.  */
> +
> +  BFD_ASSERT (!arm_dedicated_stub_output_section_required (stub_type));
> +  return NULL;
> +}
> +
> +/* If veneers of type STUB_TYPE should go in a dedicated output section,
> +   returns the address of the hash table field in HTAB holding a pointer to
> the
> +   corresponding input section.  Otherwise, returns NULL.  */
> +
> +static asection **
> +arm_dedicated_stub_input_section_ptr
> +  (struct elf32_arm_link_hash_table *htab ATTRIBUTE_UNUSED,
> +   enum elf32_arm_stub_type stub_type)
> +{
> +  if (stub_type >= max_stub_type)
> +    abort ();  /* Should be unreachable.  */
> +
> +  BFD_ASSERT (!arm_dedicated_stub_output_section_required (stub_type));
> +  return NULL;
> +}
> +
> +/* Find or create a stub section to contain a stub of type STUB_TYPE.
> SECTION
> +   is the section that branch into veneer and can be NULL if stub should go
> in
> +   a dedicated output section.  Returns a pointer to the stub section, and
> the
> +   section to which the stub section will be attached (in *LINK_SEC_P).
>     LINK_SEC_P may be NULL.  */
> 
>  static asection *
>  elf32_arm_create_or_find_stub_sec (asection **link_sec_p, asection
> *section, -				   struct elf32_arm_link_hash_table *htab)
> +				   struct elf32_arm_link_hash_table *htab,
> +				   enum elf32_arm_stub_type stub_type)
>  {
> -  asection *link_sec;
> -  asection *stub_sec;
> -  asection *out_sec;
> -
> -  link_sec = htab->stub_group[section->id].link_sec;
> -  BFD_ASSERT (link_sec != NULL);
> -  stub_sec = htab->stub_group[section->id].stub_sec;
> +  asection *link_sec, *out_sec, **stub_sec_p;
> +  const char *stub_sec_prefix;
> +  bfd_boolean dedicated_output_section =
> +    arm_dedicated_stub_output_section_required (stub_type);
> +  int align;
> 
> -  if (stub_sec == NULL)
> +  if (dedicated_output_section)
>      {
> -      stub_sec = htab->stub_group[link_sec->id].stub_sec;
> -      if (stub_sec == NULL)
> +      bfd *output_bfd = htab->obfd;
> +      const char *out_sec_name =
> +	arm_dedicated_stub_output_section_name (stub_type);
> +      link_sec = NULL;
> +      stub_sec_p = arm_dedicated_stub_input_section_ptr (htab, stub_type);
> +      stub_sec_prefix = out_sec_name;
> +      align = arm_dedicated_stub_output_section_required_alignment
> (stub_type);
> +      out_sec = bfd_get_section_by_name (output_bfd, out_sec_name);
> +      if (out_sec == NULL)
>  	{
> -	  size_t namelen;
> -	  bfd_size_type len;
> -	  char *s_name;
> -
> -	  namelen = strlen (link_sec->name);
> -	  len = namelen + sizeof (STUB_SUFFIX);
> -	  s_name = (char *) bfd_alloc (htab->stub_bfd, len);
> -	  if (s_name == NULL)
> -	    return NULL;
> -
> -	  memcpy (s_name, link_sec->name, namelen);
> -	  memcpy (s_name + namelen, STUB_SUFFIX, sizeof (STUB_SUFFIX));
> -	  out_sec = link_sec->output_section;
> -	  stub_sec = (*htab->add_stub_section) (s_name, out_sec, link_sec,
> -						htab->nacl_p ? 4 : 3);
> -	  if (stub_sec == NULL)
> -	    return NULL;
> -	  htab->stub_group[link_sec->id].stub_sec = stub_sec;
> +	  (*_bfd_error_handler) (_("No address assigned to the veneers output "
> +				   "section %s"), out_sec_name);
> +	  return NULL;
>  	}
> -      htab->stub_group[section->id].stub_sec = stub_sec;
>      }
> +  else
> +    {
> +      link_sec = htab->stub_group[section->id].link_sec;
> +      BFD_ASSERT (link_sec != NULL);
> +      stub_sec_p = &htab->stub_group[section->id].stub_sec;
> +      if (*stub_sec_p == NULL)
> +	stub_sec_p = &htab->stub_group[link_sec->id].stub_sec;
> +      stub_sec_prefix = link_sec->name;
> +      out_sec = link_sec->output_section;
> +      align = htab->nacl_p ? 4 : 3;
> +    }
> +
> +  if (*stub_sec_p == NULL)
> +    {
> +      size_t namelen;
> +      bfd_size_type len;
> +      char *s_name;
> +
> +      namelen = strlen (stub_sec_prefix);
> +      len = namelen + sizeof (STUB_SUFFIX);
> +      s_name = (char *) bfd_alloc (htab->stub_bfd, len);
> +      if (s_name == NULL)
> +	return NULL;
> +
> +      memcpy (s_name, stub_sec_prefix, namelen);
> +      memcpy (s_name + namelen, STUB_SUFFIX, sizeof (STUB_SUFFIX));
> +      *stub_sec_p = (*htab->add_stub_section) (s_name, out_sec, link_sec,
> +					       align);
> +      if (*stub_sec_p == NULL)
> +	return NULL;
> +
> +      out_sec->flags |= SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
> +			| SEC_HAS_CONTENTS | SEC_RELOC | SEC_IN_MEMORY
> +			| SEC_KEEP;
> +    }
> +
> +  if (!dedicated_output_section)
> +    htab->stub_group[section->id].stub_sec = *stub_sec_p;
> 
>    if (link_sec_p)
>      *link_sec_p = link_sec;
> 
> -  return stub_sec;
> +  return *stub_sec_p;
>  }
> 
>  /* Add a new stub entry to the stub hash.  Not all fields of the new
>     stub entry are initialised.  */
> 
>  static struct elf32_arm_stub_hash_entry *
> -elf32_arm_add_stub (const char *stub_name,
> -		    asection *section,
> -		    struct elf32_arm_link_hash_table *htab)
> +elf32_arm_add_stub (const char *stub_name, asection *section,
> +		    struct elf32_arm_link_hash_table *htab,
> +		    enum elf32_arm_stub_type stub_type)
>  {
>    asection *link_sec;
>    asection *stub_sec;
>    struct elf32_arm_stub_hash_entry *stub_entry;
> 
> -  stub_sec = elf32_arm_create_or_find_stub_sec (&link_sec, section, htab);
> +  stub_sec = elf32_arm_create_or_find_stub_sec (&link_sec, section, htab,
> +						stub_type);
>    if (stub_sec == NULL)
>      return NULL;
> 
> @@ -5187,7 +5274,7 @@ elf32_arm_create_stub (struct
> elf32_arm_link_hash_table *htab,
>        return TRUE;
>      }
> 
> -  stub_entry = elf32_arm_add_stub (stub_name, section, htab);
> +  stub_entry = elf32_arm_add_stub (stub_name, section, htab, stub_type);
>    if (stub_entry == NULL)
>      {
>        if (!sym_claimed)
> @@ -5685,7 +5772,7 @@ elf32_arm_size_stubs (bfd *output_bfd,
>  	for (i = 0; i < num_a8_fixes; i++)
>  	  {
>  	    stub_sec = elf32_arm_create_or_find_stub_sec (NULL,
> -			 a8_fixes[i].section, htab);
> +			 a8_fixes[i].section, htab, a8_fixes[i].stub_type);
> 
>  	    if (stub_sec == NULL)
>  	      goto error_ret_free_local;
> @@ -6479,6 +6566,37 @@ bfd_elf32_arm_add_glue_sections_to_bfd (bfd *abfd,
>      && arm_make_glue_section (abfd, STM32L4XX_ERRATUM_VENEER_SECTION_NAME);
> }
> 
> +/* Mark output sections of veneers needing a dedicated one with SEC_KEEP.
> This
> +   ensures they are not marked for deletion by
> +   strip_excluded_output_sections () when veneers are going to be created
> +   later.  Not doing so would trigger assert on empty section size in
> +   lang_size_sections_1 ().  */
> +
> +void
> +bfd_elf32_arm_keep_private_stub_output_sections (struct bfd_link_info
> *info) +{
> +  enum elf32_arm_stub_type stub_type;
> +
> +  /* If we are only performing a partial
> +     link do not bother adding the glue.  */
> +  if (bfd_link_relocatable (info))
> +    return;
> +
> +  for (stub_type = arm_stub_none + 1; stub_type < max_stub_type;
> stub_type++) +    {
> +      asection *out_sec;
> +      const char *out_sec_name;
> +
> +      if (!arm_dedicated_stub_output_section_required (stub_type))
> +	continue;
> +
> +     out_sec_name = arm_dedicated_stub_output_section_name (stub_type);
> +     out_sec = bfd_get_section_by_name (info->output_bfd, out_sec_name);
> +     if (out_sec != NULL)
> +	out_sec->flags |= SEC_KEEP;
> +    }
> +}
> +
>  /* Select a BFD to be used to hold the sections used by the glue code.
>     This function is called from the linker scripts in ld/emultempl/
>     {armelf/pe}.em.  */
> diff --git a/ld/emultempl/armelf.em b/ld/emultempl/armelf.em
> index
> caa2fbfff3633f2a7f69a7b197574eed8279d6c4..6074824f72d6f607449000702e7c5945bf
> e412ef 100644
> --- a/ld/emultempl/armelf.em
> +++ b/ld/emultempl/armelf.em
> @@ -83,6 +83,10 @@ arm_elf_before_allocation (void)
>    /* Auto-select Cortex-A8 erratum fix if it wasn't explicitly specified. 
> */ bfd_elf32_arm_set_cortex_a8_fix (link_info.output_bfd, &link_info);
> 
> +  /* Ensure the output sections of veneers needing a dedicated one is not
> +     removed.  */
> +  bfd_elf32_arm_keep_private_stub_output_sections (&link_info);
> +
>    /* We should be able to set the size of the interworking stub section. 
> We can't do it until later if we have dynamic sections, though.  */ if
> (elf_hash_table (&link_info)->dynobj == NULL)
> 
> Best regards,
> 
> Thomas