[PATCH v0 08/15] bfd: parse build attributes v2's section in input object files
Matthieu Longo
matthieu.longo@arm.com
Fri Mar 21 16:52:28 GMT 2025
>
>
First of all, I have an issue with your reply. When I try to reply to
it, it skip all the previous message. I had to copy-paste the content of
the previous email myself.
I am not sure whether it is your email client or the new lines that you
have at the top of your email.
> On 3/10/25 17:51, Matthieu Longo wrote:
>> ---
>> bfd/elf-attrs.c | 214 +++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 212 insertions(+), 2 deletions(-)
>>
>> diff --git a/bfd/elf-attrs.c b/bfd/elf-attrs.c
>> index ab3c80573e5..bd8ae56678a 100644
>> --- a/bfd/elf-attrs.c
>> +++ b/bfd/elf-attrs.c
>> @@ -774,7 +774,214 @@ bfd_elf_parse_attr_section_v1 (bfd *abfd,
>> }
>> }
>>
>> -/* Parse an object attributes section. */
>> +/* An helper struct for parsing returning the parsed object, the number of bytes
>> + read, and whether or not an error occurred. */
>
> Slightly hard comment to understand might be better as:
> A helper struct for parsing, which returns the parsed object, the number of bytes
> read, and whether or not an error occurred.
>
Fixed in the next revision.
>> +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);\
>> + op.read += cursor - _begin; \
>> + } \
>> + while (0)
>> +
>> +#define READ_NTBS(abfd, var, cursor, end, op) \
>> + do \
>> + { \
>> + (var) = strdup ((const char*) cursor); \
>> + size_t read_ = strnlen (var, end - cursor) + 1; \
>> + op.read += read_; \
>> + (cursor) += read_; \
>> + } \
>> + while (0)
>> +
>> +#define READ_UINT8(abfd, var, cursor, end, op) \
>> + do \
>> + { \
>> + (var) = bfd_get_8 (abfd, (cursor)); \
>> + (cursor) += sizeof(uint8_t); \
>> + op.read += sizeof(uint8_t); \
>> + } \
>> + while (0)
>> +
>> +/* Parse an objet attribute (v2 only). */
>
> Missed a letter in object.
>
Fixed in the next revision.
>> +static BufferReadOp_t
>> +bfd_elf_parse_attr_v2 (bfd *abfd,
>> + bfd_byte *cursor,
>> + bfd_byte *const end,
>> + obj_attr_encoding_v2 attr_type)
>> +{
>> + BufferReadOp_t op = { .err = false, .read = 0, .object = NULL };
>> +
>> + uint32_t attr_tag;
>> + READ_ULEB128 (abfd, attr_tag, cursor, end, op);
>> +
>> + union obj_attr_value_v2 vals;
>> + switch (attr_type)
>> + {
>> + case NTBS:
>> + READ_NTBS (abfd, vals.string_val, cursor, end, op);
>> + break;
>> + case ULEB128:
>> + READ_ULEB128 (abfd, vals.uint_val, cursor, end, op);
>> + break;
>> + }
>> +
>> + op.object = _bfd_elf_obj_attr_v2_init (attr_tag, vals)> + return op;
>> +}
>> +
>> +/* Parse a subsection (object attributes v2 only). */
>> +static BufferReadOp_t
>> +bfd_elf_parse_attrs_subsection_v2 (bfd *abfd,
>> + bfd_byte *cursor,
>> + const uint64_t max_read)
>> +{
>> + BufferReadOp_t op = { .err = false, .read = 0, .object = NULL };
>> +
>> + const uint32_t F_SUBSECTION_LEN = sizeof(uint32_t);
>> + /* The minimum subsection length is 5: 4 bytes for the length itself, and 1
>> + byte for an empty NUL-terminated string, and no vendor-data. */
>> + const uint32_t F_MIN_SUBSECTION_DATA_LEN = F_SUBSECTION_LEN + 1;
>> +
>> + if (max_read <= F_SUBSECTION_LEN)
>> + {
>> + _bfd_error_handler (_("%pB: error: attributes subsection ends "
>> + "prematurely"), abfd);
>> + bfd_set_error (bfd_error_malformed_archive);
>> + return op;
>> + }
>> +
>> + uint32_t subsection_len = bfd_get_32 (abfd, cursor);
>> + op.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);
>> + bfd_set_error (bfd_error_malformed_archive);
>> + op.err = true;
>> + return op;
>> + }
>> + /* PR 17531: file: 001-101425-0.004 */
>> + else if (subsection_len < F_MIN_SUBSECTION_DATA_LEN)
>> + {
>> + _bfd_error_handler (_("%pB: error: subsection length of %u is too small"),
>> + abfd, subsection_len);
>> + bfd_set_error (bfd_error_malformed_archive);
>> + op.err = true;
>> + return op;
>> + }
>> +
>> + size_t subsection_name_len = strnlen ((char *) cursor, subsection_len) + 1;
>> + if (subsection_name_len >= subsection_len)
>> + {
>> + _bfd_error_handler (_("%pB: error: subsection name seems corrupted "
>> + "(missing '\\0')"), abfd);
>> + bfd_set_error (bfd_error_malformed_archive);
>> + op.err = true;
>> + return op;
>> + }
>> + /* Note: if the length of the subsection name is 0 (i.e. the string is '\0'),
>> + it is still considered a valid name, even if it is not particularly
>> + usefull. */
>> +
>> + unsigned char * const end = cursor + subsection_len - F_SUBSECTION_LEN;
>> + BFD_ASSERT (cursor < end);
>> +
>> + const char* subsection_name;
>> + READ_NTBS (abfd, subsection_name, cursor, end, op);
>> +
>> + uint8_t optional_raw;
>> + READ_UINT8 (abfd, optional_raw, cursor, end, op);
>> +
>> + if (optional_raw > 1)
>> + {
>> + _bfd_error_handler (_("%pB: error: optional value seems corrupted, got"
>> + " %u but only 0x0 (false) or 0x1 (true) are "
>> + "valid values."), abfd, optional_raw);
>> + bfd_set_error (bfd_error_malformed_archive);
>> + op.err = true;
>> + free ((void*) subsection_name);
>> + return op;
>> + }
>> +
>> + uint8_t attr_type_raw;
>> + READ_UINT8 (abfd, attr_type_raw, cursor, end, op);
>> + if (attr_type_raw > NTBS)
>> + {
>> + _bfd_error_handler (_("%pB: error: attribute type seems corrupted, got"
>> + " %u but only 0x0 (ULEB128) or 0x1 (NTBS) are "
>> + "valid types."), abfd, attr_type_raw);
>> + bfd_set_error (bfd_error_malformed_archive);
>> + op.err = true;
>> + free ((void*) subsection_name);
>> + return op;
>> + }
>> +
>> + 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)
>> + ? SUBSEC_PUBLIC
>> + : SUBSEC_PRIVATE;
>> +
>> + obj_attr_subsection_v2 *subsec =
>> + _bfd_elf_obj_attr_subsection_v2_init (subsection_name, scope, optional_raw,
>> + attr_type_raw);
>> + while (cursor < end)
>> + {
>> + BufferReadOp_t op_ =
>> + bfd_elf_parse_attr_v2 (abfd, cursor, end, attr_type_raw);
>> + LINKED_LIST_APPEND(obj_attr_v2) (subsec, op_.object);
>> + op.read += op_.read;
>> + cursor += op_.read;
>> + }
>
> Maybe a clearer distinction in name between op and op_, it took me a minute to understand how this was working
> because I couldn't immediately tell them apart.
>
The op_ is internal to the while loop, the other is in the whole
function scope. There are exactly the same, so I didn't find a better
way to name them.
It is the same as if I wrote res, _res, res_, res1, ...
To me, all those alternatives look the same and are not really making
the code clearer.
I am happy to change it if you have a suggestion.
>> +
>> + op.object = subsec;
>> +
>> + BFD_ASSERT (cursor == end);
>> +
>> + return op;
>> +}
>> +
>> +/* Parse the list of subsections (object attributes v2 only). */
>> +static void
>> +bfd_elf_parse_attr_section_v2 (bfd *abfd,
>> + Elf_Internal_Shdr * hdr,
>> + bfd_byte *cursor)
>> +{
>> + obj_attr_subsection_list *subsecs = &elf_obj_attr_subsections (abfd);
>> + BufferReadOp_t op;
>> + for (uint64_t remaining = hdr->sh_size - 1; // already read 'A'
>> + remaining > 0;
>> + remaining -= op.read, cursor += op.read)
>> + {
>> + op = bfd_elf_parse_attrs_subsection_v2 (abfd, cursor, remaining);
>> + if (op.err)
>> + {
>> + _bfd_error_handler (_("%pB: error: could not parse subsection at "
>> + "offset %lx"),
>> + abfd, hdr->sh_size - remaining);
>> + bfd_set_error (bfd_error_wrong_format);
>> + break;
>> + }
>> + else
>> + LINKED_LIST_APPEND(obj_attr_subsection_v2) (subsecs,
>> + (obj_attr_subsection_v2 *) op.object);
>> + }
>> +}
>> +
>> +/* Parse an object attributes section.
>> + Note: The parsing setup is common between object attributes v1 and v2. */
>> void
>> _bfd_elf_parse_attributes (bfd *abfd, Elf_Internal_Shdr * hdr)
>> {
>> @@ -812,7 +1019,10 @@ _bfd_elf_parse_attributes (bfd *abfd, Elf_Internal_Shdr * hdr)
>>
>> ++cursor;
>>
>> - bfd_elf_parse_attr_section_v1 (abfd, hdr, cursor);
>> + if (get_elf_backend_data (abfd)->obj_attrs_version == 2)
>> + bfd_elf_parse_attr_section_v2 (abfd, hdr, cursor);
>> + else
>> + bfd_elf_parse_attr_section_v1 (abfd, hdr, cursor);
>>
>> free_data:
>> free (data);
More information about the Binutils
mailing list