[PATCH 10/11] Display type, size and address of all existing global variables

[Jan Beulich]

On 22.05.2025 13:03, Guillaume VACHERIAS wrote:
> --- a/binutils/dwarf.c
> +++ b/binutils/dwarf.c
> @@ -13952,6 +13952,138 @@ struct dwarf_section_display debug_displays[] =
>  /* A static assertion.  */
>  extern int debug_displays_assert[ARRAY_SIZE (debug_displays) == max ? 1 : -1];
>  
> +static field*
> +reverse_field (field *head)
> +{
> +  field *prev = NULL;
> +  field *current = head;
> +  field *next = NULL;
> +
> +  while (current != NULL)
> +    {
> +      next = current->next;
> +      current->next = prev;
> +      prev = current;
> +      current = next;
> +    }
> +  return prev;
> +}

What is this function (supposed to be) doing? And why does it need calling
_after_ output was done? And then why not for enumerations?

> +void
> +display_var_type (void)
> +{
> +  if (m_var_map == NULL)
> +    return;
> +
> +  variable_pair *i = m_var_map;
> +  while (i != NULL)
> +    {
> +      variable *var = i->var;
> +      printf ("0x%lx 0x%lx %s (%s)\n",
> +	      var->location, var->location + var->size,
> +	      var->name, var->type_name);
> +
> +      if (var->array_type != NULL)
> +	{
> +	  for (uint64_t j = 0; j < var->array_type->counts; j++)
> +	    {
> +	      printf ("\t0x%lx %s[%ld]\n",
> +	       var->location + (var->size / var->array_type->counts * j),
> +	       var->name, j);
> +
> +	  if (var->struct_type != NULL)
> +	    {

Already earlier in the series there were indentation issues, but this one
is particularly gross - it leaves unclear whether the earlier loop was
really meant to be ended already. Please go through the entire series and
sort all mis-indentation.

> +	      printf ("\t%s {\n", var->struct_type->name);
> +	      var->struct_type->fields = reverse_field (
> +		var->struct_type->fields);
> +	      field *i_field = var->struct_type->fields;
> +	      while (i_field != NULL)
> +		{
> +		  printf ("\t\t0x%lx %s.%s\n",
> +		    var->location + (var->size / var->array_type->counts * j)
> +		    + i_field->member_location,
> +		    var->struct_type->name, i_field->name);
> +		  i_field = i_field->next;
> +		}
> +	      var->struct_type->fields = reverse_field (
> +		var->struct_type->fields);
> +	      printf ("\t}\n");
> +	    }
> +	  if (var->union_type != NULL)
> +	    {
> +	      printf ("\tunion type {\n");
> +	      var->union_type->fields = reverse_field (var->union_type->fields);
> +	      field *i_field = var->union_type->fields;
> +	      while (i_field != NULL)
> +		{
> +		  printf ("\t\t0x%lx %s\n",
> +		    var->location + (var->size / var->array_type->counts * j)
> +		    + i_field->member_location, i_field->name);
> +		  i_field = i_field->next;
> +		}
> +	      var->union_type->fields = reverse_field (var->union_type->fields);
> +	      printf ("\t}\n");
> +	    }
> +	  if (var->enum_type != NULL)
> +	    {
> +	      printf ("\tenumeration type {\n");
> +	      enum_constant *i_enum = var->enum_type;
> +	      while (i_enum != NULL)
> +		{
> +		  printf ("\t\t0x%lx %s %ld\n",
> +		    var->location + (var->size / var->array_type->counts * j),
> +		    i_enum->name, i_enum->value);
> +		  i_enum = i_enum->next;
> +		}
> +	      printf ("\t}\n");
> +	    }
> +	    }

This, btw, is another indication of there being something fishy with indentation.
Iirc I saw this same pattern at least once more earlier in the series.

> +	}
> +      else if (var->struct_type != NULL)
> +	{
> +	  printf ("\t%s {\n", var->struct_type->name);
> +	  var->struct_type->fields = reverse_field (var->struct_type->fields);
> +	  field *i_field = var->struct_type->fields;
> +	  while (i_field != NULL)
> +	    {
> +	      printf ("\t\t0x%lx %s.%s\n",
> +		var->location + i_field->member_location,
> +		var->struct_type->name, i_field->name);
> +	      i_field = i_field->next;
> +	    }
> +	  var->struct_type->fields = reverse_field (var->struct_type->fields);
> +	  printf ("\t}\n");
> +	}
> +      else if (var->union_type != NULL)
> +	{
> +	  printf ("\tunion type {\n");
> +	  var->union_type->fields = reverse_field (var->union_type->fields);
> +	  field *i_field = var->union_type->fields;
> +	  while (i_field != NULL)
> +	    {
> +	      printf ("\t\t0x%lx %s\n",
> +		var->location + i_field->member_location, i_field->name);
> +	      i_field = i_field->next;
> +	    }
> +	  var->union_type->fields = reverse_field (var->union_type->fields);
> +	  printf ("\t}\n");
> +	}
> +      else if (var->enum_type != NULL)
> +	{
> +	  printf ("\tenumeration type {\n");
> +	  enum_constant *i_enum = var->enum_type;
> +	  while (i_enum != NULL)
> +	    {
> +	      printf ("\t\t0x%lx %s %ld\n",
> +		var->location, i_enum->name, i_enum->value);
> +	      i_enum = i_enum->next;
> +	    }
> +	  printf ("\t}\n");
> +	}
> +      i = i->next;
> +   }
> +}

Handling struct / union / enum separately twice made me look more closely:
This kind of handling almost certainly wants to use recursion. Right now,
afaict, you wouldn't properly handle e.g. a struct containing an array, or
a struct containing another one.

Jan