[PATCH v3 3/5] objdump: Add variable types and structure for DWARF info parsing

[Jan Beulich]

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.

>  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.

> +{
>    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.

Jan