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

[Jan Beulich]

On 07.11.2025 11:19, Matthieu Longo wrote:
> On 07/11/2025 08:33, Jan Beulich wrote:
>> On 06.11.2025 14:35, Matthieu Longo wrote:
>>> On 31/10/2025 10:25, Jan Beulich wrote:
>>>> On 01.09.2025 18:56, Matthieu Longo wrote:
>>>>> +  const uint32_t F_SUBSECTION_LEN = sizeof(uint32_t);
>>>>> +  const uint32_t F_SUBSECTION_COMPREHENSION = sizeof(uint8_t);
>>>>> +  const uint32_t F_SUBSECTION_ENCODING = sizeof(uint8_t);
>>>>> +  /* The minimum subsection length is 7: 4 bytes for the length itself, and 1
>>>>> +     byte for an empty NUL-terminated string, 1 byte for the comprehension,
>>>>> +     1 byte for the encoding, and no vendor-data.  */
>>>>> +  const uint32_t F_MIN_SUBSECTION_DATA_LEN
>>>>> +    = F_SUBSECTION_LEN + 1 /* for '\0' */
>>>>> +      + F_SUBSECTION_COMPREHENSION + F_SUBSECTION_ENCODING;
>>>>> +
>>>>> +  /* Similar to the issues reported in PR 17531, we need to check all the sizes
>>>>> +     and offsets as we parse the section.  */
>>>>> +  if (max_read < F_MIN_SUBSECTION_DATA_LEN)
>>>>> +    {
>>>>> +      _bfd_error_handler (_("%pB: error: attributes subsection ends "
>>>>> +			    "prematurely"), abfd);
>>>>> +      goto error;
>>>>> +    }
>>>>> +
>>>>> +  const uint32_t subsection_len = bfd_get_32 (abfd, cursor);
>>>>> +  total_read += F_SUBSECTION_LEN;
>>>>> +  cursor += F_SUBSECTION_LEN;
>>>>> +  if (subsection_len > max_read)
>>>>> +    {
>>>>> +      _bfd_error_handler (_("%pB: error: bad subsection length (%u > max=%lu)"),
>>>>> +			  abfd, subsection_len, max_read);
>>>>> +      goto error;
>>>>> +    }
>>>>> +  else if (subsection_len < F_MIN_SUBSECTION_DATA_LEN)
>>>>> +    {
>>>>> +      _bfd_error_handler (_("%pB: error: subsection length of %u is too small"),
>>>>> +			  abfd, subsection_len);
>>>>> +      goto error;
>>>>> +    }
>>>>> +
>>>>> +  const size_t MAX_SUBSECTION_NAME_LEN
>>>>> +    = subsection_len - F_SUBSECTION_LEN
>>>>> +      - F_SUBSECTION_COMPREHENSION - F_SUBSECTION_ENCODING;
>>>>
>>>> Other all-capitals identifiers are kind-of #define-s of build-time constants.
>>>> This one isn't a constant, though, so the use of such an identifier feels a
>>>> little misleading.
>>>>
>>>
>>> Fixed in the next revision.
>>>
>>>>> +  const size_t subsection_name_len
>>>>> +    = strnlen ((char *) cursor, MAX_SUBSECTION_NAME_LEN);
>>>>
>>>> I think I mentioned earlier that casts would better be used sparingly. Here we
>>>> could get away without, if we used memchr() instead. Thoughts?
>>>
>>> Is it what you had in mind ?
>>
>> Looks like so.
>>
>>>>> +  const char *vendor_name = get_elf_backend_data (abfd)->obj_attrs_vendor;
>>>>> +  obj_attr_subsection_scope_v2 scope
>>>>> +    = (strncmp (subsection_name, vendor_name, strlen (vendor_name)) == 0
>>>>> +      ? OA_SUBSEC_PUBLIC
>>>>> +      : OA_SUBSEC_PRIVATE);
>>>>
>>>> What is this following from? I'm in particular puzzled that there's no form of
>>>> separator required between the part potentially matching the sub-section name
>>>> and the rest. This way a sub-section XYZ would be public for vendor XY, which
>>>> may or may not be what is intended. Personally I would have expected
>>>> separation by e.g. some non-alphanumeric character.
>>>
>>> I discussed that with the person writing the specs, and he does not have
>>> any objection to add a separator between the prefix and the core.
>>>
>>> So, in the next revision, the parser for a subsection directive only
>>> accept [a-zA-Z][a-zA-Z0-9_]* for the subsection name, and '_' is
>>> considered a separator.
>>
>> Is '_' a good character to use as separator? It's pretty commonly used in
>> identifiers. It's also unclear to me why subsection names would need
>> constraining to alphanumeric plus underscore. (Sorry, likely lacking some
>> background here.)
> 
> Whether '_' is a good character or not to use as a separator, we don't 
> have much choice now since Clang has already implemented the feature by 
> following the spec using '_', and already released it. Using '_' instead 
> of '-' was making things easier for their parser from what I understood.
> Even if the name is following the pattern of an identifier, is it really 
> an issue ? What example do you have in mind ?

I have no particular example(s) in mind; it merely strikes me as odd to
artificially limit people in what they may do. See e.g. how symbol names
were constrained originally, until the option to quote them was added.

Jan