[PATCH v3 3/5] objdump: Add variable types and structure for DWARF info parsing
Guillaume VACHERIAS - foss
guillaume.vacherias@foss.st.com
Wed Sep 3 12:28:09 GMT 2025
On 01.09.2025 19:50, Guillaume VACHERIAS wrote:
> --- a/binutils/dwarf.c
> +++ b/binutils/dwarf.c
> @@ -65,6 +65,19 @@ static int need_base_address;
> static unsigned int num_debug_info_entries = 0;
> static unsigned int alloc_num_debug_info_entries = 0;
> static debug_info *debug_information = NULL;
> +
> +static base_type *base_type_list = NULL;
> +static type_def *type_def_list = NULL;
> +static enum_type *enum_type_list = NULL;
> +static tab_type *tab_type_list = NULL;
> +static member_parent *struct_type_list = NULL;
> +static member_parent *union_type_list = NULL;
> +static type_ref *ptr_type_list = NULL;
> +static type_ref *const_type_list = NULL;
> +static type_ref *volatile_type_list = NULL;
> +static variable_type *variable_type_list = NULL;
> +
> +static int is_union = false;
bool (and perhaps also followed by another blank line)?
> @@ -1192,6 +1205,21 @@ get_location_expression (int die_tag,
> {
> case DW_OP_addr:
> SAFE_BYTE_GET_AND_INC (uvalue, data, pointer_size, end);
> + if (die_tag == DW_TAG_variable && attribute == DW_AT_location)
> + {
> + if (variable_type_list == NULL)
> + {
> + fprintf (stderr,
> + _ ("m_var_map for DW_OP_addr should not be NULL!\n"));
What is a user supposed to take from this message? Also unlike function
calls or other macro invocations, _() wants to have no blank ahead of
the opening parenthesis.
This should be an assert instead of displaying information to user.
I'll change it this way in v4.
> static void
> insert_element_in_list (enum dwarf_tag dw_tag,
> - enum dwarf_attribute dw_attr ATTRIBUTE_UNUSED,
> - uint64_t die_offset ATTRIBUTE_UNUSED,
> - uint64_t *uvalue ATTRIBUTE_UNUSED,
> - int64_t *svalue ATTRIBUTE_UNUSED,
> - const unsigned char *data ATTRIBUTE_UNUSED)
> -{
> - /* TODO: Retrieve attributes information from
> - below dwarf entries tag. */
> + enum dwarf_attribute dw_attr,
> + uint64_t die_offset,
> + uint64_t *uvalue,
> + int64_t *svalue,
These two are inputs afaics - make them pointer-to-const?
> + const unsigned char *data)
All uses of "data" that I can spot cast the value to const char *. Why
is the parameter type not const char * or maybe const void *? Avoiding
casts wherever possible is a worthwhile goal.
Will change the argument to const char * data in v4.
> +{
> switch (dw_tag)
> {
> /* Base type. */
> case DW_TAG_base_type:
> + {
> + base_type *base = get_or_create_base_type (die_offset);
> + if (dw_attr == DW_AT_byte_size)
> + {
> + assert (uvalue != NULL || svalue != NULL);
> + base->usize = ((uvalue != NULL) ? *uvalue : 0);
> + base->ssize = ((svalue != NULL) ? *svalue : 0);
> + }
> + else if (dw_attr == DW_AT_name)
> + {
> + assert (uvalue != NULL || data != NULL);
> + if (data != NULL)
> + base->name = (const char *) data;
> + else if (uvalue != NULL)
> + base->name = (const char *) fetch_indirect_string (*uvalue);
I think I need to make a patch to eliminate the need for such casts.
> + }
> + }
> break;
> /* typedef type. */
> case DW_TAG_typedef:
> + {
> + type_def *type = get_or_create_type_def (die_offset);
> + if (dw_attr == DW_AT_type)
> + {
> + assert (uvalue != NULL);
> + type->ptr_die_offset = *uvalue;
> + }
> + else if (dw_attr == DW_AT_name)
> + {
> + assert (uvalue != NULL || data != NULL);
> + if (data != NULL)
> + type->name = (const char *) data;
> + else if (uvalue != NULL)
> + type->name = (const char *) fetch_indirect_string (*uvalue);
> + }
> + }
> break;
> /* enumeration type. */
> case DW_TAG_enumerator:
> + {
> + enum_constant *head = get_or_create_enum_constant (
> + enum_type_list, die_offset);
> + if (dw_attr == DW_AT_name)
> + {
> + assert (data != NULL || uvalue != NULL);
> + if (data != NULL)
> + head->name = (const char *) data;
> + else if (uvalue != NULL)
> + head->name = (const char *) fetch_indirect_string (*uvalue);
> + }
> + else if (dw_attr == DW_AT_const_value)
> + {
> + assert (uvalue != NULL);
> + head->value = *uvalue;
> + }
> + }
> break;
> case DW_TAG_enumeration_type:
> + {
> + enum_type *head = get_or_create_enum_type (die_offset);
> + if (dw_attr == DW_AT_name)
> + {
> + assert (uvalue != NULL || data != NULL);
> + if (data != NULL)
> + head->name = (const char *) data;
> + else if (uvalue != NULL)
> + head->name = (const char *) fetch_indirect_string (*uvalue);
> + }
> + else if (dw_attr == DW_AT_byte_size)
> + {
> + assert (uvalue != NULL || svalue != NULL);
> + head->usize = ((uvalue != NULL) ? *uvalue : 0);
> + head->ssize = ((svalue != NULL) ? *svalue : 0);
> + }
> + }
> break;
> /* Array type. */
> case DW_TAG_subrange_type:
> + {
> + subrange_type *head = get_or_create_subrange_type (tab_type_list,
> + die_offset);
> + if (dw_attr == DW_AT_upper_bound || dw_attr == DW_AT_count)
> + {
> + assert (uvalue != NULL || svalue != NULL);
> + if (dw_attr == DW_AT_upper_bound)
> + {
> + head->ssize = ((svalue != NULL) ? *svalue + 1 : 0);
> + head->usize = ((uvalue != NULL) ? *uvalue + 1 : 0);
> + }
> + else if (dw_attr == DW_AT_count)
> + {
> + head->ssize = ((svalue != NULL) ? *svalue : 0);
> + head->usize = ((uvalue != NULL) ? *uvalue : 0);
> + }
> + }
> + }
> break;
> case DW_TAG_array_type:
> + {
> + tab_type *head = get_or_create_tab (die_offset);
> + if (dw_attr == DW_AT_type)
> + {
> + assert (uvalue != NULL);
> + head->ptr_die_offset = *uvalue;
> + }
> + }
> break;
> /* Member type. */
> case DW_TAG_member:
> + {
> + member_type *member = NULL;
> + if (is_union)
> + {
> + member = get_or_create_member_type (union_type_list->members,
> + die_offset);
> + if (union_type_list->members == NULL
> + || union_type_list->members->die_offset != die_offset)
> + {
> + member->next = union_type_list->members;
> + union_type_list->members = member;
> + }
> + else
> + union_type_list->members = member;
> + }
> + else
> + {
> + member = get_or_create_member_type (struct_type_list->members,
> + die_offset);
> + if (struct_type_list->members == NULL
> + || struct_type_list->members->die_offset != die_offset)
> + {
> + member->next = struct_type_list->members;
> + struct_type_list->members = member;
> + }
> + else
> + struct_type_list->members = member;
> + }
> + if (dw_attr == DW_AT_name)
> + {
> + assert (uvalue != NULL || data != NULL);
> + if (data != NULL)
> + member->name = (const char *) data;
> + else if (uvalue != NULL)
> + member->name = (const char *) fetch_indirect_string (*uvalue);
> + }
> + else if (dw_attr == DW_AT_type)
> + {
> + assert (uvalue != NULL);
> + member->ptr_die_offset = *uvalue;
> + }
> + else if (dw_attr == DW_AT_data_member_location)
> + {
> + assert (svalue != NULL || uvalue != NULL);
> + if (svalue != NULL)
> + member->member_offset = *svalue;
> + if (uvalue != NULL)
> + member->member_offset = *uvalue;
> + }
> + }
> break;
> /* Structure type. */
> case DW_TAG_structure_type:
> /* Union type. */
> case DW_TAG_union_type:
> + {
> + member_parent *head = NULL;
> + if (dw_tag == DW_TAG_union_type)
> + {
> + is_union = true;
> + head = get_or_create_member_parent (&union_type_list, die_offset);
> + }
> + else if (dw_tag == DW_TAG_structure_type)
> + {
> + is_union = false;
> + head = get_or_create_member_parent (&struct_type_list,
> + die_offset);
> + }
The way this is written (wrongly afaics) suggests that is_union is consumed
by get_or_create_member_parent(). Question is anyway why is_union needs to be
a global var. A comment next to the variable definition may help.
Since DW_TAG_member is preceded by either DW_TAG_structure or DW_TAG_union,
The created member_type element holding its information should either be going in
the struct_type_list or the union_type_list depending on which DW_TAG seen before.
As in why it is a global var is because this would be the minimum changes needed to
handle this case. But you're right passing is_union as a parameter of insert_element_in_list()
would be clearer to understand. I will do this in v4 !
Jan
Thanks again !
Guillaume.
More information about the Binutils
mailing list