[PATCH v12 06/25] readelf: dump Object Attributes v2

Matthieu Longo matthieu.longo@arm.com
Wed Jan 21 11:33:47 GMT 2026


On 21/01/2026 10:40, Jan Beulich wrote:
> On 20.01.2026 17:25, Matthieu Longo wrote:
>> +static const unsigned char *
>> +display_attr_v2 (const unsigned char *cursor,
>> +		 const unsigned char *const end,
>> +		 const oav2_known_subsection_t *subsec_info,
>> +		 obj_attr_encoding_v2_t value_encoding,
>> +		 display_arch_attr_t display_arch_attr)
>> +{
>> +  obj_attr_tag_t tag;
>> +  READ_ULEB (tag, cursor, end);
>> +
>> +  const oav2_known_tag_t *tag_info = NULL;
>> +  if (subsec_info != NULL)
>> +    tag_info = oav2_identify_tag (subsec_info, tag);
>> +
>> +  if (tag_info != NULL)
>> +    return display_arch_attr (cursor, end, tag_info, value_encoding);
>> +
>> +  printf ("    Tag_unknown_%lu:	", tag);
> 
> Why %lu? Prior to this series obj_attr_tag_t is a typedef of uint32_t, and
> I don't expect you alter it to unsigned long (if anything I guess you
> might be widening it to uint64_t)?
> 

Yes, this was done following up a comment in v9 in [PATCH v9 04/19] gas: implement parsing of object attributes v2.

On 06/11/2025 17:05, Jan Beulich wrote:
>>>> +typedef union {
>>>> +  const char *string;
>>>> +#if (TC_OBJ_ATTR_v1)
>>>> +  uint32_t u32;
>>>> +#endif
>>> How does v1 come into play here?
>>>
>> There is no good reason to keep this.
>> The tag is a uint32_t so I guess that at the beginning, I wanted to save
>> the parsed value as a uint32_t. Then I moved everything to use uint64_t,
>> keys and associated unsigned integer values.
>>
>> Since it is inconsistent, I propose you to move the definition of
>> obj_attr_tag_t to uint64_t with the following changes (at the HEAD of my
>> branch, so the diff will be splitted to the respective patches).
> Hmm, it didn't even occur to me that obj_attr_tag_t would be involved here.
> But yes, if that's what it takes to make things consistent, so be it.

I will change %lu to PRIu64.

>> +static BufferReadOp_t
>> +elf_parse_attrs_subsection_v2 (const unsigned char *cursor,
>> +			       const uint64_t max_read,
>> +			       const char *public_name,
>> +			       display_arch_attr_t display_arch_attr)
>> +{
>> +  BufferReadOp_t op = { .err = false, .read = 0 };
>> +
>> +  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;
>> +
>> +  /* Handle cases where the attributes data is not strictly valid (e.g. due to
>> +     fuzzing).  */
>> +  if (max_read < F_MIN_SUBSECTION_DATA_LEN)
>> +    {
>> +      error (_("Object attributes section ends prematurely\n"));
>> +      return op;
>> +    }
>> +
>> +  unsigned int subsection_len = byte_get (cursor, F_SUBSECTION_LEN);
>> +  cursor += F_SUBSECTION_LEN;
>> +  op.read += F_SUBSECTION_LEN;
>> +  if (subsection_len > max_read)
>> +    {
>> +      error (_("Bad subsection length: too big (%u > max=%lu)\n"),
> 
> Why (still) %lu when max_read is uint64_t?
> 

Changed to PRIu64.

>> +	     subsection_len, max_read);
>> +      /* Error, but still try to display the content until meeting a more
>> +	 serious error.  */
>> +      subsection_len = max_read;
>> +      op.err = true;
>> +    }
>> +  else if (subsection_len < F_MIN_SUBSECTION_DATA_LEN)
>> +    {
>> +      error (_("Bad subsection length: too small (%u < min=%u)\n"),
>> +	     subsection_len, F_MIN_SUBSECTION_DATA_LEN);
> 
> This again is %u vs the variable's (really: constant's) type being uint32_t
> (for not really an apparent reason, like the other F_* as well).
> 

Changed to PRIu32.

> Okay with at least all the format character issues addressed.
> 
> Jan

Matthieu


More information about the Binutils mailing list