[PATCH 2/4] objdump: Add variable types and structure for DWARF info parsing

[Jan Beulich]

On 19.08.2025 04:12, Guillaume VACHERIAS wrote:
> @@ -2567,13 +2595,206 @@ display_lang (uint64_t uvalue)
>      }
>  }
>  
> +static base_type *
> +get_or_create_base_type (uint64_t die_offset)
> +{
> +  if (base_type_list == NULL || base_type_list->die_offset != die_offset)
> +    {
> +      base_type *ret = (base_type *)
> +	xmalloc (sizeof (base_type));

No need for the line wrapping, and - afaict - also no need for the cast. (Applies
in many other places as well.) Additionally it is imo generally preferable to use
sizeof(<expression>) where possible, to avoid the risk of a variable's types
being changed without changing the type in the sizeof() expression.

> +static enum_constant *
> +get_or_create_enum_constant (enum_type *head, uint64_t die_offset)
> +{
> +  if (head == NULL)
> +    {
> +      fprintf (stderr,
> +	_ ("get_or_create_enum_elt: head should not be NULL!\n"));

If a user gets to see such an error message, what should they do? Imo
you simply mean assert() here. That'll clearly convey that there's a bug
in the tool. (Again applies elsewhere as well.)

> +      free_mapping_info_struct ();
> +      xexit (1);

Alan - what's our general position towards such freeing before exiting
a tool? Overally that's quite a bit of code (most sitting further down),
which for all practical purposes (outside of running analysis tools) is
useless.

> --- a/binutils/dwarf.h
> +++ b/binutils/dwarf.h
> @@ -71,6 +71,154 @@ typedef struct
>  }
>  DWARF2_Internal_ARange;
>  
> +enum types

This and ...

> +{
> +  UNKNOWN = 0,

... this, at the very least, are too generic identifiers to live in a
header file.

> +/* Container for map info information to be generated.  */
> +typedef struct base_type
> +{
> +  uint64_t die_offset;
> +  uint64_t usize;
> +  int64_t ssize;
> +  const unsigned char *name;

unsigned char? Isn't this a normal string, i.e. plain char?

> +
> +  struct base_type *next;
> +}
> +base_type;

Do we need both struct tag and typedef-ed name?

> +typedef struct member_parent
> +{
> +  uint64_t die_offset;
> +  uint64_t usize;
> +  int64_t ssize;
> +  const unsigned char *name;
> +  member_type *members;
> +  bool displayed; /* Treat nested structure.  */
> +
> +  struct member_parent *next;
> +}
> +member_parent;
> +
> +typedef member_parent struct_type;
> +typedef member_parent union_type;

Are these and ...

> +typedef struct type_ref
> +{
> +  uint64_t die_offset;
> +  uint64_t ptr_die_offset;
> +  uint64_t usize;
> +  int64_t ssize;
> +
> +  enum types type;
> +  void *ptr_type;
> +  struct type_ref *next;
> +}
> +type_ref;
> +
> +typedef struct type_ref ptr_type;
> +typedef struct type_ref const_type;
> +typedef struct type_ref volatile_type;

... these aliases really needed? Especially in this latter case sooner
or later you'd likely get more (references, atomics)? IOW this doesn't
look to scale very well here.

Jan