[PATCH v8 09/19] bfd: parse Object Attributes v2's section in input object files

[Jan Beulich]

On 26.08.2025 19:19, Matthieu Longo wrote:
> On 2025-08-15 13:38, Jan Beulich wrote:
>> On 15.08.2025 13:25, Matthieu Longo wrote:
>>> On 2025-08-08 14:03, Jan Beulich wrote:
>>>> On 15.07.2025 13:39, Matthieu Longo wrote:
>>>>> --- a/bfd/elf-attrs.c
>>>>> +++ b/bfd/elf-attrs.c
>>>>> @@ -835,7 +835,7 @@ _bfd_elf_obj_attrs_arg_type (bfd *abfd,
>>>>>    }
>>>>>    
>>>>>    static void
>>>>> -bfd_elf_parse_attr_section_v1 (bfd *abfd, bfd_byte *p, bfd_byte *p_end)
>>>>> +oav1_parse_section (bfd *abfd, bfd_byte *p, bfd_byte *p_end)
>>>>>    {
>>>>>      const char *std_sec = get_elf_backend_data (abfd)->obj_attrs_vendor;
>>>>>    
>>>>> @@ -954,7 +954,217 @@ bfd_elf_parse_attr_section_v1 (bfd *abfd, bfd_byte *p, bfd_byte *p_end)
>>>>>        }
>>>>>    }
>>>>>    
>>>>> -/* Parse an object attributes section.  */
>>>>> +/* A helper struct for parsing, which returns the parsed object, the number of
>>>>> +   bytes read, and whether or not an error occurred.  */
>>>>> +typedef struct {
>>>>> +  /* Was an error met during parsing.  */
>>>>> +  bool err;
>>>>> +  /* How many bytes were read ? (until error if an error occurred)  */
>>>>> +  uint64_t read;
>>>>> +  /* The parsed object.  */
>>>>> +  void *object;
>>>>> +} BufferReadOp_t ;
>>>>> +
>>>>> +#define READ_ULEB128(abfd, var, cursor, end, op)		\
>>>>> +  do								\
>>>>> +    {								\
>>>>> +      bfd_byte *_begin = cursor;				\
>>>>> +      (var) = _bfd_safe_read_leb128 (abfd, &cursor, false, end);\
>>>>
>>>> It's pretty odd that you parenthesize "var" here (which must be an lvalue,
>>>> and hence is more limited as to what can be passed in), but not "cursor",
>>>> nor ...
>>>
>>> I surrounded all parameters with parenthesis.
>>
>> I hope you don't mean this literally. In the function invocation above,
>> "end" doesn't need parenthesizing, just to give an example.
> 
> I haven't surrounded "abfd" and "end".
> 
> Regarding this macro, it is the only one I kept in the next revision, 
> the reason being the possibly different type for "var".
> 
> The current implementation of _bfd_safe_read_leb128 does not return an 
> error if the 8th bit is set while meeting the end condition "data < 
> end". The error checking could be improved, but this is old code that is 
> used in more than 50 places. Same comment can be done for 
> _bfd_read_unsigned_leb128 and _bfd_read_signed_leb128 that don't return 
> any error.
> 
> Even if the current implementation is not perfect and might accept 
> malformed ULEB128, it is not worse than the current situation.
> Please, could we consider fixing the signature of those functions to 
> return an error out of scope of this patch series ?

Hmm, I'm not a fan of seeing new code go in with known shortcomings. If
this is to be an option, such shortcomings would at the very least need
to be prominently named, so people are aware that they've been knowingly
left there.

Jan