[PATCH v2] bfd: Add bfd_unrecoverable_error for unrecoverable errors

[Jan Beulich]

On 28.10.2025 10:57, H.J. Lu wrote:
> 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.

How do they not? An archive can similarly be matched without reading in the
full map. It similarly has a file signature. (Additionally, how big would a
map need to be to run a halfway reasonable system out of memory? I'd kind
of guess there is more being read in than just the map.)

Jan