[v1,1/5] ld: fix segfault caused by untagged stub sections

  In the case of non-contiguous memory regions, a far-call stub section
must be assigned to the memory of the section it was originally emitted
for. If the stub section does not fit, the section is marked as dropped,
and removed later. To emit a useful message to the user, however, a stub
section needs to be discernible from sections originating from input
objects.

Previously [1], this distinction was made using the SEC_LINKER_CREATED
flag only the AArch32 backend handler <arch>_add_stub_section. Other
backends that didn't set this flag on their stub sections skipped required
checks in ld/ldlang.c:size_input_section(). On AArch64, this caused the
linker to proceed into code paths that assumed output sections were set,
instead of reporting fatal errors, and ultimately led to a segmentation
fault.

However, the SEC_LINKER_CREATED flag does not solely indicate that a
section was created by the linker. Its original meaning also meant that
the section should not be handled by the generic relocation code. Reusing
this flag to identify stub sections, while it appeared to fix the issue,
introduced unintended side effects. On PowerPC, for instance, it skipped
relocations present in the stubs and interpreted them as absolute
addresses.

This patch proposes a new approach: a new boolean attribute 'veneer' added
to 'asection'. The attribute is set on AArch64 and PowerPC immediately
after the creation of the stub section. Others architectures are left
unchanged, as they do not appear to support non-contiguous memory regions
(no tests were found to verify the fix). Additionally, the diagnostic
message was improved when a stub cannot be placed in the same memory
region as its referencing code.

[1]: abf874a, Add support for non-contiguous memory regions.
---
 bfd/bfd-in2.h                                      |  6 ++++++
 bfd/libbfd.h                                       |  4 ++--
 bfd/section.c                                      | 10 ++++++++--
 ld/emultempl/aarch64elf.em                         |  2 ++
 ld/emultempl/armelf.em                             |  5 +++--
 ld/emultempl/ppc64elf.em                           |  2 ++
 ld/ldlang.c                                        | 13 +++++++------
 ld/testsuite/ld-arm/non-contiguous-arm4.d          |  2 +-
 ld/testsuite/ld-powerpc/non-contiguous-powerpc64.d |  2 +-
 9 files changed, 32 insertions(+), 14 deletions(-)
  

On 18.09.2025 17:06, Matthieu Longo wrote:
> In the case of non-contiguous memory regions, a far-call stub section
> must be assigned to the memory of the section it was originally emitted
> for. If the stub section does not fit, the section is marked as dropped,
> and removed later. To emit a useful message to the user, however, a stub
> section needs to be discernible from sections originating from input
> objects.
> 
> Previously [1], this distinction was made using the SEC_LINKER_CREATED
> flag only the AArch32 backend handler <arch>_add_stub_section. Other
> backends that didn't set this flag on their stub sections skipped required
> checks in ld/ldlang.c:size_input_section(). On AArch64, this caused the
> linker to proceed into code paths that assumed output sections were set,
> instead of reporting fatal errors, and ultimately led to a segmentation
> fault.
> 
> However, the SEC_LINKER_CREATED flag does not solely indicate that a
> section was created by the linker. Its original meaning also meant that
> the section should not be handled by the generic relocation code. Reusing
> this flag to identify stub sections, while it appeared to fix the issue,
> introduced unintended side effects. On PowerPC, for instance, it skipped
> relocations present in the stubs and interpreted them as absolute
> addresses.
> 
> This patch proposes a new approach: a new boolean attribute 'veneer' added
> to 'asection'. The attribute is set on AArch64 and PowerPC immediately
> after the creation of the stub section. Others architectures are left
> unchanged, as they do not appear to support non-contiguous memory regions
> (no tests were found to verify the fix). Additionally, the diagnostic
> message was improved when a stub cannot be placed in the same memory
> region as its referencing code.
> 
> [1]: abf874a, Add support for non-contiguous memory regions.
> ---
>  bfd/bfd-in2.h                                      |  6 ++++++
>  bfd/libbfd.h                                       |  4 ++--
>  bfd/section.c                                      | 10 ++++++++--
>  ld/emultempl/aarch64elf.em                         |  2 ++
>  ld/emultempl/armelf.em                             |  5 +++--
>  ld/emultempl/ppc64elf.em                           |  2 ++
>  ld/ldlang.c                                        | 13 +++++++------
>  ld/testsuite/ld-arm/non-contiguous-arm4.d          |  2 +-
>  ld/testsuite/ld-powerpc/non-contiguous-powerpc64.d |  2 +-
>  9 files changed, 32 insertions(+), 14 deletions(-)

I'm okay with the approach, yet there is a largely mechanical issue:

> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -834,6 +834,12 @@ typedef struct bfd_section
>      const char *linked_to_symbol_name;
>    } map_head, map_tail;
>  
> +  /* Indicate that the section contains branch veneers.  This is used when
> +     support for non-contiguous memory regions is enabled.  The veneers have
> +     to be allocated to the same memory region as the code they are refered
> +     by, i.e. they cannot be moved to a subsequent memory region.  */
> +  bool veneer;

Why would you put a bool between two pointer-sized fields, thus introducing
yet more padding, when in fact existing padding could be used. There's a set
of bitfields further up from here, and you could simply add a single-bit
field there, for example. Generic code changes are okay with that change,
but arch-specific ones will need arch maintainer approval.

Jan
  

On Thu, Sep 18, 2025 at 11:07 PM Matthieu Longo <matthieu.longo@arm.com> wrote:
>
> In the case of non-contiguous memory regions, a far-call stub section
> must be assigned to the memory of the section it was originally emitted
> for. If the stub section does not fit, the section is marked as dropped,
> and removed later. To emit a useful message to the user, however, a stub
> section needs to be discernible from sections originating from input
> objects.
>
> Previously [1], this distinction was made using the SEC_LINKER_CREATED
> flag only the AArch32 backend handler <arch>_add_stub_section. Other
> backends that didn't set this flag on their stub sections skipped required
> checks in ld/ldlang.c:size_input_section(). On AArch64, this caused the
> linker to proceed into code paths that assumed output sections were set,
> instead of reporting fatal errors, and ultimately led to a segmentation
> fault.
>
> However, the SEC_LINKER_CREATED flag does not solely indicate that a
> section was created by the linker. Its original meaning also meant that
> the section should not be handled by the generic relocation code. Reusing
> this flag to identify stub sections, while it appeared to fix the issue,
> introduced unintended side effects. On PowerPC, for instance, it skipped
> relocations present in the stubs and interpreted them as absolute
> addresses.
>
> This patch proposes a new approach: a new boolean attribute 'veneer' added
> to 'asection'. The attribute is set on AArch64 and PowerPC immediately
> after the creation of the stub section. Others architectures are left
> unchanged, as they do not appear to support non-contiguous memory regions
> (no tests were found to verify the fix). Additionally, the diagnostic
> message was improved when a stub cannot be placed in the same memory
> region as its referencing code.
>
> [1]: abf874a, Add support for non-contiguous memory regions.
> ---
>  bfd/bfd-in2.h                                      |  6 ++++++
>  bfd/libbfd.h                                       |  4 ++--
>  bfd/section.c                                      | 10 ++++++++--
>  ld/emultempl/aarch64elf.em                         |  2 ++
>  ld/emultempl/armelf.em                             |  5 +++--
>  ld/emultempl/ppc64elf.em                           |  2 ++
>  ld/ldlang.c                                        | 13 +++++++------
>  ld/testsuite/ld-arm/non-contiguous-arm4.d          |  2 +-
>  ld/testsuite/ld-powerpc/non-contiguous-powerpc64.d |  2 +-
>  9 files changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> index 5e7c6ddf1ee..a4049fa0e98 100644
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -834,6 +834,12 @@ typedef struct bfd_section
>      const char *linked_to_symbol_name;
>    } map_head, map_tail;
>
> +  /* Indicate that the section contains branch veneers.  This is used when
> +     support for non-contiguous memory regions is enabled.  The veneers have
> +     to be allocated to the same memory region as the code they are refered
> +     by, i.e. they cannot be moved to a subsequent memory region.  */
> +  bool veneer;
> +
>    /* Points to the output section this section is already assigned to,
>       if any.  This is used when support for non-contiguous memory
>       regions is enabled.  */
> diff --git a/bfd/libbfd.h b/bfd/libbfd.h
> index f2485d99078..df9d5c8cc19 100644
> --- a/bfd/libbfd.h
> +++ b/bfd/libbfd.h
> @@ -3694,8 +3694,8 @@ bool _bfd_unrecognized_reloc
>    /* symbol,                                                        */ \
>       (struct bfd_symbol *) SYM,                                        \
>                                                                        \
> -  /* map_head, map_tail, already_assigned, type                     */ \
> -     { NULL }, { NULL }, NULL,             0                           \
> +  /* map_head, map_tail, veneer, already_assigned, type            */  \
> +     { NULL }, { NULL }, false,  NULL,             0                   \
>                                                                        \
>    }
>
> diff --git a/bfd/section.c b/bfd/section.c
> index 5f0cf6e71cb..33524762e7a 100644
> --- a/bfd/section.c
> +++ b/bfd/section.c
> @@ -560,6 +560,12 @@ CODE_FRAGMENT
>  .    const char *linked_to_symbol_name;
>  .  } map_head, map_tail;
>  .
> +.  {* Indicate that the section contains branch veneers.  This is used when
> +.     support for non-contiguous memory regions is enabled.  The veneers have
> +.     to be allocated to the same memory region as the code they are refered
> +.     by, i.e. they cannot be moved to a subsequent memory region.  *}
> +.  bool veneer;

Why not a bitfield here?

> +.
>  .  {* Points to the output section this section is already assigned to,
>  .     if any.  This is used when support for non-contiguous memory
>  .     regions is enabled.  *}
> @@ -747,8 +753,8 @@ INTERNAL
>  .  {* symbol,                                                        *}        \
>  .     (struct bfd_symbol *) SYM,                                       \
>  .                                                                      \
> -.  {* map_head, map_tail, already_assigned, type                     *}        \
> -.     { NULL }, { NULL }, NULL,             0                          \
> +.  {* map_head, map_tail, veneer, already_assigned, type            *} \
> +.     { NULL }, { NULL }, false,  NULL,             0                  \
>  .                                                                      \
>  .  }
>  .
> diff --git a/ld/emultempl/aarch64elf.em b/ld/emultempl/aarch64elf.em
> index 91d58d8fe5a..08f5c4c7d58 100644
> --- a/ld/emultempl/aarch64elf.em
> +++ b/ld/emultempl/aarch64elf.em
> @@ -200,6 +200,8 @@ elf${ELFSIZE}_aarch64_add_stub_section (const char *stub_sec_name,
>    if (stub_sec == NULL)
>      goto err_ret;
>
> +  stub_sec->veneer = true;
> +
>    /* Long branch stubs contain a 64-bit address, so the section requires
>       8 byte alignment.  */
>    bfd_set_section_alignment (stub_sec, 3);
> diff --git a/ld/emultempl/armelf.em b/ld/emultempl/armelf.em
> index 6f652c59a3c..d946f096faa 100644
> --- a/ld/emultempl/armelf.em
> +++ b/ld/emultempl/armelf.em
> @@ -237,13 +237,14 @@ elf32_arm_add_stub_section (const char * stub_sec_name,
>    struct hook_stub_info info;
>
>    flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
> -          | SEC_HAS_CONTENTS | SEC_RELOC | SEC_IN_MEMORY | SEC_KEEP
> -          | SEC_LINKER_CREATED);
> +          | SEC_HAS_CONTENTS | SEC_RELOC | SEC_IN_MEMORY | SEC_KEEP);
>    stub_sec = bfd_make_section_anyway_with_flags (stub_file->the_bfd,
>                                                  stub_sec_name, flags);
>    if (stub_sec == NULL)
>      goto err_ret;
>
> +  stub_sec->veneer = true;
> +
>    bfd_set_section_alignment (stub_sec, alignment_power);
>
>    os = lang_output_section_get (output_section);
> diff --git a/ld/emultempl/ppc64elf.em b/ld/emultempl/ppc64elf.em
> index 857cf54ad06..8e3f60cb12a 100644
> --- a/ld/emultempl/ppc64elf.em
> +++ b/ld/emultempl/ppc64elf.em
> @@ -440,6 +440,8 @@ ppc_add_stub_section (const char *stub_sec_name, asection *input_section)
>                                                 : 5)))
>      goto err_ret;
>
> +  stub_sec->veneer = true;
> +
>    output_section = input_section->output_section;
>    os = lang_output_section_get (output_section);
>
> diff --git a/ld/ldlang.c b/ld/ldlang.c
> index 3c57bf7541c..bdda599a7a4 100644
> --- a/ld/ldlang.c
> +++ b/ld/ldlang.c
> @@ -5637,10 +5637,12 @@ size_input_section
>
>               if (dot + TO_ADDR (i->size) > end)
>                 {
> -                 if (i->flags & SEC_LINKER_CREATED)
> -                   fatal (_("%P: Output section `%pA' not large enough for "
> -                            "the linker-created stubs section `%pA'.\n"),
> -                          i->output_section, i);
> +                 if (i->veneer)
> +                   fatal (_("%P: Memory region `%s' not large enough for the "
> +                            "linker-created stubs section `%pA' associated to "
> +                            "output section `%pA'\n"),
> +                          output_section_statement->region->name_list.name, i,
> +                          i->output_section);
>
>                   if (i->rawsize && i->rawsize != i->size)
>                     fatal (_("%P: Relaxation not supported with "
> @@ -8239,8 +8241,7 @@ warn_non_contiguous_discards (void)
>         continue;
>
>        for (asection *s = file->the_bfd->sections; s != NULL; s = s->next)
> -       if (s->output_section == NULL
> -           && (s->flags & SEC_LINKER_CREATED) == 0)
> +       if (s->output_section == NULL && !s->veneer)
>           einfo (_("%P: warning: --enable-non-contiguous-regions "
>                    "discards section `%pA' from `%pB'\n"),
>                  s, file->the_bfd);
> diff --git a/ld/testsuite/ld-arm/non-contiguous-arm4.d b/ld/testsuite/ld-arm/non-contiguous-arm4.d
> index a8e9d66caca..d4c4b284dce 100644
> --- a/ld/testsuite/ld-arm/non-contiguous-arm4.d
> +++ b/ld/testsuite/ld-arm/non-contiguous-arm4.d
> @@ -1,4 +1,4 @@
>  #name: non-contiguous-arm4
>  #source: non-contiguous-arm.s
>  #ld: --enable-non-contiguous-regions -T non-contiguous-arm4.ld
> -# error: .*Output section .?\.ramu.? not large enough for the linker-created stubs section .?\.code\.3\.__stub.\.?
> +# error: Memory region `RAMU' not large enough for the linker-created stubs section `\.code\.3\.__stub' associated to output section `\.ramu'
> diff --git a/ld/testsuite/ld-powerpc/non-contiguous-powerpc64.d b/ld/testsuite/ld-powerpc/non-contiguous-powerpc64.d
> index 9f903bbea35..1a7e4c5a23b 100644
> --- a/ld/testsuite/ld-powerpc/non-contiguous-powerpc64.d
> +++ b/ld/testsuite/ld-powerpc/non-contiguous-powerpc64.d
> @@ -2,4 +2,4 @@
>  #source: non-contiguous-powerpc.s
>  #as: -a64
>  #ld: -melf64ppc --enable-non-contiguous-regions -T non-contiguous-powerpc.ld
> -#error: .*Could not assign .?\.text\.one\.stub.? to an output section\. Retry without --enable-non-contiguous-regions\.
> +#error: Memory region `one' not large enough for the linker-created stubs section `\.text\.one\.stub' associated to output section `one'
> --
> 2.51.0
>
  

On 2025-09-26 14:13, Jan Beulich wrote:
> On 18.09.2025 17:06, Matthieu Longo wrote:
>> In the case of non-contiguous memory regions, a far-call stub section
>> must be assigned to the memory of the section it was originally emitted
>> for. If the stub section does not fit, the section is marked as dropped,
>> and removed later. To emit a useful message to the user, however, a stub
>> section needs to be discernible from sections originating from input
>> objects.
>>
>> Previously [1], this distinction was made using the SEC_LINKER_CREATED
>> flag only the AArch32 backend handler <arch>_add_stub_section. Other
>> backends that didn't set this flag on their stub sections skipped required
>> checks in ld/ldlang.c:size_input_section(). On AArch64, this caused the
>> linker to proceed into code paths that assumed output sections were set,
>> instead of reporting fatal errors, and ultimately led to a segmentation
>> fault.
>>
>> However, the SEC_LINKER_CREATED flag does not solely indicate that a
>> section was created by the linker. Its original meaning also meant that
>> the section should not be handled by the generic relocation code. Reusing
>> this flag to identify stub sections, while it appeared to fix the issue,
>> introduced unintended side effects. On PowerPC, for instance, it skipped
>> relocations present in the stubs and interpreted them as absolute
>> addresses.
>>
>> This patch proposes a new approach: a new boolean attribute 'veneer' added
>> to 'asection'. The attribute is set on AArch64 and PowerPC immediately
>> after the creation of the stub section. Others architectures are left
>> unchanged, as they do not appear to support non-contiguous memory regions
>> (no tests were found to verify the fix). Additionally, the diagnostic
>> message was improved when a stub cannot be placed in the same memory
>> region as its referencing code.
>>
>> [1]: abf874a, Add support for non-contiguous memory regions.
>> ---
>>   bfd/bfd-in2.h                                      |  6 ++++++
>>   bfd/libbfd.h                                       |  4 ++--
>>   bfd/section.c                                      | 10 ++++++++--
>>   ld/emultempl/aarch64elf.em                         |  2 ++
>>   ld/emultempl/armelf.em                             |  5 +++--
>>   ld/emultempl/ppc64elf.em                           |  2 ++
>>   ld/ldlang.c                                        | 13 +++++++------
>>   ld/testsuite/ld-arm/non-contiguous-arm4.d          |  2 +-
>>   ld/testsuite/ld-powerpc/non-contiguous-powerpc64.d |  2 +-
>>   9 files changed, 32 insertions(+), 14 deletions(-)
> 
> I'm okay with the approach, yet there is a largely mechanical issue:
> 
>> --- a/bfd/bfd-in2.h
>> +++ b/bfd/bfd-in2.h
>> @@ -834,6 +834,12 @@ typedef struct bfd_section
>>       const char *linked_to_symbol_name;
>>     } map_head, map_tail;
>>   
>> +  /* Indicate that the section contains branch veneers.  This is used when
>> +     support for non-contiguous memory regions is enabled.  The veneers have
>> +     to be allocated to the same memory region as the code they are refered
>> +     by, i.e. they cannot be moved to a subsequent memory region.  */
>> +  bool veneer;
> 
> Why would you put a bool between two pointer-sized fields, thus introducing
> yet more padding, when in fact existing padding could be used. There's a set
> of bitfields further up from here, and you could simply add a single-bit
> field there, for example. Generic code changes are okay with that change,
> but arch-specific ones will need arch maintainer approval.
> 
> Jan

Hi Jan,

Sorry for the silence, I was on annual leave for about 3 weeks.

Here is the new attribute as a flag:

diff --git a/bfd/section.c b/bfd/section.c
index 5f0cf6e71cb..f110ed77363 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -379,6 +379,12 @@ CODE_FRAGMENT
  .     when memory read flag isn't set. *}
  .#define SEC_COFF_NOREAD            0x40000000
  .
+.  {* Indicate that the section contains branch veneers.  This is used when
+.     support for non-contiguous memory regions is enabled.  The 
veneers have
+.     to be allocated to the same memory region as the code they are 
refered
+.     by, i.e. they cannot be moved to a subsequent memory region.  *}
+.#define SEC_VENEER                 0x80000000
+.
  .  {*  End of section flags.  *}
  .
  .  {* Some internal packed boolean fields.  *}


With the tagging of the stub section on AArch64:

diff --git a/ld/emultempl/aarch64elf.em b/ld/emultempl/aarch64elf.em
index 91d58d8fe5a..b00c9460da2 100644
--- a/ld/emultempl/aarch64elf.em
+++ b/ld/emultempl/aarch64elf.em
@@ -193,7 +193,7 @@ elf${ELFSIZE}_aarch64_add_stub_section (const char 
*stub_sec_name,
    lang_output_section_statement_type *os;
    struct hook_stub_info info;

-  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
+  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE | SEC_VENEER
            | SEC_HAS_CONTENTS | SEC_RELOC | SEC_IN_MEMORY | SEC_KEEP);
    stub_sec = bfd_make_section_anyway_with_flags (stub_file->the_bfd,
                                                  stub_sec_name, flags);

Same thing on AArch32 and PowerPC.

Is this what you suggested ?

Matthieu
  

On 2025-09-26 22:39, H.J. Lu wrote:
> On Thu, Sep 18, 2025 at 11:07 PM Matthieu Longo <matthieu.longo@arm.com> wrote:
>>
>> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
>> index 5e7c6ddf1ee..a4049fa0e98 100644
>> --- a/bfd/bfd-in2.h
>> +++ b/bfd/bfd-in2.h
>> @@ -834,6 +834,12 @@ typedef struct bfd_section
>>       const char *linked_to_symbol_name;
>>     } map_head, map_tail;
>>
>> +  /* Indicate that the section contains branch veneers.  This is used when
>> +     support for non-contiguous memory regions is enabled.  The veneers have
>> +     to be allocated to the same memory region as the code they are refered
>> +     by, i.e. they cannot be moved to a subsequent memory region.  */
>> +  bool veneer;
>> +
>>     /* Points to the output section this section is already assigned to,
>>        if any.  This is used when support for non-contiguous memory
>>        regions is enabled.  */
>> diff --git a/bfd/section.c b/bfd/section.c
>> index 5f0cf6e71cb..33524762e7a 100644
>> --- a/bfd/section.c
>> +++ b/bfd/section.c
>> @@ -560,6 +560,12 @@ CODE_FRAGMENT
>>   .    const char *linked_to_symbol_name;
>>   .  } map_head, map_tail;
>>   .
>> +.  {* Indicate that the section contains branch veneers.  This is used when
>> +.     support for non-contiguous memory regions is enabled.  The veneers have
>> +.     to be allocated to the same memory region as the code they are refered
>> +.     by, i.e. they cannot be moved to a subsequent memory region.  *}
>> +.  bool veneer;
> 
> Why not a bitfield here?

Jan also suggested it. See my reply to his email.

Matthieu
  

On 13.10.2025 17:22, Matthieu Longo wrote:
> On 2025-09-26 14:13, Jan Beulich wrote:
>> On 18.09.2025 17:06, Matthieu Longo wrote:
>>> --- a/bfd/bfd-in2.h
>>> +++ b/bfd/bfd-in2.h
>>> @@ -834,6 +834,12 @@ typedef struct bfd_section
>>>       const char *linked_to_symbol_name;
>>>     } map_head, map_tail;
>>>   
>>> +  /* Indicate that the section contains branch veneers.  This is used when
>>> +     support for non-contiguous memory regions is enabled.  The veneers have
>>> +     to be allocated to the same memory region as the code they are refered
>>> +     by, i.e. they cannot be moved to a subsequent memory region.  */
>>> +  bool veneer;
>>
>> Why would you put a bool between two pointer-sized fields, thus introducing
>> yet more padding, when in fact existing padding could be used. There's a set
>> of bitfields further up from here, and you could simply add a single-bit
>> field there, for example. Generic code changes are okay with that change,
>> but arch-specific ones will need arch maintainer approval.
> 
> Here is the new attribute as a flag:
> 
> diff --git a/bfd/section.c b/bfd/section.c
> index 5f0cf6e71cb..f110ed77363 100644
> --- a/bfd/section.c
> +++ b/bfd/section.c
> @@ -379,6 +379,12 @@ CODE_FRAGMENT
>   .     when memory read flag isn't set. *}
>   .#define SEC_COFF_NOREAD            0x40000000
>   .
> +.  {* Indicate that the section contains branch veneers.  This is used when
> +.     support for non-contiguous memory regions is enabled.  The 
> veneers have
> +.     to be allocated to the same memory region as the code they are 
> refered
> +.     by, i.e. they cannot be moved to a subsequent memory region.  *}
> +.#define SEC_VENEER                 0x80000000
> +.
>   .  {*  End of section flags.  *}
>   .
>   .  {* Some internal packed boolean fields.  *}
> 
> 
> With the tagging of the stub section on AArch64:
> 
> diff --git a/ld/emultempl/aarch64elf.em b/ld/emultempl/aarch64elf.em
> index 91d58d8fe5a..b00c9460da2 100644
> --- a/ld/emultempl/aarch64elf.em
> +++ b/ld/emultempl/aarch64elf.em
> @@ -193,7 +193,7 @@ elf${ELFSIZE}_aarch64_add_stub_section (const char 
> *stub_sec_name,
>     lang_output_section_statement_type *os;
>     struct hook_stub_info info;
> 
> -  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
> +  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE | SEC_VENEER
>             | SEC_HAS_CONTENTS | SEC_RELOC | SEC_IN_MEMORY | SEC_KEEP);
>     stub_sec = bfd_make_section_anyway_with_flags (stub_file->the_bfd,
>                                                   stub_sec_name, flags);
> 
> Same thing on AArch32 and PowerPC.
> 
> Is this what you suggested ?

No, sorry. I said "bit field", not "section flag". I'd rather not see you
use the last available section flag. Right below where you put the new
#define is a comment "Some internal packed boolean fields" - that's where
I thought your new flag would go.

Jan
  

On 2025-10-13 16:30, Jan Beulich wrote:
> On 13.10.2025 17:22, Matthieu Longo wrote:
>> On 2025-09-26 14:13, Jan Beulich wrote:
>>> On 18.09.2025 17:06, Matthieu Longo wrote:
>>>> --- a/bfd/bfd-in2.h
>>>> +++ b/bfd/bfd-in2.h
>>>> @@ -834,6 +834,12 @@ typedef struct bfd_section
>>>>        const char *linked_to_symbol_name;
>>>>      } map_head, map_tail;
>>>>    
>>>> +  /* Indicate that the section contains branch veneers.  This is used when
>>>> +     support for non-contiguous memory regions is enabled.  The veneers have
>>>> +     to be allocated to the same memory region as the code they are refered
>>>> +     by, i.e. they cannot be moved to a subsequent memory region.  */
>>>> +  bool veneer;
>>>
>>> Why would you put a bool between two pointer-sized fields, thus introducing
>>> yet more padding, when in fact existing padding could be used. There's a set
>>> of bitfields further up from here, and you could simply add a single-bit
>>> field there, for example. Generic code changes are okay with that change,
>>> but arch-specific ones will need arch maintainer approval.
>>
>> Here is the new attribute as a flag:
>>
>> diff --git a/bfd/section.c b/bfd/section.c
>> index 5f0cf6e71cb..f110ed77363 100644
>> --- a/bfd/section.c
>> +++ b/bfd/section.c
>> @@ -379,6 +379,12 @@ CODE_FRAGMENT
>>    .     when memory read flag isn't set. *}
>>    .#define SEC_COFF_NOREAD            0x40000000
>>    .
>> +.  {* Indicate that the section contains branch veneers.  This is used when
>> +.     support for non-contiguous memory regions is enabled.  The
>> veneers have
>> +.     to be allocated to the same memory region as the code they are
>> refered
>> +.     by, i.e. they cannot be moved to a subsequent memory region.  *}
>> +.#define SEC_VENEER                 0x80000000
>> +.
>>    .  {*  End of section flags.  *}
>>    .
>>    .  {* Some internal packed boolean fields.  *}
>>
>>
>> With the tagging of the stub section on AArch64:
>>
>> diff --git a/ld/emultempl/aarch64elf.em b/ld/emultempl/aarch64elf.em
>> index 91d58d8fe5a..b00c9460da2 100644
>> --- a/ld/emultempl/aarch64elf.em
>> +++ b/ld/emultempl/aarch64elf.em
>> @@ -193,7 +193,7 @@ elf${ELFSIZE}_aarch64_add_stub_section (const char
>> *stub_sec_name,
>>      lang_output_section_statement_type *os;
>>      struct hook_stub_info info;
>>
>> -  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
>> +  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE | SEC_VENEER
>>              | SEC_HAS_CONTENTS | SEC_RELOC | SEC_IN_MEMORY | SEC_KEEP);
>>      stub_sec = bfd_make_section_anyway_with_flags (stub_file->the_bfd,
>>                                                    stub_sec_name, flags);
>>
>> Same thing on AArch32 and PowerPC.
>>
>> Is this what you suggested ?
> 
> No, sorry. I said "bit field", not "section flag". I'd rather not see you
> use the last available section flag. Right below where you put the new
> #define is a comment "Some internal packed boolean fields" - that's where
> I thought your new flag would go.
> 
> Jan

What about the below ?

diff --git a/bfd/section.c b/bfd/section.c
index 5f0cf6e71cb..424e53029d9 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -428,6 +428,12 @@ CODE_FRAGMENT
  .  {* Nonzero if section contents should not be freed.  *}
  .  unsigned int alloced:1;
  .
+.  {* Indicate that the section contains branch veneers.  This is used when
+.     support for non-contiguous memory regions is enabled.  The 
veneers have
+.     to be allocated to the same memory region as the code they are 
refered
+.     by, i.e. they cannot be moved to a subsequent memory region.  *}
+.  unsigned int veneer : 1;
+.
  .  {* Bits used by various backends.  The generic code doesn't touch
  .     these fields.  *}
  .
@@ -723,11 +729,11 @@ INTERNAL
  .  {* segment_mark, sec_info_type, use_rela_p, mmapped_p, alloced, 
*}        \
  .     0,            0,             0,          0,         0,           \
  .                                                                      \
-.  {* sec_flg0, sec_flg1, sec_flg2, sec_flg3, sec_flg4, sec_flg5,    *} 
        \
+.  {* veneer, sec_flg0, sec_flg1, sec_flg2, sec_flg3, sec_flg4,      *} 
        \
  .     0,        0,        0,        0,        0,        0,             \
  .                                                                      \
-.  {* vma, lma, size, rawsize, compressed_size,                      *} 
        \
-.     0,   0,   0,    0,       0,                                      \
+.  {* sec_flg5, vma, lma, size, rawsize, compressed_size,            *} 
        \
+.     0,        0,   0,   0,    0,       0,                            \
  .                                                                      \
  .  {* output_offset, output_section, relocation, orelocation, 
*}        \
  .     0,             &SEC,           NULL,       NULL,                 \

Matthieu
  

On 13.10.2025 19:01, Matthieu Longo wrote:
> On 2025-10-13 16:30, Jan Beulich wrote:
>> On 13.10.2025 17:22, Matthieu Longo wrote:
>>> On 2025-09-26 14:13, Jan Beulich wrote:
>>>> On 18.09.2025 17:06, Matthieu Longo wrote:
>>>>> --- a/bfd/bfd-in2.h
>>>>> +++ b/bfd/bfd-in2.h
>>>>> @@ -834,6 +834,12 @@ typedef struct bfd_section
>>>>>        const char *linked_to_symbol_name;
>>>>>      } map_head, map_tail;
>>>>>    
>>>>> +  /* Indicate that the section contains branch veneers.  This is used when
>>>>> +     support for non-contiguous memory regions is enabled.  The veneers have
>>>>> +     to be allocated to the same memory region as the code they are refered
>>>>> +     by, i.e. they cannot be moved to a subsequent memory region.  */
>>>>> +  bool veneer;
>>>>
>>>> Why would you put a bool between two pointer-sized fields, thus introducing
>>>> yet more padding, when in fact existing padding could be used. There's a set
>>>> of bitfields further up from here, and you could simply add a single-bit
>>>> field there, for example. Generic code changes are okay with that change,
>>>> but arch-specific ones will need arch maintainer approval.
>>>
>>> Here is the new attribute as a flag:
>>>
>>> diff --git a/bfd/section.c b/bfd/section.c
>>> index 5f0cf6e71cb..f110ed77363 100644
>>> --- a/bfd/section.c
>>> +++ b/bfd/section.c
>>> @@ -379,6 +379,12 @@ CODE_FRAGMENT
>>>    .     when memory read flag isn't set. *}
>>>    .#define SEC_COFF_NOREAD            0x40000000
>>>    .
>>> +.  {* Indicate that the section contains branch veneers.  This is used when
>>> +.     support for non-contiguous memory regions is enabled.  The
>>> veneers have
>>> +.     to be allocated to the same memory region as the code they are
>>> refered
>>> +.     by, i.e. they cannot be moved to a subsequent memory region.  *}
>>> +.#define SEC_VENEER                 0x80000000
>>> +.
>>>    .  {*  End of section flags.  *}
>>>    .
>>>    .  {* Some internal packed boolean fields.  *}
>>>
>>>
>>> With the tagging of the stub section on AArch64:
>>>
>>> diff --git a/ld/emultempl/aarch64elf.em b/ld/emultempl/aarch64elf.em
>>> index 91d58d8fe5a..b00c9460da2 100644
>>> --- a/ld/emultempl/aarch64elf.em
>>> +++ b/ld/emultempl/aarch64elf.em
>>> @@ -193,7 +193,7 @@ elf${ELFSIZE}_aarch64_add_stub_section (const char
>>> *stub_sec_name,
>>>      lang_output_section_statement_type *os;
>>>      struct hook_stub_info info;
>>>
>>> -  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
>>> +  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE | SEC_VENEER
>>>              | SEC_HAS_CONTENTS | SEC_RELOC | SEC_IN_MEMORY | SEC_KEEP);
>>>      stub_sec = bfd_make_section_anyway_with_flags (stub_file->the_bfd,
>>>                                                    stub_sec_name, flags);
>>>
>>> Same thing on AArch32 and PowerPC.
>>>
>>> Is this what you suggested ?
>>
>> No, sorry. I said "bit field", not "section flag". I'd rather not see you
>> use the last available section flag. Right below where you put the new
>> #define is a comment "Some internal packed boolean fields" - that's where
>> I thought your new flag would go.
> 
> What about the below ?

Yes, with one cosmetic request though:

> diff --git a/bfd/section.c b/bfd/section.c
> index 5f0cf6e71cb..424e53029d9 100644
> --- a/bfd/section.c
> +++ b/bfd/section.c
> @@ -428,6 +428,12 @@ CODE_FRAGMENT
>   .  {* Nonzero if section contents should not be freed.  *}
>   .  unsigned int alloced:1;
>   .
> +.  {* Indicate that the section contains branch veneers.  This is used when
> +.     support for non-contiguous memory regions is enabled.  The 
> veneers have
> +.     to be allocated to the same memory region as the code they are 
> refered
> +.     by, i.e. they cannot be moved to a subsequent memory region.  *}
> +.  unsigned int veneer : 1;
> +.
>   .  {* Bits used by various backends.  The generic code doesn't touch
>   .     these fields.  *}
>   .
> @@ -723,11 +729,11 @@ INTERNAL
>   .  {* segment_mark, sec_info_type, use_rela_p, mmapped_p, alloced, 
> *}        \
>   .     0,            0,             0,          0,         0,           \
>   .                                                                      \
> -.  {* sec_flg0, sec_flg1, sec_flg2, sec_flg3, sec_flg4, sec_flg5,    *} 

Please can these all stay on one line, while you ...

>         \
> +.  {* veneer, sec_flg0, sec_flg1, sec_flg2, sec_flg3, sec_flg4,      *} 

... add a new line to put the new initializer on? (Overall the macro likely
wants switching to C99 dedicated initializers, at which point the comments
can all go away, as can the many 0s and NULL-s. But that's for another day,
I guess.)

Jan
  

On 2025-10-14 08:23, Jan Beulich wrote:
> On 13.10.2025 19:01, Matthieu Longo wrote:
>> On 2025-10-13 16:30, Jan Beulich wrote:
>>> On 13.10.2025 17:22, Matthieu Longo wrote:
>>>> On 2025-09-26 14:13, Jan Beulich wrote:
>>>>> On 18.09.2025 17:06, Matthieu Longo wrote:
>>>>>> --- a/bfd/bfd-in2.h
>>>>>> +++ b/bfd/bfd-in2.h
>>>>>> @@ -834,6 +834,12 @@ typedef struct bfd_section
>>>>>>         const char *linked_to_symbol_name;
>>>>>>       } map_head, map_tail;
>>>>>>     
>>>>>> +  /* Indicate that the section contains branch veneers.  This is used when
>>>>>> +     support for non-contiguous memory regions is enabled.  The veneers have
>>>>>> +     to be allocated to the same memory region as the code they are refered
>>>>>> +     by, i.e. they cannot be moved to a subsequent memory region.  */
>>>>>> +  bool veneer;
>>>>>
>>>>> Why would you put a bool between two pointer-sized fields, thus introducing
>>>>> yet more padding, when in fact existing padding could be used. There's a set
>>>>> of bitfields further up from here, and you could simply add a single-bit
>>>>> field there, for example. Generic code changes are okay with that change,
>>>>> but arch-specific ones will need arch maintainer approval.
>>>>
>>>> Here is the new attribute as a flag:
>>>>
>>>> diff --git a/bfd/section.c b/bfd/section.c
>>>> index 5f0cf6e71cb..f110ed77363 100644
>>>> --- a/bfd/section.c
>>>> +++ b/bfd/section.c
>>>> @@ -379,6 +379,12 @@ CODE_FRAGMENT
>>>>     .     when memory read flag isn't set. *}
>>>>     .#define SEC_COFF_NOREAD            0x40000000
>>>>     .
>>>> +.  {* Indicate that the section contains branch veneers.  This is used when
>>>> +.     support for non-contiguous memory regions is enabled.  The
>>>> veneers have
>>>> +.     to be allocated to the same memory region as the code they are
>>>> refered
>>>> +.     by, i.e. they cannot be moved to a subsequent memory region.  *}
>>>> +.#define SEC_VENEER                 0x80000000
>>>> +.
>>>>     .  {*  End of section flags.  *}
>>>>     .
>>>>     .  {* Some internal packed boolean fields.  *}
>>>>
>>>>
>>>> With the tagging of the stub section on AArch64:
>>>>
>>>> diff --git a/ld/emultempl/aarch64elf.em b/ld/emultempl/aarch64elf.em
>>>> index 91d58d8fe5a..b00c9460da2 100644
>>>> --- a/ld/emultempl/aarch64elf.em
>>>> +++ b/ld/emultempl/aarch64elf.em
>>>> @@ -193,7 +193,7 @@ elf${ELFSIZE}_aarch64_add_stub_section (const char
>>>> *stub_sec_name,
>>>>       lang_output_section_statement_type *os;
>>>>       struct hook_stub_info info;
>>>>
>>>> -  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
>>>> +  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE | SEC_VENEER
>>>>               | SEC_HAS_CONTENTS | SEC_RELOC | SEC_IN_MEMORY | SEC_KEEP);
>>>>       stub_sec = bfd_make_section_anyway_with_flags (stub_file->the_bfd,
>>>>                                                     stub_sec_name, flags);
>>>>
>>>> Same thing on AArch32 and PowerPC.
>>>>
>>>> Is this what you suggested ?
>>>
>>> No, sorry. I said "bit field", not "section flag". I'd rather not see you
>>> use the last available section flag. Right below where you put the new
>>> #define is a comment "Some internal packed boolean fields" - that's where
>>> I thought your new flag would go.
>>
>> What about the below ?
> 
> Yes, with one cosmetic request though:
> 
>> diff --git a/bfd/section.c b/bfd/section.c
>> index 5f0cf6e71cb..424e53029d9 100644
>> --- a/bfd/section.c
>> +++ b/bfd/section.c
>> @@ -428,6 +428,12 @@ CODE_FRAGMENT
>>    .  {* Nonzero if section contents should not be freed.  *}
>>    .  unsigned int alloced:1;
>>    .
>> +.  {* Indicate that the section contains branch veneers.  This is used when
>> +.     support for non-contiguous memory regions is enabled.  The
>> veneers have
>> +.     to be allocated to the same memory region as the code they are
>> refered
>> +.     by, i.e. they cannot be moved to a subsequent memory region.  *}
>> +.  unsigned int veneer : 1;
>> +.
>>    .  {* Bits used by various backends.  The generic code doesn't touch
>>    .     these fields.  *}
>>    .
>> @@ -723,11 +729,11 @@ INTERNAL
>>    .  {* segment_mark, sec_info_type, use_rela_p, mmapped_p, alloced,
>> *}        \
>>    .     0,            0,             0,          0,         0,           \
>>    .                                                                      \
>> -.  {* sec_flg0, sec_flg1, sec_flg2, sec_flg3, sec_flg4, sec_flg5,    *}
> 
> Please can these all stay on one line, while you ...
> 
>>          \
>> +.  {* veneer, sec_flg0, sec_flg1, sec_flg2, sec_flg3, sec_flg4,      *}
> 
> ... add a new line to put the new initializer on? (Overall the macro likely
> wants switching to C99 dedicated initializers, at which point the comments
> can all go away, as can the many 0s and NULL-s. But that's for another day,
> I guess.)
> 
> Jan

Fixed in the next revision.

Matthieu
  

On 2025-10-14 11:38, Matthieu Longo wrote:
> On 2025-10-14 08:23, Jan Beulich wrote:
>> On 13.10.2025 19:01, Matthieu Longo wrote:
>>> On 2025-10-13 16:30, Jan Beulich wrote:
>>>> On 13.10.2025 17:22, Matthieu Longo wrote:
>>>>> On 2025-09-26 14:13, Jan Beulich wrote:
>>>>>> On 18.09.2025 17:06, Matthieu Longo wrote:
>>>>>>> --- a/bfd/bfd-in2.h
>>>>>>> +++ b/bfd/bfd-in2.h
>>>>>>> @@ -834,6 +834,12 @@ typedef struct bfd_section
>>>>>>>         const char *linked_to_symbol_name;
>>>>>>>       } map_head, map_tail;
>>>>>>> +  /* Indicate that the section contains branch veneers.  This is 
>>>>>>> used when
>>>>>>> +     support for non-contiguous memory regions is enabled.  The 
>>>>>>> veneers have
>>>>>>> +     to be allocated to the same memory region as the code they 
>>>>>>> are refered
>>>>>>> +     by, i.e. they cannot be moved to a subsequent memory 
>>>>>>> region.  */
>>>>>>> +  bool veneer;
>>>>>>
>>>>>> Why would you put a bool between two pointer-sized fields, thus 
>>>>>> introducing
>>>>>> yet more padding, when in fact existing padding could be used. 
>>>>>> There's a set
>>>>>> of bitfields further up from here, and you could simply add a 
>>>>>> single-bit
>>>>>> field there, for example. Generic code changes are okay with that 
>>>>>> change,
>>>>>> but arch-specific ones will need arch maintainer approval.
>>>>>
>>>>> Here is the new attribute as a flag:
>>>>>
>>>>> diff --git a/bfd/section.c b/bfd/section.c
>>>>> index 5f0cf6e71cb..f110ed77363 100644
>>>>> --- a/bfd/section.c
>>>>> +++ b/bfd/section.c
>>>>> @@ -379,6 +379,12 @@ CODE_FRAGMENT
>>>>>     .     when memory read flag isn't set. *}
>>>>>     .#define SEC_COFF_NOREAD            0x40000000
>>>>>     .
>>>>> +.  {* Indicate that the section contains branch veneers.  This is 
>>>>> used when
>>>>> +.     support for non-contiguous memory regions is enabled.  The
>>>>> veneers have
>>>>> +.     to be allocated to the same memory region as the code they are
>>>>> refered
>>>>> +.     by, i.e. they cannot be moved to a subsequent memory 
>>>>> region.  *}
>>>>> +.#define SEC_VENEER                 0x80000000
>>>>> +.
>>>>>     .  {*  End of section flags.  *}
>>>>>     .
>>>>>     .  {* Some internal packed boolean fields.  *}
>>>>>
>>>>>
>>>>> With the tagging of the stub section on AArch64:
>>>>>
>>>>> diff --git a/ld/emultempl/aarch64elf.em b/ld/emultempl/aarch64elf.em
>>>>> index 91d58d8fe5a..b00c9460da2 100644
>>>>> --- a/ld/emultempl/aarch64elf.em
>>>>> +++ b/ld/emultempl/aarch64elf.em
>>>>> @@ -193,7 +193,7 @@ elf${ELFSIZE}_aarch64_add_stub_section (const char
>>>>> *stub_sec_name,
>>>>>       lang_output_section_statement_type *os;
>>>>>       struct hook_stub_info info;
>>>>>
>>>>> -  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
>>>>> +  flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE | 
>>>>> SEC_VENEER
>>>>>               | SEC_HAS_CONTENTS | SEC_RELOC | SEC_IN_MEMORY | 
>>>>> SEC_KEEP);
>>>>>       stub_sec = bfd_make_section_anyway_with_flags (stub_file- 
>>>>> >the_bfd,
>>>>>                                                     stub_sec_name, 
>>>>> flags);
>>>>>
>>>>> Same thing on AArch32 and PowerPC.
>>>>>
>>>>> Is this what you suggested ?
>>>>
>>>> No, sorry. I said "bit field", not "section flag". I'd rather not 
>>>> see you
>>>> use the last available section flag. Right below where you put the new
>>>> #define is a comment "Some internal packed boolean fields" - that's 
>>>> where
>>>> I thought your new flag would go.
>>>
>>> What about the below ?
>>
>> Yes, with one cosmetic request though:
>>
>>> diff --git a/bfd/section.c b/bfd/section.c
>>> index 5f0cf6e71cb..424e53029d9 100644
>>> --- a/bfd/section.c
>>> +++ b/bfd/section.c
>>> @@ -428,6 +428,12 @@ CODE_FRAGMENT
>>>    .  {* Nonzero if section contents should not be freed.  *}
>>>    .  unsigned int alloced:1;
>>>    .
>>> +.  {* Indicate that the section contains branch veneers.  This is 
>>> used when
>>> +.     support for non-contiguous memory regions is enabled.  The
>>> veneers have
>>> +.     to be allocated to the same memory region as the code they are
>>> refered
>>> +.     by, i.e. they cannot be moved to a subsequent memory region.  *}
>>> +.  unsigned int veneer : 1;
>>> +.
>>>    .  {* Bits used by various backends.  The generic code doesn't touch
>>>    .     these fields.  *}
>>>    .
>>> @@ -723,11 +729,11 @@ INTERNAL
>>>    .  {* segment_mark, sec_info_type, use_rela_p, mmapped_p, alloced,
>>> *}        \
>>>    .     0,            0,             0,          0,         
>>> 0,           \
>>>    .                                                                      \
>>> -.  {* sec_flg0, sec_flg1, sec_flg2, sec_flg3, sec_flg4, sec_flg5,    *}
>>
>> Please can these all stay on one line, while you ...
>>
>>>          \
>>> +.  {* veneer, sec_flg0, sec_flg1, sec_flg2, sec_flg3, sec_flg4,      *}
>>
>> ... add a new line to put the new initializer on? (Overall the macro 
>> likely
>> wants switching to C99 dedicated initializers, at which point the 
>> comments
>> can all go away, as can the many 0s and NULL-s. But that's for another 
>> day,
>> I guess.)
>>
>> Jan
> 
> Fixed in the next revision.
> 
> Matthieu

Sorry Jan, I sent the new revision but forgot to add you in CC.

Matthieu