check sframe version in _bfd_elf_parse_sframe

[Jens Remus]

On 3/13/2026 12:33 AM, Alan Modra wrote:
> Prompted by ld/bootstrap test failues on Ubuntu 25.10
> ld-new: error in /lib/x86_64-linux-gnu/crt1.o (.sframe); unexpected SFrame format version 2
> ld-new: final link failed
> collect2: error: ld returned 1 exit status

Ouch!

> Currently if you attempt to link an object file with sframe version 2
> contents, the linker will complain in _bfd_elf_merge_section_sframe
> and fail to produce an output.  This affects anyone who has objects
> assembled by binutils-2.45 with .sframe sections.  A work-around is to
> pass --discard-sframe to ld.

Not ideal.

> Ideally the linker would rewrite version 2 sframe input to version 3,
> but in the absence of that support it would be nicer if ld just
> ignored version 2 (or 1) sframe sections without needing to pass
> --discard-sframe.  That is the aim of this patch.
> 
> It does so by checking the sframe version in _bfd_elf_parse_sframe
> where various failing checks result in an error being printed and the
> section not being marked as SEC_INFO_TYPE_SFRAME, but causes no
> overall linker failure.  In addition, an sframe version failure now
> also marks the sframe section with SEC_EXCLUDE.  This is also done for
> other sframe failures.  See the changelog below for other fixes.

Nice!  Thank you for taking care of this, Alan!

> bfd/
> 	* elf-sframe.c (_bfd_elf_merge_section_sframe): Don't check
> 	sframe version here.
> 	(_bfd_elf_parse_sframe): Check sframe version here instead.
> 	Do the "already parsed" check first.  Reorder various other
> 	checks.  Do not bother parsing or print errors when
> 	discard_sframe is set.  Free section contents on failure
> 	paths.  Set SEC_EXCLUDE for fails.  Check for NULL bfd_zalloc
> 	return, and only allocate after successfully parsing sframe.
> 	On errors report that the section will be ignored, not "no
> 	.sframe will be created".
> binutils/
> 	* testsuite/lib/binutils-common.exp: Ignore sframe version
> 	errors.

> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c

> @@ -220,7 +220,7 @@ _bfd_elf_sframe_present (struct bfd_link_info *info)
>  
>  bool
>  _bfd_elf_parse_sframe (bfd *abfd,
> -		       struct bfd_link_info *info ATTRIBUTE_UNUSED,
> +		       struct bfd_link_info *info,
>  		       asection *sec, struct elf_reloc_cookie *cookie)
>  {
>    bfd_byte *sfbuf = NULL;
> @@ -229,74 +229,85 @@ _bfd_elf_parse_sframe (bfd *abfd,
>    bfd_size_type sf_size;
>    int decerr = 0;
>  
> +  /* Check if this section was already parsed.  */
> +  if (sec->sec_info_type == SEC_INFO_TYPE_SFRAME)
> +    return true;
> +
>    if (info->discard_sframe)
> -    sec->flags |= SEC_EXCLUDE;
> +    {
> +      sec->flags |= SEC_EXCLUDE;
> +      return false;
> +    }

The following would accomplish the same, but might confuse readers that
it is a failure?  Maybe come up with better label names than fail1-4?

    goto fail4;

>  
> -  /* Prior versions of assembler and ld were generating SFrame sections with
> -     section type SHT_PROGBITS.  Issue an error for lack of support for such
> -     objects now.  Even if section size is zero, a valid section type is
> -     expected.  */
> -  if (elf_section_type (sec) != SHT_GNU_SFRAME)
> +  if ((sec->flags & SEC_EXCLUDE) != 0
> +      || bfd_is_abs_section (sec->output_section))
>      {
> -      _bfd_error_handler
> -	(_("error in %pB(%pA); unexpected SFrame section type"),
> -	 abfd, sec);
> +      /* This sections is being discarded from the link, ignore it.  */
>        return false;
>      }
>  
>    if (sec->size == 0
>        || (sec->flags & SEC_HAS_CONTENTS) == 0)
>      {
> -      /* This file does not contain .sframe information.  */
> -      return false;
> +      /* This section does not contain .sframe information.  */

Perhaps mention what will happen?

      /* This section does not contain .sframe information.  Discard and ignore it.  */

> +      goto fail4;
>      }
>  
> -  /* Check if this section was already parsed.  */
> -  if (sec->sec_info_type == SEC_INFO_TYPE_SFRAME)
> -    return true;
> -
> -  if (bfd_is_abs_section (sec->output_section))
> +  /* Prior versions of assembler and ld were generating SFrame sections with
> +     section type SHT_PROGBITS.  Issue an error for lack of support for such
> +     objects now.  Even if section size is zero, a valid section type is
> +     expected.  */
> +  if (elf_section_type (sec) != SHT_GNU_SFRAME)
>      {
> -      /* At least one of the sections is being discarded from the
> -	 link, so we should just ignore them.  */
> -      return false;
> +      _bfd_error_handler
> +	(_("error in %pB(%pA); unexpected SFrame section type; section ignored"),
> +	 abfd, sec);
> +      goto fail4;
>      }
>  
>    /* Read the SFrame stack trace information from abfd.  */
>    if (!_bfd_elf_mmap_section_contents (abfd, sec, &sfbuf))
> -    goto fail_no_free;
> +    goto fail3;
>  
>    /* Decode the buffer and keep decoded contents for later use.
>       Relocations are performed later, but are such that the section's
>       size is unaffected.  */
> -  sfd_info = bfd_zalloc (abfd, sizeof (*sfd_info));
>    sf_size = sec->size;
> +  sfd_ctx = sframe_decode ((const char *) sfbuf, sf_size, &decerr);
> +  if (!sfd_ctx)
> +    goto fail2;
>  
> -  sfd_info->sfd_ctx = sframe_decode ((const char*)sfbuf, sf_size, &decerr);

A brief moment I was worried this would re-introduce a previous issue ...

> +  uint8_t dctx_version = sframe_decoder_get_version (sfd_ctx);
> +  if (dctx_version != SFRAME_VERSION)
> +    {
> +      _bfd_error_handler
> +	(_("error in %pB(%pA); unexpected SFrame format version %" PRIu8),
> +	 abfd, sec, dctx_version);
> +      goto fail2;
> +    }
> +
> +  sfd_info = bfd_zalloc (abfd, sizeof (*sfd_info));
> +  if (!sfd_info)
> +    goto fail1;
> +  sfd_info->sfd_ctx = sfd_ctx;

... but you did the right thing. :-)

>    sfd_info->sfd_state = SFRAME_SEC_DECODED;
> -  sfd_ctx = sfd_info->sfd_ctx;
> -  if (!sfd_ctx)
> -    /* Free'ing up any memory held by decoder context is done by
> -       sframe_decode in case of error.  */
> -    goto fail_no_free;
>  
>    if (!sframe_decoder_init_func_bfdinfo (abfd, sec, sfd_info, cookie))
>      {
> +    fail1:
>        sframe_decoder_free (&sfd_info->sfd_ctx);
> -      goto fail_no_free;
> +    fail2:
> +      _bfd_elf_munmap_section_contents (sec, sfbuf);
> +    fail3:
> +      _bfd_error_handler (_("error in %pB(%pA); SFrame section ignored"),
> +			  abfd, sec);
> +    fail4:
> +      sec->flags |= SEC_EXCLUDE;
> +      return false;
>      }
>  
>    sec->sec_info = sfd_info;
>    sec->sec_info_type = SEC_INFO_TYPE_SFRAME;
> -
> -  goto success;
> -
> -fail_no_free:
> -  _bfd_error_handler
> -   (_("error in %pB(%pA); no .sframe will be created"),
> -    abfd, sec);
> -  return false;
> -success:
>    _bfd_elf_munmap_section_contents (sec, sfbuf);
>    return true;
>  }

> diff --git a/binutils/testsuite/lib/binutils-common.exp b/binutils/testsuite/lib/binutils-common.exp

> @@ -792,6 +792,9 @@ proc prune_warnings_extra { text } {
>      regsub -all {(^|\n)([^\n]*lto-wrapper: warning: using serial compilation of [0-9]+ LTRANS jobs[^\n]*\n?)+} $text {\1} text
>      regsub -all {(^|\n)([^\n]*lto-wrapper: note: [^\n]*\n?)+} $text {\1} text
>  
> +    # Ignore warnings about linking objects with an old sframe format
> +    regsub -all {(^|\n)([^\n]* unexpected SFrame format version [^\n]*\n[^\n]* SFrame section ignored\n?)+} $text {\1} text
> +

Nice!  Did not know about that.

>      return $text
>  }

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/