[PATCH v9 06/19] bfd: write Object Attributes v2

[Matthieu Longo]

On 30/10/2025 15:55, Jan Beulich wrote:
> On 01.09.2025 18:56, Matthieu Longo wrote:
>> --- a/bfd/elf-attrs.c
>> +++ b/bfd/elf-attrs.c
>> @@ -131,13 +131,61 @@ vendor_obj_attrs_v1_size (bfd *abfd, int vendor)
>>   }
>>   
>>   static bfd_vma
>> -bfd_elf_obj_attrs_v1_size (bfd *abfd)
>> +oav1_section_size (bfd *abfd)
>>   {
>>     bfd_vma size = 0;
>>     size = vendor_obj_attrs_v1_size (abfd, OBJ_ATTR_PROC);
>>     size += vendor_obj_attrs_v1_size (abfd, OBJ_ATTR_GNU);
>>     if (size > 0)
>> -    size += sizeof(uint8_t); /* <format-version: ‘A’>  */
>> +    size += sizeof (uint8_t); /* <format-version: uint8>  */
>> +  return size;
>> +}
>> +
>> +/* Return the size of a single attribute.  */
>> +static bfd_vma
>> +oav2_attr_size (const obj_attr_v2 *attr, obj_attr_encoding_v2 type)
> 
> With the const here, ...
> 
>> +{
>> +  bfd_vma size = uleb128_size (attr->tag);
>> +  switch (type)
>> +    {
>> +    case OA_ENC_ULEB128:
>> +      size += uleb128_size (attr->val.uint_val);
>> +      break;
>> +    case OA_ENC_NTBS:
>> +      size += strlen (attr->val.string_val) + 1; /* +1 for '\0'.  */
>> +      break;
>> +    default:
>> +      abort ();
>> +    }
>> +  return size;
>> +}
>> +
>> +/* Return the size of a subsection.  */
>> +static bfd_vma
>> +oav2_subsection_size (const obj_attr_subsection_v2 *subsec)
>> +{
>> +  bfd_vma size = sizeof (uint32_t); /* <subsection-length: uint32>  */
>> +  size += strlen (subsec->name) + 1; /* <subsection-name: NTBS>  so +1 for '\0'.  */
>> +  size += 2 * sizeof (uint8_t); /* <optional: uint8> <encoding: uint8>  */
>> +  /* <attribute>*  */
>> +  for (obj_attr_v2 *attr = subsec->first;
> 
> ... please also use it e.g. here.
> 

Fixed in the next revision.

>> +       attr != NULL;
>> +       attr = attr->next)
>> +    size += oav2_attr_size (attr, subsec->encoding);
>> +  return size;
>> +}
>> +
>> +/* Return the size of a build attributes section.  */
>> +static bfd_vma
>> +oav2_section_size (bfd *abfd)
>> +{
>> +  const obj_attr_subsection_v2 *subsec = elf_obj_attr_subsections (abfd).first;
>> +  if (subsec == NULL)
>> +    return 0;
>> +
>> +  bfd_vma size = sizeof (uint8_t); /* <format-version: uint8>  */
>> +  for (; subsec != NULL; subsec = subsec->next)
>> +    size += oav2_subsection_size (subsec);
>>     return size;
>>   }
>>   
>> @@ -145,7 +193,15 @@ bfd_elf_obj_attrs_v1_size (bfd *abfd)
>>   bfd_vma
>>   bfd_elf_obj_attr_size (bfd *abfd)
>>   {
>> -  return bfd_elf_obj_attrs_v1_size (abfd);
>> +  obj_attr_version_t version = elf_obj_attr_version (abfd);
>> +  if (version == OBJ_ATTR_V1)
>> +    return oav1_section_size (abfd);
>> +  else if (version == OBJ_ATTR_V2)
>> +    return oav2_section_size (abfd);
>> +  else if (version == OBJ_ATTR_VERSION_NONE)
>> +    return 0;
>> +  else
>> +    abort ();
>>   }
> 
> Better use switch() in such a case?
> 

Fixed in the next revision.

>> @@ -227,8 +283,8 @@ write_vendor_obj_attrs_v1 (bfd *abfd, bfd_byte *contents, bfd_vma size,
>>       p = write_obj_attr_v1 (p, list->tag, &list->attr);
>>   }
>>   
>> -static void
>> -write_obj_attr_section_v1 (bfd *abfd, bfd_byte *buffer, bfd_vma size)
>> +static bfd_vma
>> +oav1_write_section (bfd *abfd, bfd_byte *buffer, bfd_vma size)
>>   {
>>     bfd_byte *p = buffer;
> 
> Why the change in return type? The sole caller doesn't use the new return value.
> 

Good question...
I don't know. I will revert it.

>> @@ -246,13 +302,126 @@ write_obj_attr_section_v1 (bfd *abfd, bfd_byte *buffer, bfd_vma size)
>>   
>>     /* We didn't overrun the buffer.  */
>>     BFD_ASSERT (p <= buffer + size);
>> +  return p - buffer;
>> +}
>> +
>> +static bfd_byte *
>> +oav2_write_attr (bfd_byte *p,
>> +		 const obj_attr_v2 *attr,
>> +		 obj_attr_encoding_v2 type)
>> +{
>> +  p = write_uleb128 (p, attr->tag);
>> +  switch (type)
>> +    {
>> +    case OA_ENC_ULEB128:
>> +      p = write_uleb128 (p, attr->val.uint_val);
>> +      break;
>> +    case OA_ENC_NTBS:
>> +      {
>> +	size_t len = strlen (attr->val.string_val) + 1; /* +1 for '\0'.  */
>> +	memcpy (p, attr->val.string_val, len);
>> +	p += len;
> 
> 	p = stpcpy (p, attr->val.string_val) + 1;
> 
> ?
> 

Fixed in the next revision.

>> +static bfd_byte *
>> +oav2_write_subsection (bfd *abfd,
>> +		       const obj_attr_subsection_v2 *subsec,
>> +		       bfd_byte *p)
>> +{
>> +  /* <subsection-length: uint32>  */
>> +  bfd_vma subsec_size = oav2_subsection_size (subsec);
>> +  bfd_put_32 (abfd, subsec_size, p);
>> +  p += sizeof (uint32_t);
>> +
>> +  /* <vendor-name: NTBS>  */
>> +  size_t vendor_name_size = strlen (subsec->name) + 1; /* +1 for '\0'.  */
>> +  memcpy (p, subsec->name, vendor_name_size);
>> +  p += vendor_name_size;
>> +
>> +  /* -- <vendor-data: bytes> --  */
>> +  /* <optional: uint8>  */
>> +  bfd_put_8 (abfd, subsec->optional, p);
>> +  ++p;
>> +  /* <encoding: uint8>  */
>> +  bfd_put_8 (abfd, obj_attr_encoding_v2_to_u8 (subsec->encoding), p);
>> +  ++p;
>> +  /* <attribute>*  */
>> +  for (obj_attr_v2 *attr = subsec->first; attr != NULL; attr = attr->next)
> 
> Again - const here (and potentially elsewhere) as well please.
> 

Fixed.

>> @@ -1006,7 +1175,7 @@ _bfd_elf_merge_unknown_attribute_list (bfd *ibfd, bfd *obfd)
>>     return result;
>>   }
>>   
>> -/* Create a new object attribute with key TAG and value VALS.
>> +/* Create a new object attribute with key TAG and value VAL.
>>      Return a pointer to it.  */
> 
> Likely belongs in an earlier patch?
> 

Fixed.

>> --- a/gas/write.c
>> +++ b/gas/write.c
>> @@ -1919,6 +1919,8 @@ create_obj_attrs_section (void)
>>     bfd_set_section_flags (s, SEC_READONLY | SEC_DATA);
>>     frag_now_fix ();
>>     char *p = frag_more (size);
>> +
>> +  bfd_elf_obj_attr_finalize_contents (stdoutput);
>>     bfd_elf_set_obj_attr_contents (stdoutput, (bfd_byte *)p, size);
> 
> Why is another export needed from libbfd? Can't bfd_elf_set_obj_attr_contents()
> simply do whatever it takes? The other caller likely needs doing the same anyway?
> 
> Jan

bfd_elf_set_obj_attr_contents() is also used in objcpy, and objcpy 
should not change the order of subsections and attributes, but simply 
make a copy of the data as they were in the original object.
Consequently, I added another handler to perform any pre-serialization work.

Another possibility might be to add a parameter to 
bfd_elf_set_obj_attr_contents() to enable/disable the sorting.

What do you prefer ?

Matthieu