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

[Jan Beulich]

On 06.10.2025 18:50, Guillaume VACHERIAS - foss wrote:
>> On 26.09.2025 14:22, Guillaume VACHERIAS wrote:
>>> @@ -2569,41 +2593,517 @@ display_lang (uint64_t uvalue)
>>>      }
>>>  }
>>>
>>> +static generic_type *
>>> +get_or_create_generic_base_type (uint64_t die_offset)
>>> +{
>>> +  if (base_type_list.tail == NULL
>>> +      || base_type_list.tail->die_type.base_type.die_offset != die_offset)
>>> +    {
>>> +      generic_type *ret = xmalloc (sizeof (*ret));
>>
>> Isn't it potentially pretty wasteful to allocate a generic_type here
>> (and elsewhere) when all you need is a base_type? You may, after all, be
>> allocating very many of them.
> 
> generic_type is easier to manipulate. In the patch 4/5 where the linking
> mechanism is introduced generic_type abstracts the need to determine
> which type is linked to another, letting the linking mechanism be one
> line assignment only in the function do_link_variable_information in patch
> 4/5.

I fear I don't understand this reply, first and foremost the first sentence.
Isn't "easier to manipulate" mainly an issue with data type arrangements here?
I.e. the union approach I had outlined before possibly being more suitable,
making manipulation "easier" in general?

>>> +      base_type bt = {die_offset, 0, 0, NULL};
>>> +      ret->type = BASE_TYPE;
>>> +      ret->die_type.base_type = bt;
>>> +      ret->linked = false;
>>> +      ret->next = NULL;
>>> +      if (base_type_list.head == NULL)
>>> +	{
>>> +	  base_type_list.head = ret;
>>> +	  base_type_list.tail = base_type_list.head;
>>
>> Better write NULL here explicitly (and then similarly elsewhere)?
> 
> You meant ret ? In that case, yes, I'll correct it this way.

Why "ret"? That isn't NULL here. base_type_list.head is.

>> Also the field could be placed better to avoid unnecessary padding holes on
>> (at least) 64-bit architectures (where there already is a padding hole of 4
>> bytes between type and die_type).
>>
>> Before you (and I) put any more work into this, I have to ask a more general
>> question (Cc-ing Nick for this reason): This is a lot of new code (~1.600
>> lines) for something that I'm not really sure belongs in objdump. What you
>> do here goes beyond mere "dumping", imo.
> 
> I understand your concerns regarding the size of the changes, and I agree that
> the functionality does go beyond "dumping" as objdump is typically used for.
> 
> That said, I'm not entirely sure where else this code would best fit within the
> binutils-gdb. In the LLVM ecosystem, this feature would fit in llvm-dwarfdump,
> but as far as I know, there isn't an equivalent standalone utility in GNU. After
> considering the available options, objdump seemed like the most suitable place
> for this functionality as it manipulates DWARF information. However, do not
> hesitate to suggest a more suitable tool in the binutils-gdb suite !

Afaict there's none. There's still the (likely more "expensive") option of adding
a new one ...

Jan