[PATCH v2] bfd: Add bfd_unrecoverable_error for unrecoverable errors

[H.J. Lu]

On Tue, Oct 28, 2025 at 4:11 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 28.10.2025 08:58, H.J. Lu wrote:
> > On Tue, Oct 28, 2025 at 3:55 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 27.10.2025 22:06, H.J. Lu wrote:
> >>> On Tue, Oct 28, 2025 at 12:54 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 18.10.2025 03:03, H.J. Lu wrote:
> >>>>> Add bfd_update_error and update bfd_check_format_matches to handle memory
> >>>>> exhaustion.  This changes linker error message from
> >>>>>
> >>>>> ld:lib/libLLVMTransformUtils.a: file format not recognized; treating
> >>>>> as linker script
> >>>>> ld:lib/libLLVMTransformUtils.a:1: syntax error
> >>>>>
> >>>>> to
> >>>>>
> >>>>> ld: lib/libLLVMTransformUtils.a: file not recognized: memory exhausted
> >>>>>
> >>>>> when linking the 3.1GB clang executable in LLVM 21 debug build with the
> >>>>> 32-bit linker.
> >>>>>
> >>>>> PR ld/33550
> >>>>> * archive.c (bfd_generic_archive_p): Call bfd_update_error if
> >>>>> failed to read armap.
> >>>>> * bfd-in2.h: Regenerated.
> >>>>> * bfd.c (bfd_update_error): New.
> >>>>> * format.c (bfd_check_format_matches): Go out if memory is
> >>>>> exhausted.
> >>>>
> >>>> For this last change, aren't we then at risk of bailing out when really
> >>>> a later target could have matched (without running oom)? Also, why is
> >>>
> >>> When we run out of memory, the current target may or may not
> >>> match.  Everything will go downhill from here.
> >>
> >> Only if no later target would match. Of course things get interesting if
> >> in principle the current target would have matched if there was enough
> >> memory, but a later target can also match. Imo there's a conceptual issue
> >
> > If there are 2 similar matches, it is an error since the linker
> > doesn't know which
> > one should be used.
> >
> >> here: Matching shouldn't consume meaningful amounts of memory. It should
> >> in particular not include the reading in of significant parts of an input
> >> file. In particular for ELF objects that's plain unnecessary, as the ELF
> >> header is all that should be needed to determine whether there is a
> >> "match".
> >>
> >> IOW your change feels more like fiddling with a symptom. Which,
> >> considering the alternative would likely be very intrusive, may indeed be
> >> the best that can be done right now. Yet then imo at least the description
> >> would want to make this apparent.
> >
> > Any suggestions for wordings?
>
> Assuming you agree with what I said in the earlier reply, simply along
> the lines of that reply of mine. E.g. start with "While matching shouldn't
> consume meaningful amounts of memory, in practice it may, as significant
> parts of an input file may be read in and then be processed."
>

The case here is

  if (!BFD_SEND (abfd, _bfd_slurp_armap, (abfd))
      || !BFD_SEND (abfd, _bfd_slurp_extended_name_table, (abfd)))
    {
      if (!bfd_unrecoverable_error (bfd_get_error ()))
        bfd_set_error (bfd_error_wrong_format);
      bfd_release (abfd, bfd_ardata (abfd));
      return NULL;
    }

This check isn't ELF specific.  It runs out of memory while reading
the archive map.   Your comments don't apply here.

-- 
H.J.