[PATCH] Introduce linked list for dynamic attributes

[PATCH] Introduce linked list for dynamic attributes

[Keven Boell]

This patch introduces a linked list for dynamic
attributes of a type. This is a pre-work for
the Fortran dynamic array support. The Fortran
dynamic array support will add more dynamic
attributes to a type. As only a few types
will have such dynamic attributes set, a linked
list is more efficient in terms of memory
consumption than adding multiple attributes
to main_type.

Transformed the data_location dynamic attribute
to use the linked list.

2015-02-23  Keven Boell  <keven.boell@intel.com>

	* gdbtypes.c (resolve_dynamic_type_internal):
	Adapted data_location usage to linked list.
	(resolve_dynamic_type_internal): Adapted
	data_location usage to linked list.
	(get_dyn_attr): New function.
	(add_dyn_attr): New function.
	(copy_type_recursive): Add copy of linked
	list.
	(copy_type): Add copy of linked list.
	* gdbtypes.h (enum dynamic_prop_kind): Kind
	of the dynamic attribute in linked list.
	(struct dynamic_prop_list): Dynamic list
	node.
	* dwarf2read.c (set_die_type): Add data_location
	data to linked list using helper functions.
---
 gdb/dwarf2read.c |    4 +-
 gdb/gdbtypes.c   |  120 +++++++++++++++++++++++++++++++++++++++++++++++-------
 gdb/gdbtypes.h   |   47 ++++++++++++++++++---
 3 files changed, 148 insertions(+), 23 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index ac78165..9923758 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -22077,9 +22077,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_data_location, cu);
   if (attr_to_dynamic_prop (attr, die, cu, &prop))
     {
-      TYPE_DATA_LOCATION (type)
-        = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
-      *TYPE_DATA_LOCATION (type) = prop;
+      add_dyn_attr (type, objfile, prop, DYN_ATTR_DATA_LOCATION);
     }
 
   if (dwarf2_per_objfile->die_type_hash == NULL)
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index a80151c..65a6897 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2021,7 +2021,7 @@ resolve_dynamic_type_internal (struct type *type,
 {
   struct type *real_type = check_typedef (type);
   struct type *resolved_type = type;
-  const struct dynamic_prop *prop;
+  struct dynamic_prop *prop;
   CORE_ADDR value;
 
   if (!is_dynamic_type_internal (real_type, top_level))
@@ -2078,11 +2078,11 @@ resolve_dynamic_type_internal (struct type *type,
   prop = TYPE_DATA_LOCATION (resolved_type);
   if (dwarf2_evaluate_property (prop, addr_stack, &value))
     {
-      TYPE_DATA_LOCATION_ADDR (resolved_type) = value;
-      TYPE_DATA_LOCATION_KIND (resolved_type) = PROP_CONST;
+      TYPE_DYN_ATTR_ADDR (prop) = value;
+      TYPE_DYN_ATTR_KIND (prop) = PROP_CONST;
     }
   else
-    TYPE_DATA_LOCATION (resolved_type) = NULL;
+    prop = NULL;
 
   return resolved_type;
 }
@@ -2097,6 +2097,48 @@ resolve_dynamic_type (struct type *type, CORE_ADDR addr)
   return resolve_dynamic_type_internal (type, &pinfo, 1);
 }
 
+/* See gdbtypes.h  */
+
+struct dynamic_prop * get_dyn_attr (const struct type * type,
+  enum dynamic_prop_kind kind)
+{
+  struct dynamic_prop_list * head =
+    (TYPE_MAIN_TYPE (type))->dyn_attribs;
+
+  while (head != NULL)
+    {
+      if (head->kind == kind)
+        return head->prop;
+      head = head->next;
+    }
+  return NULL;
+}
+
+/* See gdbtypes.h  */
+
+void add_dyn_attr (struct type * type, struct objfile *objfile,
+  struct dynamic_prop prop, enum dynamic_prop_kind kind)
+{
+  struct dynamic_prop_list * temp
+    = obstack_alloc (&objfile->objfile_obstack,
+              sizeof (struct dynamic_prop_list));
+  temp->prop = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
+  *temp->prop = prop;
+  temp->kind = kind;
+
+  if (TYPE_MAIN_TYPE (type)->dyn_attribs == NULL)
+    {
+      TYPE_MAIN_TYPE (type)->dyn_attribs = temp;
+      temp->next = NULL;
+    }
+  else
+    {
+      temp->next = TYPE_MAIN_TYPE (type)->dyn_attribs;
+      TYPE_MAIN_TYPE (type)->dyn_attribs = temp;
+    }
+}
+
+
 /* Find the real type of TYPE.  This function returns the real type,
    after removing all layers of typedefs, and completing opaque or stub
    types.  Completion changes the TYPE argument, but stripping of
@@ -4321,15 +4363,39 @@ copy_type_recursive (struct objfile *objfile,
       *TYPE_RANGE_DATA (new_type) = *TYPE_RANGE_DATA (type);
     }
 
-  /* Copy the data location information.  */
-  if (TYPE_DATA_LOCATION (type) != NULL)
+  if (TYPE_DYN_ATTRIBS (type) != NULL)
     {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
+      struct dynamic_prop_list * p;
+      struct dynamic_prop_list * trav = TYPE_DYN_ATTRIBS (type);
+
+      /* Copy head.  */
+      struct dynamic_prop_list * new_head =
+        TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
+      memcpy (new_head, TYPE_DYN_ATTRIBS (type),
+        sizeof (struct dynamic_prop_list));
+      new_head->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
+      memcpy (new_head->prop, TYPE_DYN_ATTRIBS (type)->prop,
+        sizeof (struct dynamic_prop));
+
+      /* Rest of list.  */
+      p = new_head;
+      trav = trav->next;
+      while (trav != NULL)
+        {
+          p->next = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
+          memcpy (p->next, trav,
+            sizeof (struct dynamic_prop_list));
+          p = p->next;
+          p->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
+          memcpy (p->prop, trav->prop, sizeof (struct dynamic_prop));
+          trav = trav->next;
+        }
+      p->next = NULL;
+
+      TYPE_DYN_ATTRIBS (new_type) = new_head;
     }
 
+
   /* Copy pointers to other types.  */
   if (TYPE_TARGET_TYPE (type))
     TYPE_TARGET_TYPE (new_type) = 
@@ -4392,12 +4458,36 @@ copy_type (const struct type *type)
   TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
   memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
 	  sizeof (struct main_type));
-  if (TYPE_DATA_LOCATION (type) != NULL)
+  if (TYPE_DYN_ATTRIBS (type) != NULL)
     {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
+      struct dynamic_prop_list * p;
+      struct dynamic_prop_list * trav = TYPE_DYN_ATTRIBS (type);
+
+      /* Copy head.  */
+      struct dynamic_prop_list * new_head =
+        TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
+      memcpy (new_head, TYPE_DYN_ATTRIBS (type),
+        sizeof (struct dynamic_prop_list));
+      new_head->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
+      memcpy (new_head->prop, TYPE_DYN_ATTRIBS (type)->prop,
+        sizeof (struct dynamic_prop));
+
+      /* Rest of list.  */
+      p = new_head;
+      trav = trav->next;
+      while (trav != NULL)
+        {
+          p->next = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
+          memcpy (p->next, trav,
+            sizeof (struct dynamic_prop_list));
+          p = p->next;
+          p->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
+          memcpy (p->prop, trav->prop, sizeof (struct dynamic_prop));
+          trav = trav->next;
+        }
+      p->next = NULL;
+
+      TYPE_DYN_ATTRIBS (new_type) = new_head;
     }
 
   return new_type;
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index ef6d92c..ab3e2cc 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -430,6 +430,25 @@ struct dynamic_prop
   } data;
 };
 
+/* * Defines the kind of a dynamic attribute of a type. The dynamic attributes
+   can be the following:
+
+   * DYN_ATTR_DATA_LOCATION:
+     Contains a location description value for the current type.
+     Evaluating this field yields to the location of the data for an object.
+*/
+enum dynamic_prop_kind
+{
+  DYN_ATTR_DATA_LOCATION
+};
+
+/* * List for dynamic type attributes.  */
+struct dynamic_prop_list
+{
+  enum dynamic_prop_kind kind;
+  struct dynamic_prop *prop;
+  struct dynamic_prop_list *next;
+};
 
 /* * Determine which field of the union main_type.fields[x].loc is
    used.  */
@@ -704,10 +723,8 @@ struct main_type
     struct type *self_type;
   } type_specific;
 
-  /* * Contains a location description value for the current type. Evaluating
-     this field yields to the location of the data for an object.  */
-
-  struct dynamic_prop *data_location;
+  /* * Contains all dynamic type attributes.  */
+  struct dynamic_prop_list *dyn_attribs;
 };
 
 /* * A ``struct type'' describes a particular instance of a type, with
@@ -1221,7 +1238,7 @@ extern void allocate_gnat_aux_type (struct type *);
 
 /* Attribute accessors for the type data location.  */
 #define TYPE_DATA_LOCATION(thistype) \
-  TYPE_MAIN_TYPE(thistype)->data_location
+  get_dyn_attr (thistype, DYN_ATTR_DATA_LOCATION)
 #define TYPE_DATA_LOCATION_BATON(thistype) \
   TYPE_DATA_LOCATION (thistype)->data.baton
 #define TYPE_DATA_LOCATION_ADDR(thistype) \
@@ -1229,6 +1246,17 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_DATA_LOCATION_KIND(thistype) \
   TYPE_DATA_LOCATION (thistype)->kind
 
+  /* Attribute accessors for dynamic attributes.  */
+#define TYPE_DYN_ATTRIBS(thistype) \
+  TYPE_MAIN_TYPE(thistype)->dyn_attribs
+#define TYPE_DYN_ATTR_BATON(dynprop) \
+  dynprop->data.baton
+#define TYPE_DYN_ATTR_ADDR(dynprop) \
+  dynprop->data.const_val
+#define TYPE_DYN_ATTR_KIND(dynprop) \
+  dynprop->kind
+
+
 /* Moto-specific stuff for FORTRAN arrays.  */
 
 #define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
@@ -1748,6 +1776,15 @@ extern struct type *resolve_dynamic_type (struct type *type, CORE_ADDR addr);
 /* * Predicate if the type has dynamic values, which are not resolved yet.  */
 extern int is_dynamic_type (struct type *type);
 
+/* * Fetches a dynamic attribute of a type out of the dynamic attribute
+   list.  */
+extern struct dynamic_prop * get_dyn_attr
+  (const struct type * type, enum dynamic_prop_kind kind);
+
+/* * Adds a dynamic attribute to a type.  */
+extern void add_dyn_attr (struct type * type, struct objfile *objfile,
+  struct dynamic_prop prop, enum dynamic_prop_kind kind);
+
 extern struct type *check_typedef (struct type *);
 
 #define CHECK_TYPEDEF(TYPE)			\
-- 
1.7.9.5

Re: [PATCH] Introduce linked list for dynamic attributes

[Joel Brobecker]

Keven,

On Mon, Mar 02, 2015 at 08:37:56AM +0100, Keven Boell wrote:
> This patch introduces a linked list for dynamic
> attributes of a type. This is a pre-work for
> the Fortran dynamic array support. The Fortran
> dynamic array support will add more dynamic
> attributes to a type. As only a few types
> will have such dynamic attributes set, a linked
> list is more efficient in terms of memory
> consumption than adding multiple attributes
> to main_type.
> 
> Transformed the data_location dynamic attribute
> to use the linked list.

Thanks for working on this.

Comments below. I have a bit of time this week for follow up
reviews, if you have time also.

> 2015-02-23  Keven Boell  <keven.boell@intel.com>
> 
> 	* gdbtypes.c (resolve_dynamic_type_internal):
> 	Adapted data_location usage to linked list.

We use the imperattive style... So "Adapt data_location [...]".

> 	(resolve_dynamic_type_internal): Adapted
> 	data_location usage to linked list.
> 	(get_dyn_attr): New function.
> 	(add_dyn_attr): New function.
> 	(copy_type_recursive): Add copy of linked
> 	list.
> 	(copy_type): Add copy of linked list.
> 	* gdbtypes.h (enum dynamic_prop_kind): Kind
> 	of the dynamic attribute in linked list.
> 	(struct dynamic_prop_list): Dynamic list
> 	node.
> 	* dwarf2read.c (set_die_type): Add data_location
> 	data to linked list using helper functions.

Lots of little comments, but the patch is going in the correct
direction, IMO, so we should be able to converge relatively
quickly, I think.

The comments are in the same order as the patch, which is a little
bit the wrong order. So, you'll see that I make suggestions earlier
that will need fixing due to comments  made afterwards. I thought
it was simpler for me to do it this way, rather than trying to
reorder the patch and risk missing something. I hope this is OK.

> ---
>  gdb/dwarf2read.c |    4 +-
>  gdb/gdbtypes.c   |  120 +++++++++++++++++++++++++++++++++++++++++++++++-------
>  gdb/gdbtypes.h   |   47 ++++++++++++++++++---
>  3 files changed, 148 insertions(+), 23 deletions(-)
> 
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index ac78165..9923758 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -22077,9 +22077,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
>    attr = dwarf2_attr (die, DW_AT_data_location, cu);
>    if (attr_to_dynamic_prop (attr, die, cu, &prop))
>      {
> -      TYPE_DATA_LOCATION (type)
> -        = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
> -      *TYPE_DATA_LOCATION (type) = prop;
> +      add_dyn_attr (type, objfile, prop, DYN_ATTR_DATA_LOCATION);
>      }

Can you remove the curly braces. It's part of the GDB coding style
where we only use the curly braces when required; if you have only
one statement inside the if block, then we omit the curly braces.
The exception to that rule is when you have a comment in addition
to the statment. Visually, the comment looks the same as a statement,
so we then use curly braces.

>    if (dwarf2_per_objfile->die_type_hash == NULL)
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index a80151c..65a6897 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -2021,7 +2021,7 @@ resolve_dynamic_type_internal (struct type *type,
>  {
>    struct type *real_type = check_typedef (type);
>    struct type *resolved_type = type;
> -  const struct dynamic_prop *prop;
> +  struct dynamic_prop *prop;
>    CORE_ADDR value;
>  
>    if (!is_dynamic_type_internal (real_type, top_level))
> @@ -2078,11 +2078,11 @@ resolve_dynamic_type_internal (struct type *type,
>    prop = TYPE_DATA_LOCATION (resolved_type);
>    if (dwarf2_evaluate_property (prop, addr_stack, &value))
>      {
> -      TYPE_DATA_LOCATION_ADDR (resolved_type) = value;
> -      TYPE_DATA_LOCATION_KIND (resolved_type) = PROP_CONST;
> +      TYPE_DYN_ATTR_ADDR (prop) = value;
> +      TYPE_DYN_ATTR_KIND (prop) = PROP_CONST;
>      }
>    else
> -    TYPE_DATA_LOCATION (resolved_type) = NULL;
> +    prop = NULL;

I think we can do better, in this case, as we don't really need
to set prop to NULL. In fact, this appears to be useless to do so?

So, how about:

  prop = TYPE_DATA_LOCATION (resolved_type);
  if (prop != NULL && dwarf2_evaluate_property (prop, addr_stack, &value))
    {
      TYPE_DYN_ATTR_ADDR (prop) = value;
      TYPE_DYN_ATTR_KIND (prop) = PROP_CONST;
    }

This avoids a function call if prop is NULL, even if
dwarf2_evaluate_property does handle it the way we expect it to.

FTR, a thought occured to me, that we might perhaps want
dwarf2_evaluate_property to evaluate in place, but after thinking
about it some more, I think it's going to be more practical to
leave things as is.

> +/* See gdbtypes.h  */
> +
> +struct dynamic_prop * get_dyn_attr (const struct type * type,
> +  enum dynamic_prop_kind kind)

The formatting needs to be fixed:
  - function names for function implementations should be at the start
    of the line
  - no space after '*'

Also, would you mind if I nit-picked a bit, and asked that parameters
be in the following order (generally speaking, not just limited to
that function):
  - prop_kind
  - prop
  - type
  - objfile

Thus:

struct dynamic_prop *
get_dyn_attr (enum dynamic_prop_kind prop_kind, const struct type *type)

(I also changed "kind" to "prop_kind" which, in my opinion, is
a little easier to grasp what it is when reading the code).

Feel free to disagree, of course, but I find that order a little
more logical in terms of how I think about this feature...


> +{
> +  struct dynamic_prop_list * head =
> +    (TYPE_MAIN_TYPE (type))->dyn_attribs;

Formatting:
  - No space after '*';
  - The '=' should be at the start of the next line (GNU coding
    style regarding the formatting where binary operators are involved)

Also, please forgive the nitpick, but I think it would be better
if you renamed "head" to something like "node". Anything that does
not suggest that your variable points to the head of the list.

> +  while (head != NULL)
> +    {
> +      if (head->kind == kind)
> +        return head->prop;
> +      head = head->next;
> +    }
> +  return NULL;
> +}
> +
> +/* See gdbtypes.h  */
> +
> +void add_dyn_attr (struct type * type, struct objfile *objfile,
> +  struct dynamic_prop prop, enum dynamic_prop_kind kind)
> +{
> +  struct dynamic_prop_list * temp
> +    = obstack_alloc (&objfile->objfile_obstack,
> +              sizeof (struct dynamic_prop_list));
> +  temp->prop = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
> +  *temp->prop = prop;

Same remarks as above (formatting, parameter order and names, etc).
So:

void
add_dyn_attr (enum dynamic_prop_kind prop_kind, struct dynamic_prop prop,
              struct type *type, struct objfile *objfile)

> +  temp->kind = kind;
> +
> +  if (TYPE_MAIN_TYPE (type)->dyn_attribs == NULL)
> +    {
> +      TYPE_MAIN_TYPE (type)->dyn_attribs = temp;
> +      temp->next = NULL;
> +    }
> +  else
> +    {
> +      temp->next = TYPE_MAIN_TYPE (type)->dyn_attribs;
> +      TYPE_MAIN_TYPE (type)->dyn_attribs = temp;
> +    }
> +}

I think you can simply the above with:

    temp->next = TYPE_MAIN_TYPE (type)->dyn_attribs;
    TYPE_MAIN_TYPE (type)->dyn_attribs = temp;

>  /* Find the real type of TYPE.  This function returns the real type,
>     after removing all layers of typedefs, and completing opaque or stub
>     types.  Completion changes the TYPE argument, but stripping of
> @@ -4321,15 +4363,39 @@ copy_type_recursive (struct objfile *objfile,
>        *TYPE_RANGE_DATA (new_type) = *TYPE_RANGE_DATA (type);
>      }
>  
> -  /* Copy the data location information.  */
> -  if (TYPE_DATA_LOCATION (type) != NULL)
> +  if (TYPE_DYN_ATTRIBS (type) != NULL)
>      {
> -      TYPE_DATA_LOCATION (new_type)
> -	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
> -      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
> -	      sizeof (struct dynamic_prop));
> +      struct dynamic_prop_list * p;
> +      struct dynamic_prop_list * trav = TYPE_DYN_ATTRIBS (type);
> +
> +      /* Copy head.  */
> +      struct dynamic_prop_list * new_head =
> +        TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
> +      memcpy (new_head, TYPE_DYN_ATTRIBS (type),
> +        sizeof (struct dynamic_prop_list));
> +      new_head->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
> +      memcpy (new_head->prop, TYPE_DYN_ATTRIBS (type)->prop,
> +        sizeof (struct dynamic_prop));
> +
> +      /* Rest of list.  */
> +      p = new_head;
> +      trav = trav->next;
> +      while (trav != NULL)
> +        {
> +          p->next = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
> +          memcpy (p->next, trav,
> +            sizeof (struct dynamic_prop_list));
> +          p = p->next;
> +          p->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
> +          memcpy (p->prop, trav->prop, sizeof (struct dynamic_prop));
> +          trav = trav->next;
> +        }
> +      p->next = NULL;
> +
> +      TYPE_DYN_ATTRIBS (new_type) = new_head;

The same is done at two locations, so let's factorize this code.
Suggest we create a function which, given a prop list, returns
a copy of the prop list. That way, I suspect the code will just
become:

    if (TYPE_DYN_ATTRIBS (type) != NULL)
      TYPE_DYN_ATTRIBS (new_type)
        =  dynamic_prop_list_copy (TYPE_DYN_ATTRIBS (type));

And the function will probably not have to worry about the "head"
vs "the rest" thing.

Watch out for the formatting issues I've been mentioning earlier.

> +
>    /* Copy pointers to other types.  */
>    if (TYPE_TARGET_TYPE (type))
>      TYPE_TARGET_TYPE (new_type) = 
> @@ -4392,12 +4458,36 @@ copy_type (const struct type *type)
>    TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
>    memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
>  	  sizeof (struct main_type));
> -  if (TYPE_DATA_LOCATION (type) != NULL)
> +  if (TYPE_DYN_ATTRIBS (type) != NULL)
>      {
> -      TYPE_DATA_LOCATION (new_type)
> -	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
> -      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
> -	      sizeof (struct dynamic_prop));
> +      struct dynamic_prop_list * p;
> +      struct dynamic_prop_list * trav = TYPE_DYN_ATTRIBS (type);
> +
> +      /* Copy head.  */
> +      struct dynamic_prop_list * new_head =
> +        TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
> +      memcpy (new_head, TYPE_DYN_ATTRIBS (type),
> +        sizeof (struct dynamic_prop_list));
> +      new_head->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
> +      memcpy (new_head->prop, TYPE_DYN_ATTRIBS (type)->prop,
> +        sizeof (struct dynamic_prop));
> +
> +      /* Rest of list.  */
> +      p = new_head;
> +      trav = trav->next;
> +      while (trav != NULL)
> +        {
> +          p->next = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
> +          memcpy (p->next, trav,
> +            sizeof (struct dynamic_prop_list));
> +          p = p->next;
> +          p->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
> +          memcpy (p->prop, trav->prop, sizeof (struct dynamic_prop));
> +          trav = trav->next;
> +        }
> +      p->next = NULL;
> +
> +      TYPE_DYN_ATTRIBS (new_type) = new_head;

Same as above - factorize and simplify.

>      }
>  
>    return new_type;
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index ef6d92c..ab3e2cc 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -430,6 +430,25 @@ struct dynamic_prop
>    } data;
>  };
>  
> +/* * Defines the kind of a dynamic attribute of a type. The dynamic attributes
> +   can be the following:
> +
> +   * DYN_ATTR_DATA_LOCATION:
> +     Contains a location description value for the current type.
> +     Evaluating this field yields to the location of the data for an object.
> +*/
> +enum dynamic_prop_kind
> +{
> +  DYN_ATTR_DATA_LOCATION
> +};

"Defines" -> "Define" (we use the imperative in our function
documentation). But I'm having difficulties understanding
that sentence, so let me try to suggest something else (see
proposal below).

Two spaces after periods. But I think this becomes moot if we document
each enum kind individually (inside the enum definition), which is what
we usually do. This will also avoid risks of the documentation
bit-rotting on us when we add/remove/change names...

Therefore:

/* * Define a type's dynamic property kind.  */

enum dynamic_prop_kind
{
  /* A property providing a type's data location.
     Evaluating this field yields to the location of an object's data.  */
  DYN_ATTR_DATA_LOCATION,
};

(I've added a coma at the end of DYN_ATTR_DATA_LOCATION. It's not
necessary, but avoids having to touch that line when adding extra
enumerates)

> +/* * List for dynamic type attributes.  */
> +struct dynamic_prop_list
> +{
> +  enum dynamic_prop_kind kind;
> +  struct dynamic_prop *prop;
> +  struct dynamic_prop_list *next;
> +};

If you don't mind, let's rename "kind" to "prop_kind" to be consistent
with the naming used elsewhere?

Also, I know I propose to make "prop" a pointer, but I think the code
will be simpler if we remove the pointer indirection. And we'll also
having to store a pointer, which saves us 4-8 bytes per dynamic
property...


>  /* * Determine which field of the union main_type.fields[x].loc is
>     used.  */
> @@ -704,10 +723,8 @@ struct main_type
>      struct type *self_type;
>    } type_specific;
>  
> -  /* * Contains a location description value for the current type. Evaluating
> -     this field yields to the location of the data for an object.  */
> -
> -  struct dynamic_prop *data_location;
> +  /* * Contains all dynamic type attributes.  */
> +  struct dynamic_prop_list *dyn_attribs;

We have been calling these "properties", so can we remain consistent
with that? Can we rename the field to "prop_list", for instance?
Or something else that you might prefer? Also, let's adjust the
comment to use "property" as well.

>  };
>  
>  /* * A ``struct type'' describes a particular instance of a type, with
> @@ -1221,7 +1238,7 @@ extern void allocate_gnat_aux_type (struct type *);
>  
>  /* Attribute accessors for the type data location.  */
>  #define TYPE_DATA_LOCATION(thistype) \
> -  TYPE_MAIN_TYPE(thistype)->data_location
> +  get_dyn_attr (thistype, DYN_ATTR_DATA_LOCATION)
>  #define TYPE_DATA_LOCATION_BATON(thistype) \
>    TYPE_DATA_LOCATION (thistype)->data.baton
>  #define TYPE_DATA_LOCATION_ADDR(thistype) \

IMO, I would remove all these macros, and let users call
get_dyn_attr, and then manipulate the property using the generic
macros that you're defining instead.

If it makes the patch bigger, let's keep that activity as patch #2.
It can be done independently.

> @@ -1229,6 +1246,17 @@ extern void allocate_gnat_aux_type (struct type *);
>  #define TYPE_DATA_LOCATION_KIND(thistype) \
>    TYPE_DATA_LOCATION (thistype)->kind
>  
> +  /* Attribute accessors for dynamic attributes.  */

Properties.

> +#define TYPE_DYN_ATTRIBS(thistype) \
> +  TYPE_MAIN_TYPE(thistype)->dyn_attribs
> +#define TYPE_DYN_ATTR_BATON(dynprop) \
> +  dynprop->data.baton
> +#define TYPE_DYN_ATTR_ADDR(dynprop) \
> +  dynprop->data.const_val
> +#define TYPE_DYN_ATTR_KIND(dynprop) \
> +  dynprop->kind

Let's also s/_ATTR_/_PROP_/ if you don't mind.

>  /* Moto-specific stuff for FORTRAN arrays.  */
>  
>  #define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
> @@ -1748,6 +1776,15 @@ extern struct type *resolve_dynamic_type (struct type *type, CORE_ADDR addr);
>  /* * Predicate if the type has dynamic values, which are not resolved yet.  */
>  extern int is_dynamic_type (struct type *type);
>  
> +/* * Fetches a dynamic attribute of a type out of the dynamic attribute
> +   list.  */
> +extern struct dynamic_prop * get_dyn_attr
> +  (const struct type * type, enum dynamic_prop_kind kind);

"Fetches" -> "Fetch"; "attribute" -> "property";
"out of" -> "from".
Also, can you document the fact that the function returns NULL
of the dynamic property cannnot be found.

Also, can you change the function's name to say "prop" instead of
"attr"?

Please also fix the function's formatting. In this case, since this
is only a declaration, the function's name should  NOT be at the start

> +/* * Adds a dynamic attribute to a type.  */
> +extern void add_dyn_attr (struct type * type, struct objfile *objfile,
> +  struct dynamic_prop prop, enum dynamic_prop_kind kind);

Same kind of remarks as above.

> +
>  extern struct type *check_typedef (struct type *);
>  
>  #define CHECK_TYPEDEF(TYPE)			\
> -- 
> 1.7.9.5

Thanks again for the patch,
-- 
Joel

Re: [PATCH] Introduce linked list for dynamic attributes

[Keven Boell]

Hi Joel,

Here is the updated patch.

Thanks,
Keven

---

From: Keven Boell <keven.boell@intel.com>
Date: Mon, 23 Feb 2015 15:45:06 +0100
Subject: [PATCH] Introduce linked list for dynamic attributes

This patch introduces a linked list for dynamic
attributes of a type. This is a pre-work for
the Fortran dynamic array support. The Fortran
dynamic array support will add more dynamic
attributes to a type. As only a few types
will have such dynamic attributes set, a linked
list is more efficient in terms of memory
consumption than adding multiple attributes
to main_type.

Transformed the data_location dynamic attribute
to use the linked list.

2015-02-23  Keven Boell  <keven.boell@intel.com>

	* gdbtypes.c (resolve_dynamic_type_internal):
	Adapt data_location usage to linked list.
	(resolve_dynamic_type_internal): Adapt
	data_location usage to linked list.
	(get_dyn_prop): New function.
	(add_dyn_prop): New function.
	(copy_dynamic_prop_list): New function.
	(copy_type_recursive): Add copy of linked
	list.
	(copy_type): Add copy of linked list.
	* gdbtypes.h (enum dynamic_prop_node_kind):
	Kind of the dynamic attribute in linked list.
	(struct dynamic_prop_list): Dynamic list
	node.
	* dwarf2read.c (set_die_type): Add data_location
	data to linked list using helper functions.
---
 gdb/dwarf2read.c |    6 +---
 gdb/gdbtypes.c   |   86 +++++++++++++++++++++++++++++++++++++++++-------------
 gdb/gdbtypes.h   |   48 ++++++++++++++++++++++++++----
 3 files changed, 108 insertions(+), 32 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 071f97b..370143d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -22084,11 +22084,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   /* Read DW_AT_data_location and set in type.  */
   attr = dwarf2_attr (die, DW_AT_data_location, cu);
   if (attr_to_dynamic_prop (attr, die, cu, &prop))
-    {
-      TYPE_DATA_LOCATION (type)
-        = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
-      *TYPE_DATA_LOCATION (type) = prop;
-    }
+    add_dyn_prop (DYN_ATTR_DATA_LOCATION, prop, type, objfile);
 
   if (dwarf2_per_objfile->die_type_hash == NULL)
     {
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 24f64be..20bdc20 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2025,7 +2025,7 @@ resolve_dynamic_type_internal (struct type *type,
 {
   struct type *real_type = check_typedef (type);
   struct type *resolved_type = type;
-  const struct dynamic_prop *prop;
+  struct dynamic_prop *prop;
   CORE_ADDR value;
 
   if (!is_dynamic_type_internal (real_type, top_level))
@@ -2080,13 +2080,11 @@ resolve_dynamic_type_internal (struct type *type,
 
   /* Resolve data_location attribute.  */
   prop = TYPE_DATA_LOCATION (resolved_type);
-  if (dwarf2_evaluate_property (prop, addr_stack, &value))
+  if (prop != NULL && dwarf2_evaluate_property (prop, addr_stack, &value))
     {
-      TYPE_DATA_LOCATION_ADDR (resolved_type) = value;
-      TYPE_DATA_LOCATION_KIND (resolved_type) = PROP_CONST;
+      TYPE_DYN_PROP_ADDR (prop) = value;
+      TYPE_DYN_PROP_KIND (prop) = PROP_CONST;
     }
-  else
-    TYPE_DATA_LOCATION (resolved_type) = NULL;
 
   return resolved_type;
 }
@@ -2101,6 +2099,41 @@ resolve_dynamic_type (struct type *type, CORE_ADDR addr)
   return resolve_dynamic_type_internal (type, &pinfo, 1);
 }
 
+/* See gdbtypes.h  */
+
+struct dynamic_prop *
+get_dyn_prop (enum dynamic_prop_node_kind prop_kind, const struct type *type)
+{
+  struct dynamic_prop_list *node
+    = TYPE_DYN_PROP_LIST (type);
+
+  while (node != NULL)
+    {
+      if (node->prop_kind == prop_kind)
+        return node->prop;
+      node = node->next;
+    }
+  return NULL;
+}
+
+/* See gdbtypes.h  */
+
+void
+add_dyn_prop (enum dynamic_prop_node_kind prop_kind, struct dynamic_prop prop,
+              struct type *type, struct objfile *objfile)
+{
+  struct dynamic_prop_list *temp
+    = obstack_alloc (&objfile->objfile_obstack,
+              sizeof (struct dynamic_prop_list));
+  temp->prop = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
+  *temp->prop = prop;
+  temp->prop_kind = prop_kind;
+
+  temp->next = TYPE_DYN_PROP_LIST (type);
+  TYPE_DYN_PROP_LIST (type) = temp;
+}
+
+
 /* Find the real type of TYPE.  This function returns the real type,
    after removing all layers of typedefs, and completing opaque or stub
    types.  Completion changes the TYPE argument, but stripping of
@@ -4222,6 +4255,25 @@ create_copied_types_hash (struct objfile *objfile)
 			       dummy_obstack_deallocate);
 }
 
+/* Recursively copy (deep copy) a dynamic attribute list of a type.  */
+
+struct dynamic_prop_list *
+copy_dynamic_prop_list (struct dynamic_prop_list *list)
+{
+  struct dynamic_prop_list *copy;
+
+  copy = xmalloc (sizeof (struct dynamic_prop_list));
+  *copy = *list;
+
+  copy->prop = xmalloc (sizeof (struct dynamic_prop));
+  *(copy->prop) = *(list->prop);
+
+  if (list->next != NULL)
+    copy->next = copy_dynamic_prop_list (list->next);
+
+  return copy;
+}
+
 /* Recursively copy (deep copy) TYPE, if it is associated with
    OBJFILE.  Return a new type allocated using malloc, a saved type if
    we have already visited TYPE (using COPIED_TYPES), or TYPE if it is
@@ -4325,14 +4377,10 @@ copy_type_recursive (struct objfile *objfile,
       *TYPE_RANGE_DATA (new_type) = *TYPE_RANGE_DATA (type);
     }
 
-  /* Copy the data location information.  */
-  if (TYPE_DATA_LOCATION (type) != NULL)
-    {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
-    }
+  if (TYPE_DYN_PROP_LIST (type) != NULL)
+    TYPE_DYN_PROP_LIST (new_type)
+      = copy_dynamic_prop_list (TYPE_DYN_PROP_LIST (type));
+
 
   /* Copy pointers to other types.  */
   if (TYPE_TARGET_TYPE (type))
@@ -4396,13 +4444,9 @@ copy_type (const struct type *type)
   TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
   memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
 	  sizeof (struct main_type));
-  if (TYPE_DATA_LOCATION (type) != NULL)
-    {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
-    }
+  if (TYPE_DYN_PROP_LIST (type) != NULL)
+    TYPE_DYN_PROP_LIST (new_type)
+      = copy_dynamic_prop_list (TYPE_DYN_PROP_LIST (type));
 
   return new_type;
 }
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 2c5ccf4..c852c5d 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -434,6 +434,21 @@ struct dynamic_prop
   union dynamic_prop_data data;
 };
 
+/* * Define a type's dynamic property node kind.  */
+enum dynamic_prop_node_kind
+{
+  /* A property providing a type's data location.
+     Evaluating this field yields to the location of an object's data.  */
+  DYN_ATTR_DATA_LOCATION,
+};
+
+/* * List for dynamic type attributes.  */
+struct dynamic_prop_list
+{
+  enum dynamic_prop_node_kind prop_kind;
+  struct dynamic_prop *prop;
+  struct dynamic_prop_list *next;
+};
 
 /* * Determine which field of the union main_type.fields[x].loc is
    used.  */
@@ -719,10 +734,8 @@ struct main_type
 
   union type_specific type_specific;
 
-  /* * Contains a location description value for the current type. Evaluating
-     this field yields to the location of the data for an object.  */
-
-  struct dynamic_prop *data_location;
+  /* * Contains all dynamic type properties.  */
+  struct dynamic_prop_list *dyn_prop_list;
 };
 
 /* * A ``struct type'' describes a particular instance of a type, with
@@ -1238,9 +1251,9 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_LOW_BOUND_KIND(range_type) \
   TYPE_RANGE_DATA(range_type)->low.kind
 
-/* Attribute accessors for the type data location.  */
+/* Property accessors for the type data location.  */
 #define TYPE_DATA_LOCATION(thistype) \
-  TYPE_MAIN_TYPE(thistype)->data_location
+  get_dyn_prop (DYN_ATTR_DATA_LOCATION, thistype)
 #define TYPE_DATA_LOCATION_BATON(thistype) \
   TYPE_DATA_LOCATION (thistype)->data.baton
 #define TYPE_DATA_LOCATION_ADDR(thistype) \
@@ -1248,6 +1261,17 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_DATA_LOCATION_KIND(thistype) \
   TYPE_DATA_LOCATION (thistype)->kind
 
+/* Attribute accessors for dynamic properties.  */
+#define TYPE_DYN_PROP_LIST(thistype) \
+  TYPE_MAIN_TYPE(thistype)->dyn_prop_list
+#define TYPE_DYN_PROP_BATON(dynprop) \
+  dynprop->data.baton
+#define TYPE_DYN_PROP_ADDR(dynprop) \
+  dynprop->data.const_val
+#define TYPE_DYN_PROP_KIND(dynprop) \
+  dynprop->kind
+
+
 /* Moto-specific stuff for FORTRAN arrays.  */
 
 #define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
@@ -1767,6 +1791,18 @@ extern struct type *resolve_dynamic_type (struct type *type, CORE_ADDR addr);
 /* * Predicate if the type has dynamic values, which are not resolved yet.  */
 extern int is_dynamic_type (struct type *type);
 
+/* * Fetch a dynamic property of a type from the dynamic attribute
+   list.  */
+extern struct dynamic_prop *get_dyn_prop
+  (enum dynamic_prop_node_kind kind, const struct type *type);
+
+/* * Add a dynamic property to a type.  */
+extern void add_dyn_prop (enum dynamic_prop_node_kind kind,
+  struct dynamic_prop prop, struct type *type, struct objfile *objfile);
+
+extern struct dynamic_prop_list *copy_dynamic_prop_list
+  (struct dynamic_prop_list *list);
+
 extern struct type *check_typedef (struct type *);
 
 #define CHECK_TYPEDEF(TYPE)			\
-- 
1.7.9.5

Re: [PATCH] Introduce linked list for dynamic attributes

[Keven Boell]

Joel,

Thanks for the review and all the feedback. I've addressed most of the things you mentioned.
Please see my comments below.

On 02.03.2015 20:49, Joel Brobecker wrote:
> Keven,
> 
> On Mon, Mar 02, 2015 at 08:37:56AM +0100, Keven Boell wrote:
>> This patch introduces a linked list for dynamic
>> attributes of a type. This is a pre-work for
>> the Fortran dynamic array support. The Fortran
>> dynamic array support will add more dynamic
>> attributes to a type. As only a few types
>> will have such dynamic attributes set, a linked
>> list is more efficient in terms of memory
>> consumption than adding multiple attributes
>> to main_type.
>>
>> Transformed the data_location dynamic attribute
>> to use the linked list.
> 
> Thanks for working on this.
> 
> Comments below. I have a bit of time this week for follow up
> reviews, if you have time also.
> 
>> 2015-02-23  Keven Boell  <keven.boell@intel.com>
>>
>> 	* gdbtypes.c (resolve_dynamic_type_internal):
>> 	Adapted data_location usage to linked list.
> 
> We use the imperattive style... So "Adapt data_location [...]".
> 
>> 	(resolve_dynamic_type_internal): Adapted
>> 	data_location usage to linked list.
>> 	(get_dyn_attr): New function.
>> 	(add_dyn_attr): New function.
>> 	(copy_type_recursive): Add copy of linked
>> 	list.
>> 	(copy_type): Add copy of linked list.
>> 	* gdbtypes.h (enum dynamic_prop_kind): Kind
>> 	of the dynamic attribute in linked list.
>> 	(struct dynamic_prop_list): Dynamic list
>> 	node.
>> 	* dwarf2read.c (set_die_type): Add data_location
>> 	data to linked list using helper functions.
> 
> Lots of little comments, but the patch is going in the correct
> direction, IMO, so we should be able to converge relatively
> quickly, I think.
> 
> The comments are in the same order as the patch, which is a little
> bit the wrong order. So, you'll see that I make suggestions earlier
> that will need fixing due to comments  made afterwards. I thought
> it was simpler for me to do it this way, rather than trying to
> reorder the patch and risk missing something. I hope this is OK.
> 
>> ---
>>  gdb/dwarf2read.c |    4 +-
>>  gdb/gdbtypes.c   |  120 +++++++++++++++++++++++++++++++++++++++++++++++-------
>>  gdb/gdbtypes.h   |   47 ++++++++++++++++++---
>>  3 files changed, 148 insertions(+), 23 deletions(-)
>>
>> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
>> index ac78165..9923758 100644
>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -22077,9 +22077,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
>>    attr = dwarf2_attr (die, DW_AT_data_location, cu);
>>    if (attr_to_dynamic_prop (attr, die, cu, &prop))
>>      {
>> -      TYPE_DATA_LOCATION (type)
>> -        = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
>> -      *TYPE_DATA_LOCATION (type) = prop;
>> +      add_dyn_attr (type, objfile, prop, DYN_ATTR_DATA_LOCATION);
>>      }
> 
> Can you remove the curly braces. It's part of the GDB coding style
> where we only use the curly braces when required; if you have only
> one statement inside the if block, then we omit the curly braces.
> The exception to that rule is when you have a comment in addition
> to the statment. Visually, the comment looks the same as a statement,
> so we then use curly braces.

Done.

> 
>>    if (dwarf2_per_objfile->die_type_hash == NULL)
>> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
>> index a80151c..65a6897 100644
>> --- a/gdb/gdbtypes.c
>> +++ b/gdb/gdbtypes.c
>> @@ -2021,7 +2021,7 @@ resolve_dynamic_type_internal (struct type *type,
>>  {
>>    struct type *real_type = check_typedef (type);
>>    struct type *resolved_type = type;
>> -  const struct dynamic_prop *prop;
>> +  struct dynamic_prop *prop;
>>    CORE_ADDR value;
>>  
>>    if (!is_dynamic_type_internal (real_type, top_level))
>> @@ -2078,11 +2078,11 @@ resolve_dynamic_type_internal (struct type *type,
>>    prop = TYPE_DATA_LOCATION (resolved_type);
>>    if (dwarf2_evaluate_property (prop, addr_stack, &value))
>>      {
>> -      TYPE_DATA_LOCATION_ADDR (resolved_type) = value;
>> -      TYPE_DATA_LOCATION_KIND (resolved_type) = PROP_CONST;
>> +      TYPE_DYN_ATTR_ADDR (prop) = value;
>> +      TYPE_DYN_ATTR_KIND (prop) = PROP_CONST;
>>      }
>>    else
>> -    TYPE_DATA_LOCATION (resolved_type) = NULL;
>> +    prop = NULL;
> 
> I think we can do better, in this case, as we don't really need
> to set prop to NULL. In fact, this appears to be useless to do so?
> 
> So, how about:
> 
>   prop = TYPE_DATA_LOCATION (resolved_type);
>   if (prop != NULL && dwarf2_evaluate_property (prop, addr_stack, &value))
>     {
>       TYPE_DYN_ATTR_ADDR (prop) = value;
>       TYPE_DYN_ATTR_KIND (prop) = PROP_CONST;
>     }
> 
> This avoids a function call if prop is NULL, even if
> dwarf2_evaluate_property does handle it the way we expect it to.

Done.

> 
> FTR, a thought occured to me, that we might perhaps want
> dwarf2_evaluate_property to evaluate in place, but after thinking
> about it some more, I think it's going to be more practical to
> leave things as is.
> 
>> +/* See gdbtypes.h  */
>> +
>> +struct dynamic_prop * get_dyn_attr (const struct type * type,
>> +  enum dynamic_prop_kind kind)
> 
> The formatting needs to be fixed:
>   - function names for function implementations should be at the start
>     of the line
>   - no space after '*'
> 
> Also, would you mind if I nit-picked a bit, and asked that parameters
> be in the following order (generally speaking, not just limited to
> that function):
>   - prop_kind
>   - prop
>   - type
>   - objfile
> 
> Thus:
> 
> struct dynamic_prop *
> get_dyn_attr (enum dynamic_prop_kind prop_kind, const struct type *type)
> 
> (I also changed "kind" to "prop_kind" which, in my opinion, is
> a little easier to grasp what it is when reading the code).
> 
> Feel free to disagree, of course, but I find that order a little
> more logical in terms of how I think about this feature...
> 

Makes sense, I was not thinking about the order of the attributes to be honest.
I've changed them as proposed.

> 
>> +{
>> +  struct dynamic_prop_list * head =
>> +    (TYPE_MAIN_TYPE (type))->dyn_attribs;
> 
> Formatting:
>   - No space after '*';
>   - The '=' should be at the start of the next line (GNU coding
>     style regarding the formatting where binary operators are involved)
> 
> Also, please forgive the nitpick, but I think it would be better
> if you renamed "head" to something like "node". Anything that does
> not suggest that your variable points to the head of the list.
> 

Done.

>> +  while (head != NULL)
>> +    {
>> +      if (head->kind == kind)
>> +        return head->prop;
>> +      head = head->next;
>> +    }
>> +  return NULL;
>> +}
>> +
>> +/* See gdbtypes.h  */
>> +
>> +void add_dyn_attr (struct type * type, struct objfile *objfile,
>> +  struct dynamic_prop prop, enum dynamic_prop_kind kind)
>> +{
>> +  struct dynamic_prop_list * temp
>> +    = obstack_alloc (&objfile->objfile_obstack,
>> +              sizeof (struct dynamic_prop_list));
>> +  temp->prop = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
>> +  *temp->prop = prop;
> 
> Same remarks as above (formatting, parameter order and names, etc).
> So:
> 
> void
> add_dyn_attr (enum dynamic_prop_kind prop_kind, struct dynamic_prop prop,
>               struct type *type, struct objfile *objfile)

Done.

> 
>> +  temp->kind = kind;
>> +
>> +  if (TYPE_MAIN_TYPE (type)->dyn_attribs == NULL)
>> +    {
>> +      TYPE_MAIN_TYPE (type)->dyn_attribs = temp;
>> +      temp->next = NULL;
>> +    }
>> +  else
>> +    {
>> +      temp->next = TYPE_MAIN_TYPE (type)->dyn_attribs;
>> +      TYPE_MAIN_TYPE (type)->dyn_attribs = temp;
>> +    }
>> +}
> 
> I think you can simply the above with:
> 
>     temp->next = TYPE_MAIN_TYPE (type)->dyn_attribs;
>     TYPE_MAIN_TYPE (type)->dyn_attribs = temp;

Done.

> 
>>  /* Find the real type of TYPE.  This function returns the real type,
>>     after removing all layers of typedefs, and completing opaque or stub
>>     types.  Completion changes the TYPE argument, but stripping of
>> @@ -4321,15 +4363,39 @@ copy_type_recursive (struct objfile *objfile,
>>        *TYPE_RANGE_DATA (new_type) = *TYPE_RANGE_DATA (type);
>>      }
>>  
>> -  /* Copy the data location information.  */
>> -  if (TYPE_DATA_LOCATION (type) != NULL)
>> +  if (TYPE_DYN_ATTRIBS (type) != NULL)
>>      {
>> -      TYPE_DATA_LOCATION (new_type)
>> -	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
>> -      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
>> -	      sizeof (struct dynamic_prop));
>> +      struct dynamic_prop_list * p;
>> +      struct dynamic_prop_list * trav = TYPE_DYN_ATTRIBS (type);
>> +
>> +      /* Copy head.  */
>> +      struct dynamic_prop_list * new_head =
>> +        TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
>> +      memcpy (new_head, TYPE_DYN_ATTRIBS (type),
>> +        sizeof (struct dynamic_prop_list));
>> +      new_head->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
>> +      memcpy (new_head->prop, TYPE_DYN_ATTRIBS (type)->prop,
>> +        sizeof (struct dynamic_prop));
>> +
>> +      /* Rest of list.  */
>> +      p = new_head;
>> +      trav = trav->next;
>> +      while (trav != NULL)
>> +        {
>> +          p->next = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
>> +          memcpy (p->next, trav,
>> +            sizeof (struct dynamic_prop_list));
>> +          p = p->next;
>> +          p->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
>> +          memcpy (p->prop, trav->prop, sizeof (struct dynamic_prop));
>> +          trav = trav->next;
>> +        }
>> +      p->next = NULL;
>> +
>> +      TYPE_DYN_ATTRIBS (new_type) = new_head;
> 
> The same is done at two locations, so let's factorize this code.
> Suggest we create a function which, given a prop list, returns
> a copy of the prop list. That way, I suspect the code will just
> become:
> 
>     if (TYPE_DYN_ATTRIBS (type) != NULL)
>       TYPE_DYN_ATTRIBS (new_type)
>         =  dynamic_prop_list_copy (TYPE_DYN_ATTRIBS (type));
> 
> And the function will probably not have to worry about the "head"
> vs "the rest" thing.
> 
> Watch out for the formatting issues I've been mentioning earlier.

I've refactored and simplified the code.

> 
>> +
>>    /* Copy pointers to other types.  */
>>    if (TYPE_TARGET_TYPE (type))
>>      TYPE_TARGET_TYPE (new_type) = 
>> @@ -4392,12 +4458,36 @@ copy_type (const struct type *type)
>>    TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
>>    memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
>>  	  sizeof (struct main_type));
>> -  if (TYPE_DATA_LOCATION (type) != NULL)
>> +  if (TYPE_DYN_ATTRIBS (type) != NULL)
>>      {
>> -      TYPE_DATA_LOCATION (new_type)
>> -	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
>> -      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
>> -	      sizeof (struct dynamic_prop));
>> +      struct dynamic_prop_list * p;
>> +      struct dynamic_prop_list * trav = TYPE_DYN_ATTRIBS (type);
>> +
>> +      /* Copy head.  */
>> +      struct dynamic_prop_list * new_head =
>> +        TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
>> +      memcpy (new_head, TYPE_DYN_ATTRIBS (type),
>> +        sizeof (struct dynamic_prop_list));
>> +      new_head->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
>> +      memcpy (new_head->prop, TYPE_DYN_ATTRIBS (type)->prop,
>> +        sizeof (struct dynamic_prop));
>> +
>> +      /* Rest of list.  */
>> +      p = new_head;
>> +      trav = trav->next;
>> +      while (trav != NULL)
>> +        {
>> +          p->next = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop_list));
>> +          memcpy (p->next, trav,
>> +            sizeof (struct dynamic_prop_list));
>> +          p = p->next;
>> +          p->prop = TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
>> +          memcpy (p->prop, trav->prop, sizeof (struct dynamic_prop));
>> +          trav = trav->next;
>> +        }
>> +      p->next = NULL;
>> +
>> +      TYPE_DYN_ATTRIBS (new_type) = new_head;
> 
> Same as above - factorize and simplify.

Done.

> 
>>      }
>>  
>>    return new_type;
>> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
>> index ef6d92c..ab3e2cc 100644
>> --- a/gdb/gdbtypes.h
>> +++ b/gdb/gdbtypes.h
>> @@ -430,6 +430,25 @@ struct dynamic_prop
>>    } data;
>>  };
>>  
>> +/* * Defines the kind of a dynamic attribute of a type. The dynamic attributes
>> +   can be the following:
>> +
>> +   * DYN_ATTR_DATA_LOCATION:
>> +     Contains a location description value for the current type.
>> +     Evaluating this field yields to the location of the data for an object.
>> +*/
>> +enum dynamic_prop_kind
>> +{
>> +  DYN_ATTR_DATA_LOCATION
>> +};
> 
> "Defines" -> "Define" (we use the imperative in our function
> documentation). But I'm having difficulties understanding
> that sentence, so let me try to suggest something else (see
> proposal below).
> 
> Two spaces after periods. But I think this becomes moot if we document
> each enum kind individually (inside the enum definition), which is what
> we usually do. This will also avoid risks of the documentation
> bit-rotting on us when we add/remove/change names...
> 
> Therefore:
> 
> /* * Define a type's dynamic property kind.  */
> 
> enum dynamic_prop_kind
> {
>   /* A property providing a type's data location.
>      Evaluating this field yields to the location of an object's data.  */
>   DYN_ATTR_DATA_LOCATION,
> };
> 
> (I've added a coma at the end of DYN_ATTR_DATA_LOCATION. It's not
> necessary, but avoids having to touch that line when adding extra
> enumerates)
> 

Done.

>> +/* * List for dynamic type attributes.  */
>> +struct dynamic_prop_list
>> +{
>> +  enum dynamic_prop_kind kind;
>> +  struct dynamic_prop *prop;
>> +  struct dynamic_prop_list *next;
>> +};
> 
> If you don't mind, let's rename "kind" to "prop_kind" to be consistent
> with the naming used elsewhere?
> 

Done.

> Also, I know I propose to make "prop" a pointer, but I think the code
> will be simpler if we remove the pointer indirection. And we'll also
> having to store a pointer, which saves us 4-8 bytes per dynamic
> property...
> 

I would like to keep the pointer for now as the data_location was a pointer before.
It would otherwise require some refactoring of the callers. Is this ok with you?

> 
>>  /* * Determine which field of the union main_type.fields[x].loc is
>>     used.  */
>> @@ -704,10 +723,8 @@ struct main_type
>>      struct type *self_type;
>>    } type_specific;
>>  
>> -  /* * Contains a location description value for the current type. Evaluating
>> -     this field yields to the location of the data for an object.  */
>> -
>> -  struct dynamic_prop *data_location;
>> +  /* * Contains all dynamic type attributes.  */
>> +  struct dynamic_prop_list *dyn_attribs;
> 
> We have been calling these "properties", so can we remain consistent
> with that? Can we rename the field to "prop_list", for instance?
> Or something else that you might prefer? Also, let's adjust the
> comment to use "property" as well.

Done.

> 
>>  };
>>  
>>  /* * A ``struct type'' describes a particular instance of a type, with
>> @@ -1221,7 +1238,7 @@ extern void allocate_gnat_aux_type (struct type *);
>>  
>>  /* Attribute accessors for the type data location.  */
>>  #define TYPE_DATA_LOCATION(thistype) \
>> -  TYPE_MAIN_TYPE(thistype)->data_location
>> +  get_dyn_attr (thistype, DYN_ATTR_DATA_LOCATION)
>>  #define TYPE_DATA_LOCATION_BATON(thistype) \
>>    TYPE_DATA_LOCATION (thistype)->data.baton
>>  #define TYPE_DATA_LOCATION_ADDR(thistype) \
> 
> IMO, I would remove all these macros, and let users call
> get_dyn_attr, and then manipulate the property using the generic
> macros that you're defining instead.
> 
> If it makes the patch bigger, let's keep that activity as patch #2.
> It can be done independently.
> 

I think it makes the patch bigger, as all callers need to be refactored. Not much work actually, but this is kind of unrelated to this patch. So I would like to keep it as is for now. Agree?

>> @@ -1229,6 +1246,17 @@ extern void allocate_gnat_aux_type (struct type *);
>>  #define TYPE_DATA_LOCATION_KIND(thistype) \
>>    TYPE_DATA_LOCATION (thistype)->kind
>>  
>> +  /* Attribute accessors for dynamic attributes.  */
> 
> Properties.

Done.

> 
>> +#define TYPE_DYN_ATTRIBS(thistype) \
>> +  TYPE_MAIN_TYPE(thistype)->dyn_attribs
>> +#define TYPE_DYN_ATTR_BATON(dynprop) \
>> +  dynprop->data.baton
>> +#define TYPE_DYN_ATTR_ADDR(dynprop) \
>> +  dynprop->data.const_val
>> +#define TYPE_DYN_ATTR_KIND(dynprop) \
>> +  dynprop->kind
> 
> Let's also s/_ATTR_/_PROP_/ if you don't mind.

Done.
> 
>>  /* Moto-specific stuff for FORTRAN arrays.  */
>>  
>>  #define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
>> @@ -1748,6 +1776,15 @@ extern struct type *resolve_dynamic_type (struct type *type, CORE_ADDR addr);
>>  /* * Predicate if the type has dynamic values, which are not resolved yet.  */
>>  extern int is_dynamic_type (struct type *type);
>>  
>> +/* * Fetches a dynamic attribute of a type out of the dynamic attribute
>> +   list.  */
>> +extern struct dynamic_prop * get_dyn_attr
>> +  (const struct type * type, enum dynamic_prop_kind kind);
> 
> "Fetches" -> "Fetch"; "attribute" -> "property";
> "out of" -> "from".
> Also, can you document the fact that the function returns NULL
> of the dynamic property cannnot be found.
> 
> Also, can you change the function's name to say "prop" instead of
> "attr"?

Done.

> 
> Please also fix the function's formatting. In this case, since this
> is only a declaration, the function's name should  NOT be at the start
> 

Isn't this the case here? I don't understand what to change here.

>> +/* * Adds a dynamic attribute to a type.  */
>> +extern void add_dyn_attr (struct type * type, struct objfile *objfile,
>> +  struct dynamic_prop prop, enum dynamic_prop_kind kind);
> 
> Same kind of remarks as above.
> 
>> +
>>  extern struct type *check_typedef (struct type *);
>>  
>>  #define CHECK_TYPEDEF(TYPE)			\
>> -- 
>> 1.7.9.5
> 
> Thanks again for the patch,
> 

Thanks,
Keven

Re: [PATCH] Introduce linked list for dynamic attributes

[Joel Brobecker]

> Here is the updated patch.

Thanks. This is looking very good, if I may say so myself ;-)

> From: Keven Boell <keven.boell@intel.com>
> Date: Mon, 23 Feb 2015 15:45:06 +0100
> Subject: [PATCH] Introduce linked list for dynamic attributes
> 
> This patch introduces a linked list for dynamic
> attributes of a type. This is a pre-work for
> the Fortran dynamic array support. The Fortran
> dynamic array support will add more dynamic
> attributes to a type. As only a few types
> will have such dynamic attributes set, a linked
> list is more efficient in terms of memory
> consumption than adding multiple attributes
> to main_type.
> 
> Transformed the data_location dynamic attribute
> to use the linked list.
> 
> 2015-02-23  Keven Boell  <keven.boell@intel.com>
> 
> 	* gdbtypes.c (resolve_dynamic_type_internal):
> 	Adapt data_location usage to linked list.
> 	(resolve_dynamic_type_internal): Adapt
> 	data_location usage to linked list.
> 	(get_dyn_prop): New function.
> 	(add_dyn_prop): New function.
> 	(copy_dynamic_prop_list): New function.
> 	(copy_type_recursive): Add copy of linked
> 	list.
> 	(copy_type): Add copy of linked list.
> 	* gdbtypes.h (enum dynamic_prop_node_kind):
> 	Kind of the dynamic attribute in linked list.
> 	(struct dynamic_prop_list): Dynamic list
> 	node.
> 	* dwarf2read.c (set_die_type): Add data_location
> 	data to linked list using helper functions.

A few more comments, but I hope that the next revision will
be approved.

One thing I should ask: I assume that you've run the testsuite
before and after your change to confirm there are no regressions,
right? Can you tell us what platform you tested on, just for
the record?

> ---
>  gdb/dwarf2read.c |    6 +---
>  gdb/gdbtypes.c   |   86 +++++++++++++++++++++++++++++++++++++++++-------------
>  gdb/gdbtypes.h   |   48 ++++++++++++++++++++++++++----
>  3 files changed, 108 insertions(+), 32 deletions(-)
> 
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 071f97b..370143d 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -22084,11 +22084,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
>    /* Read DW_AT_data_location and set in type.  */
>    attr = dwarf2_attr (die, DW_AT_data_location, cu);
>    if (attr_to_dynamic_prop (attr, die, cu, &prop))
> -    {
> -      TYPE_DATA_LOCATION (type)
> -        = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
> -      *TYPE_DATA_LOCATION (type) = prop;
> -    }
> +    add_dyn_prop (DYN_ATTR_DATA_LOCATION, prop, type, objfile);
>  
>    if (dwarf2_per_objfile->die_type_hash == NULL)
>      {
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 24f64be..20bdc20 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -2025,7 +2025,7 @@ resolve_dynamic_type_internal (struct type *type,
>  {
>    struct type *real_type = check_typedef (type);
>    struct type *resolved_type = type;
> -  const struct dynamic_prop *prop;
> +  struct dynamic_prop *prop;
>    CORE_ADDR value;
>  
>    if (!is_dynamic_type_internal (real_type, top_level))
> @@ -2080,13 +2080,11 @@ resolve_dynamic_type_internal (struct type *type,
>  
>    /* Resolve data_location attribute.  */
>    prop = TYPE_DATA_LOCATION (resolved_type);
> -  if (dwarf2_evaluate_property (prop, addr_stack, &value))
> +  if (prop != NULL && dwarf2_evaluate_property (prop, addr_stack, &value))
>      {
> -      TYPE_DATA_LOCATION_ADDR (resolved_type) = value;
> -      TYPE_DATA_LOCATION_KIND (resolved_type) = PROP_CONST;
> +      TYPE_DYN_PROP_ADDR (prop) = value;
> +      TYPE_DYN_PROP_KIND (prop) = PROP_CONST;
>      }
> -  else
> -    TYPE_DATA_LOCATION (resolved_type) = NULL;
>  
>    return resolved_type;
>  }
> @@ -2101,6 +2099,41 @@ resolve_dynamic_type (struct type *type, CORE_ADDR addr)
>    return resolve_dynamic_type_internal (type, &pinfo, 1);
>  }
>  
> +/* See gdbtypes.h  */
> +
> +struct dynamic_prop *
> +get_dyn_prop (enum dynamic_prop_node_kind prop_kind, const struct type *type)
> +{
> +  struct dynamic_prop_list *node
> +    = TYPE_DYN_PROP_LIST (type);

You can join these two lines into one.

> +
> +  while (node != NULL)
> +    {
> +      if (node->prop_kind == prop_kind)
> +        return node->prop;
> +      node = node->next;
> +    }
> +  return NULL;
> +}
> +
> +/* See gdbtypes.h  */
> +
> +void
> +add_dyn_prop (enum dynamic_prop_node_kind prop_kind, struct dynamic_prop prop,
> +              struct type *type, struct objfile *objfile)
> +{
> +  struct dynamic_prop_list *temp
> +    = obstack_alloc (&objfile->objfile_obstack,
> +              sizeof (struct dynamic_prop_list));

The formatting is a little off on this one (the "sizeof" is missalined).

Also, I forgot in the previous review to ask that we add an assert
to make sure that the type is objfile-owned. Would you mind doing
that? Otherwise, what we might be doing is allocating memory on
an obstack whose lifetime is independent of the type's thus potentially
creating a dangling pointer, which is always hard to track down after
the fact.

> +  temp->prop = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
> +  *temp->prop = prop;
> +  temp->prop_kind = prop_kind;
> +
> +  temp->next = TYPE_DYN_PROP_LIST (type);
> +  TYPE_DYN_PROP_LIST (type) = temp;
> +}
> +
> +
>  /* Find the real type of TYPE.  This function returns the real type,
>     after removing all layers of typedefs, and completing opaque or stub
>     types.  Completion changes the TYPE argument, but stripping of
> @@ -4222,6 +4255,25 @@ create_copied_types_hash (struct objfile *objfile)
>  			       dummy_obstack_deallocate);
>  }
>  
> +/* Recursively copy (deep copy) a dynamic attribute list of a type.  */
> +
> +struct dynamic_prop_list *
> +copy_dynamic_prop_list (struct dynamic_prop_list *list)
> +{

I think this function can be made static.

> +  struct dynamic_prop_list *copy;
> +
> +  copy = xmalloc (sizeof (struct dynamic_prop_list));
> +  *copy = *list;
> +
> +  copy->prop = xmalloc (sizeof (struct dynamic_prop));

xmalloc is the wrong function to be using, as we're allocating
the dynamic properties on the type's obfile instead.  Otherwise,
we'd be creating a memory leak. So I guess this function needs
to be given an obstack as parameter.

> +  *(copy->prop) = *(list->prop);

Looks like you forgot to copy the prop kind?

> +
> +  if (list->next != NULL)
> +    copy->next = copy_dynamic_prop_list (list->next);

I am reluctant to use recursion, here, as this is theoretically
open-ended, and can lead to stack over-consumption. Better to
use a simple loop, IMO. Less elegant, but I think it will protect
GDB's stack a little more.

> +
> +  return copy;
> +}
> +
>  /* Recursively copy (deep copy) TYPE, if it is associated with
>     OBJFILE.  Return a new type allocated using malloc, a saved type if
>     we have already visited TYPE (using COPIED_TYPES), or TYPE if it is
> @@ -4325,14 +4377,10 @@ copy_type_recursive (struct objfile *objfile,
>        *TYPE_RANGE_DATA (new_type) = *TYPE_RANGE_DATA (type);
>      }
>  
> -  /* Copy the data location information.  */
> -  if (TYPE_DATA_LOCATION (type) != NULL)
> -    {
> -      TYPE_DATA_LOCATION (new_type)
> -	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
> -      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
> -	      sizeof (struct dynamic_prop));
> -    }
> +  if (TYPE_DYN_PROP_LIST (type) != NULL)
> +    TYPE_DYN_PROP_LIST (new_type)
> +      = copy_dynamic_prop_list (TYPE_DYN_PROP_LIST (type));
> +
>  
>    /* Copy pointers to other types.  */
>    if (TYPE_TARGET_TYPE (type))
> @@ -4396,13 +4444,9 @@ copy_type (const struct type *type)
>    TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
>    memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
>  	  sizeof (struct main_type));
> -  if (TYPE_DATA_LOCATION (type) != NULL)
> -    {
> -      TYPE_DATA_LOCATION (new_type)
> -	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
> -      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
> -	      sizeof (struct dynamic_prop));
> -    }
> +  if (TYPE_DYN_PROP_LIST (type) != NULL)
> +    TYPE_DYN_PROP_LIST (new_type)
> +      = copy_dynamic_prop_list (TYPE_DYN_PROP_LIST (type));
>  
>    return new_type;
>  }
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 2c5ccf4..c852c5d 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -434,6 +434,21 @@ struct dynamic_prop
>    union dynamic_prop_data data;
>  };
>  
> +/* * Define a type's dynamic property node kind.  */
> +enum dynamic_prop_node_kind
> +{
> +  /* A property providing a type's data location.
> +     Evaluating this field yields to the location of an object's data.  */
> +  DYN_ATTR_DATA_LOCATION,
> +};
> +
> +/* * List for dynamic type attributes.  */
> +struct dynamic_prop_list
> +{
> +  enum dynamic_prop_node_kind prop_kind;
> +  struct dynamic_prop *prop;
> +  struct dynamic_prop_list *next;
> +};

OK with keeping it a pointer for now.

>  
>  /* * Determine which field of the union main_type.fields[x].loc is
>     used.  */
> @@ -719,10 +734,8 @@ struct main_type
>  
>    union type_specific type_specific;
>  
> -  /* * Contains a location description value for the current type. Evaluating
> -     this field yields to the location of the data for an object.  */
> -
> -  struct dynamic_prop *data_location;
> +  /* * Contains all dynamic type properties.  */
> +  struct dynamic_prop_list *dyn_prop_list;
>  };
>  
>  /* * A ``struct type'' describes a particular instance of a type, with
> @@ -1238,9 +1251,9 @@ extern void allocate_gnat_aux_type (struct type *);
>  #define TYPE_LOW_BOUND_KIND(range_type) \
>    TYPE_RANGE_DATA(range_type)->low.kind
>  
> -/* Attribute accessors for the type data location.  */
> +/* Property accessors for the type data location.  */
>  #define TYPE_DATA_LOCATION(thistype) \
> -  TYPE_MAIN_TYPE(thistype)->data_location
> +  get_dyn_prop (DYN_ATTR_DATA_LOCATION, thistype)
>  #define TYPE_DATA_LOCATION_BATON(thistype) \
>    TYPE_DATA_LOCATION (thistype)->data.baton
>  #define TYPE_DATA_LOCATION_ADDR(thistype) \
> @@ -1248,6 +1261,17 @@ extern void allocate_gnat_aux_type (struct type *);
>  #define TYPE_DATA_LOCATION_KIND(thistype) \
>    TYPE_DATA_LOCATION (thistype)->kind
>  
> +/* Attribute accessors for dynamic properties.  */
> +#define TYPE_DYN_PROP_LIST(thistype) \
> +  TYPE_MAIN_TYPE(thistype)->dyn_prop_list
> +#define TYPE_DYN_PROP_BATON(dynprop) \
> +  dynprop->data.baton
> +#define TYPE_DYN_PROP_ADDR(dynprop) \
> +  dynprop->data.const_val
> +#define TYPE_DYN_PROP_KIND(dynprop) \
> +  dynprop->kind
> +
> +
>  /* Moto-specific stuff for FORTRAN arrays.  */
>  
>  #define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
> @@ -1767,6 +1791,18 @@ extern struct type *resolve_dynamic_type (struct type *type, CORE_ADDR addr);
>  /* * Predicate if the type has dynamic values, which are not resolved yet.  */
>  extern int is_dynamic_type (struct type *type);
>  
> +/* * Fetch a dynamic property of a type from the dynamic attribute
> +   list.  */
> +extern struct dynamic_prop *get_dyn_prop
> +  (enum dynamic_prop_node_kind kind, const struct type *type);
> +
> +/* * Add a dynamic property to a type.  */
> +extern void add_dyn_prop (enum dynamic_prop_node_kind kind,
> +  struct dynamic_prop prop, struct type *type, struct objfile *objfile);

extern void add_dyn_prop
  (enum dynamic_prop_node_kind kind, struct dynamic_prop prop,
   struct type *type, struct objfile *objfile);

> +
> +extern struct dynamic_prop_list *copy_dynamic_prop_list
> +  (struct dynamic_prop_list *list);

You'll be able to delete this declaration.

> +
>  extern struct type *check_typedef (struct type *);
>  
>  #define CHECK_TYPEDEF(TYPE)			\
> -- 
> 1.7.9.5

-- 
Joel

Re: [PATCH] Introduce linked list for dynamic attributes

[Keven Boell]

Hi Joel,

On 03.03.2015 16:54, Joel Brobecker wrote:
>> Here is the updated patch.
> 
> Thanks. This is looking very good, if I may say so myself ;-)
> 
>> From: Keven Boell <keven.boell@intel.com>
>> Date: Mon, 23 Feb 2015 15:45:06 +0100
>> Subject: [PATCH] Introduce linked list for dynamic attributes
>>
>> This patch introduces a linked list for dynamic
>> attributes of a type. This is a pre-work for
>> the Fortran dynamic array support. The Fortran
>> dynamic array support will add more dynamic
>> attributes to a type. As only a few types
>> will have such dynamic attributes set, a linked
>> list is more efficient in terms of memory
>> consumption than adding multiple attributes
>> to main_type.
>>
>> Transformed the data_location dynamic attribute
>> to use the linked list.
>>
>> 2015-02-23  Keven Boell  <keven.boell@intel.com>
>>
>> 	* gdbtypes.c (resolve_dynamic_type_internal):
>> 	Adapt data_location usage to linked list.
>> 	(resolve_dynamic_type_internal): Adapt
>> 	data_location usage to linked list.
>> 	(get_dyn_prop): New function.
>> 	(add_dyn_prop): New function.
>> 	(copy_dynamic_prop_list): New function.
>> 	(copy_type_recursive): Add copy of linked
>> 	list.
>> 	(copy_type): Add copy of linked list.
>> 	* gdbtypes.h (enum dynamic_prop_node_kind):
>> 	Kind of the dynamic attribute in linked list.
>> 	(struct dynamic_prop_list): Dynamic list
>> 	node.
>> 	* dwarf2read.c (set_die_type): Add data_location
>> 	data to linked list using helper functions.
> 
> A few more comments, but I hope that the next revision will
> be approved.
> 
> One thing I should ask: I assume that you've run the testsuite
> before and after your change to confirm there are no regressions,
> right? Can you tell us what platform you tested on, just for
> the record?
> 

Yes, I executed the test suite on the following systems:
Ubuntu 12.04 64bit
Ubuntu 14.04 64bit
Fedora 17 64bit

No regressions on these systems. However, I see some flaky tests with and without my patch.

Any other recommended systems I should try?

>> ---
>>  gdb/dwarf2read.c |    6 +---
>>  gdb/gdbtypes.c   |   86 +++++++++++++++++++++++++++++++++++++++++-------------
>>  gdb/gdbtypes.h   |   48 ++++++++++++++++++++++++++----
>>  3 files changed, 108 insertions(+), 32 deletions(-)
>>
>> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
>> index 071f97b..370143d 100644
>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -22084,11 +22084,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
>>    /* Read DW_AT_data_location and set in type.  */
>>    attr = dwarf2_attr (die, DW_AT_data_location, cu);
>>    if (attr_to_dynamic_prop (attr, die, cu, &prop))
>> -    {
>> -      TYPE_DATA_LOCATION (type)
>> -        = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
>> -      *TYPE_DATA_LOCATION (type) = prop;
>> -    }
>> +    add_dyn_prop (DYN_ATTR_DATA_LOCATION, prop, type, objfile);
>>  
>>    if (dwarf2_per_objfile->die_type_hash == NULL)
>>      {
>> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
>> index 24f64be..20bdc20 100644
>> --- a/gdb/gdbtypes.c
>> +++ b/gdb/gdbtypes.c
>> @@ -2025,7 +2025,7 @@ resolve_dynamic_type_internal (struct type *type,
>>  {
>>    struct type *real_type = check_typedef (type);
>>    struct type *resolved_type = type;
>> -  const struct dynamic_prop *prop;
>> +  struct dynamic_prop *prop;
>>    CORE_ADDR value;
>>  
>>    if (!is_dynamic_type_internal (real_type, top_level))
>> @@ -2080,13 +2080,11 @@ resolve_dynamic_type_internal (struct type *type,
>>  
>>    /* Resolve data_location attribute.  */
>>    prop = TYPE_DATA_LOCATION (resolved_type);
>> -  if (dwarf2_evaluate_property (prop, addr_stack, &value))
>> +  if (prop != NULL && dwarf2_evaluate_property (prop, addr_stack, &value))
>>      {
>> -      TYPE_DATA_LOCATION_ADDR (resolved_type) = value;
>> -      TYPE_DATA_LOCATION_KIND (resolved_type) = PROP_CONST;
>> +      TYPE_DYN_PROP_ADDR (prop) = value;
>> +      TYPE_DYN_PROP_KIND (prop) = PROP_CONST;
>>      }
>> -  else
>> -    TYPE_DATA_LOCATION (resolved_type) = NULL;
>>  
>>    return resolved_type;
>>  }
>> @@ -2101,6 +2099,41 @@ resolve_dynamic_type (struct type *type, CORE_ADDR addr)
>>    return resolve_dynamic_type_internal (type, &pinfo, 1);
>>  }
>>  
>> +/* See gdbtypes.h  */
>> +
>> +struct dynamic_prop *
>> +get_dyn_prop (enum dynamic_prop_node_kind prop_kind, const struct type *type)
>> +{
>> +  struct dynamic_prop_list *node
>> +    = TYPE_DYN_PROP_LIST (type);
> 
> You can join these two lines into one.

Done.

> 
>> +
>> +  while (node != NULL)
>> +    {
>> +      if (node->prop_kind == prop_kind)
>> +        return node->prop;
>> +      node = node->next;
>> +    }
>> +  return NULL;
>> +}
>> +
>> +/* See gdbtypes.h  */
>> +
>> +void
>> +add_dyn_prop (enum dynamic_prop_node_kind prop_kind, struct dynamic_prop prop,
>> +              struct type *type, struct objfile *objfile)
>> +{
>> +  struct dynamic_prop_list *temp
>> +    = obstack_alloc (&objfile->objfile_obstack,
>> +              sizeof (struct dynamic_prop_list));
> 
> The formatting is a little off on this one (the "sizeof" is missalined).
> 
> Also, I forgot in the previous review to ask that we add an assert
> to make sure that the type is objfile-owned. Would you mind doing
> that? Otherwise, what we might be doing is allocating memory on
> an obstack whose lifetime is independent of the type's thus potentially
> creating a dangling pointer, which is always hard to track down after
> the fact.

Done.

> 
>> +  temp->prop = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
>> +  *temp->prop = prop;
>> +  temp->prop_kind = prop_kind;
>> +
>> +  temp->next = TYPE_DYN_PROP_LIST (type);
>> +  TYPE_DYN_PROP_LIST (type) = temp;
>> +}
>> +
>> +
>>  /* Find the real type of TYPE.  This function returns the real type,
>>     after removing all layers of typedefs, and completing opaque or stub
>>     types.  Completion changes the TYPE argument, but stripping of
>> @@ -4222,6 +4255,25 @@ create_copied_types_hash (struct objfile *objfile)
>>  			       dummy_obstack_deallocate);
>>  }
>>  
>> +/* Recursively copy (deep copy) a dynamic attribute list of a type.  */
>> +
>> +struct dynamic_prop_list *
>> +copy_dynamic_prop_list (struct dynamic_prop_list *list)
>> +{
> 
> I think this function can be made static.

Makes sense, done.

> 
>> +  struct dynamic_prop_list *copy;
>> +
>> +  copy = xmalloc (sizeof (struct dynamic_prop_list));
>> +  *copy = *list;
>> +
>> +  copy->prop = xmalloc (sizeof (struct dynamic_prop));
> 
> xmalloc is the wrong function to be using, as we're allocating
> the dynamic properties on the type's obfile instead.  Otherwise,
> we'd be creating a memory leak. So I guess this function needs
> to be given an obstack as parameter.
> 
>> +  *(copy->prop) = *(list->prop);
> 
> Looks like you forgot to copy the prop kind?

Good catch, added the prop kind.

> 
>> +
>> +  if (list->next != NULL)
>> +    copy->next = copy_dynamic_prop_list (list->next);
> 
> I am reluctant to use recursion, here, as this is theoretically
> open-ended, and can lead to stack over-consumption. Better to
> use a simple loop, IMO. Less elegant, but I think it will protect
> GDB's stack a little more.

Changed it to a while loop.

> 
>> +
>> +  return copy;
>> +}
>> +
>>  /* Recursively copy (deep copy) TYPE, if it is associated with
>>     OBJFILE.  Return a new type allocated using malloc, a saved type if
>>     we have already visited TYPE (using COPIED_TYPES), or TYPE if it is
>> @@ -4325,14 +4377,10 @@ copy_type_recursive (struct objfile *objfile,
>>        *TYPE_RANGE_DATA (new_type) = *TYPE_RANGE_DATA (type);
>>      }
>>  
>> -  /* Copy the data location information.  */
>> -  if (TYPE_DATA_LOCATION (type) != NULL)
>> -    {
>> -      TYPE_DATA_LOCATION (new_type)
>> -	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
>> -      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
>> -	      sizeof (struct dynamic_prop));
>> -    }
>> +  if (TYPE_DYN_PROP_LIST (type) != NULL)
>> +    TYPE_DYN_PROP_LIST (new_type)
>> +      = copy_dynamic_prop_list (TYPE_DYN_PROP_LIST (type));
>> +
>>  
>>    /* Copy pointers to other types.  */
>>    if (TYPE_TARGET_TYPE (type))
>> @@ -4396,13 +4444,9 @@ copy_type (const struct type *type)
>>    TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
>>    memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
>>  	  sizeof (struct main_type));
>> -  if (TYPE_DATA_LOCATION (type) != NULL)
>> -    {
>> -      TYPE_DATA_LOCATION (new_type)
>> -	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
>> -      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
>> -	      sizeof (struct dynamic_prop));
>> -    }
>> +  if (TYPE_DYN_PROP_LIST (type) != NULL)
>> +    TYPE_DYN_PROP_LIST (new_type)
>> +      = copy_dynamic_prop_list (TYPE_DYN_PROP_LIST (type));
>>  
>>    return new_type;
>>  }
>> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
>> index 2c5ccf4..c852c5d 100644
>> --- a/gdb/gdbtypes.h
>> +++ b/gdb/gdbtypes.h
>> @@ -434,6 +434,21 @@ struct dynamic_prop
>>    union dynamic_prop_data data;
>>  };
>>  
>> +/* * Define a type's dynamic property node kind.  */
>> +enum dynamic_prop_node_kind
>> +{
>> +  /* A property providing a type's data location.
>> +     Evaluating this field yields to the location of an object's data.  */
>> +  DYN_ATTR_DATA_LOCATION,
>> +};
>> +
>> +/* * List for dynamic type attributes.  */
>> +struct dynamic_prop_list
>> +{
>> +  enum dynamic_prop_node_kind prop_kind;
>> +  struct dynamic_prop *prop;
>> +  struct dynamic_prop_list *next;
>> +};
> 
> OK with keeping it a pointer for now.
> 
>>  
>>  /* * Determine which field of the union main_type.fields[x].loc is
>>     used.  */
>> @@ -719,10 +734,8 @@ struct main_type
>>  
>>    union type_specific type_specific;
>>  
>> -  /* * Contains a location description value for the current type. Evaluating
>> -     this field yields to the location of the data for an object.  */
>> -
>> -  struct dynamic_prop *data_location;
>> +  /* * Contains all dynamic type properties.  */
>> +  struct dynamic_prop_list *dyn_prop_list;
>>  };
>>  
>>  /* * A ``struct type'' describes a particular instance of a type, with
>> @@ -1238,9 +1251,9 @@ extern void allocate_gnat_aux_type (struct type *);
>>  #define TYPE_LOW_BOUND_KIND(range_type) \
>>    TYPE_RANGE_DATA(range_type)->low.kind
>>  
>> -/* Attribute accessors for the type data location.  */
>> +/* Property accessors for the type data location.  */
>>  #define TYPE_DATA_LOCATION(thistype) \
>> -  TYPE_MAIN_TYPE(thistype)->data_location
>> +  get_dyn_prop (DYN_ATTR_DATA_LOCATION, thistype)
>>  #define TYPE_DATA_LOCATION_BATON(thistype) \
>>    TYPE_DATA_LOCATION (thistype)->data.baton
>>  #define TYPE_DATA_LOCATION_ADDR(thistype) \
>> @@ -1248,6 +1261,17 @@ extern void allocate_gnat_aux_type (struct type *);
>>  #define TYPE_DATA_LOCATION_KIND(thistype) \
>>    TYPE_DATA_LOCATION (thistype)->kind
>>  
>> +/* Attribute accessors for dynamic properties.  */
>> +#define TYPE_DYN_PROP_LIST(thistype) \
>> +  TYPE_MAIN_TYPE(thistype)->dyn_prop_list
>> +#define TYPE_DYN_PROP_BATON(dynprop) \
>> +  dynprop->data.baton
>> +#define TYPE_DYN_PROP_ADDR(dynprop) \
>> +  dynprop->data.const_val
>> +#define TYPE_DYN_PROP_KIND(dynprop) \
>> +  dynprop->kind
>> +
>> +
>>  /* Moto-specific stuff for FORTRAN arrays.  */
>>  
>>  #define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
>> @@ -1767,6 +1791,18 @@ extern struct type *resolve_dynamic_type (struct type *type, CORE_ADDR addr);
>>  /* * Predicate if the type has dynamic values, which are not resolved yet.  */
>>  extern int is_dynamic_type (struct type *type);
>>  
>> +/* * Fetch a dynamic property of a type from the dynamic attribute
>> +   list.  */
>> +extern struct dynamic_prop *get_dyn_prop
>> +  (enum dynamic_prop_node_kind kind, const struct type *type);
>> +
>> +/* * Add a dynamic property to a type.  */
>> +extern void add_dyn_prop (enum dynamic_prop_node_kind kind,
>> +  struct dynamic_prop prop, struct type *type, struct objfile *objfile);
> 
> extern void add_dyn_prop
>   (enum dynamic_prop_node_kind kind, struct dynamic_prop prop,
>    struct type *type, struct objfile *objfile);

Done.

> 
>> +
>> +extern struct dynamic_prop_list *copy_dynamic_prop_list
>> +  (struct dynamic_prop_list *list);
> 
> You'll be able to delete this declaration.

Done.

> 
>> +
>>  extern struct type *check_typedef (struct type *);
>>  
>>  #define CHECK_TYPEDEF(TYPE)			\
>> -- 
>> 1.7.9.5
> 

The updated patch:

---
From: Keven Boell <keven.boell@intel.com>
Date: Mon, 23 Feb 2015 15:45:06 +0100
Subject: [PATCH] Introduce linked list for dynamic attributes

This patch introduces a linked list for dynamic
attributes of a type. This is a pre-work for
the Fortran dynamic array support. The Fortran
dynamic array support will add more dynamic
attributes to a type. As only a few types
will have such dynamic attributes set, a linked
list is more efficient in terms of memory
consumption than adding multiple attributes
to main_type.

Transformed the data_location dynamic attribute
to use the linked list.

2015-02-23  Keven Boell  <keven.boell@intel.com>

	* gdbtypes.c (resolve_dynamic_type_internal):
	Adapt data_location usage to linked list.
	(resolve_dynamic_type_internal): Adapt
	data_location usage to linked list.
	(get_dyn_prop): New function.
	(add_dyn_prop): New function.
	(copy_dynamic_prop_list): New function.
	(copy_type_recursive): Add copy of linked
	list.
	(copy_type): Add copy of linked list.
	* gdbtypes.h (enum dynamic_prop_node_kind):
	Kind of the dynamic attribute in linked list.
	(struct dynamic_prop_list): Dynamic list
	node.
	* dwarf2read.c (set_die_type): Add data_location
	data to linked list using helper functions.
---
 gdb/dwarf2read.c |    6 +---
 gdb/gdbtypes.c   |  102 +++++++++++++++++++++++++++++++++++++++++++-----------
 gdb/gdbtypes.h   |   46 ++++++++++++++++++++----
 3 files changed, 122 insertions(+), 32 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 071f97b..370143d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -22084,11 +22084,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   /* Read DW_AT_data_location and set in type.  */
   attr = dwarf2_attr (die, DW_AT_data_location, cu);
   if (attr_to_dynamic_prop (attr, die, cu, &prop))
-    {
-      TYPE_DATA_LOCATION (type)
-        = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
-      *TYPE_DATA_LOCATION (type) = prop;
-    }
+    add_dyn_prop (DYN_ATTR_DATA_LOCATION, prop, type, objfile);
 
   if (dwarf2_per_objfile->die_type_hash == NULL)
     {
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 24f64be..3f3a26c 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2025,7 +2025,7 @@ resolve_dynamic_type_internal (struct type *type,
 {
   struct type *real_type = check_typedef (type);
   struct type *resolved_type = type;
-  const struct dynamic_prop *prop;
+  struct dynamic_prop *prop;
   CORE_ADDR value;
 
   if (!is_dynamic_type_internal (real_type, top_level))
@@ -2080,13 +2080,11 @@ resolve_dynamic_type_internal (struct type *type,
 
   /* Resolve data_location attribute.  */
   prop = TYPE_DATA_LOCATION (resolved_type);
-  if (dwarf2_evaluate_property (prop, addr_stack, &value))
+  if (prop != NULL && dwarf2_evaluate_property (prop, addr_stack, &value))
     {
-      TYPE_DATA_LOCATION_ADDR (resolved_type) = value;
-      TYPE_DATA_LOCATION_KIND (resolved_type) = PROP_CONST;
+      TYPE_DYN_PROP_ADDR (prop) = value;
+      TYPE_DYN_PROP_KIND (prop) = PROP_CONST;
     }
-  else
-    TYPE_DATA_LOCATION (resolved_type) = NULL;
 
   return resolved_type;
 }
@@ -2101,6 +2099,44 @@ resolve_dynamic_type (struct type *type, CORE_ADDR addr)
   return resolve_dynamic_type_internal (type, &pinfo, 1);
 }
 
+/* See gdbtypes.h  */
+
+struct dynamic_prop *
+get_dyn_prop (enum dynamic_prop_node_kind prop_kind, const struct type *type)
+{
+  struct dynamic_prop_list *node = TYPE_DYN_PROP_LIST (type);
+
+  while (node != NULL)
+    {
+      if (node->prop_kind == prop_kind)
+        return node->prop;
+      node = node->next;
+    }
+  return NULL;
+}
+
+/* See gdbtypes.h  */
+
+void
+add_dyn_prop (enum dynamic_prop_node_kind prop_kind, struct dynamic_prop prop,
+              struct type *type, struct objfile *objfile)
+{
+  struct dynamic_prop_list *temp;
+
+  gdb_assert (TYPE_OBJFILE_OWNED (type));
+
+  temp = obstack_alloc (&objfile->objfile_obstack,
+    sizeof (struct dynamic_prop_list));
+
+  temp->prop = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
+  *temp->prop = prop;
+  temp->prop_kind = prop_kind;
+
+  temp->next = TYPE_DYN_PROP_LIST (type);
+  TYPE_DYN_PROP_LIST (type) = temp;
+}
+
+
 /* Find the real type of TYPE.  This function returns the real type,
    after removing all layers of typedefs, and completing opaque or stub
    types.  Completion changes the TYPE argument, but stripping of
@@ -4222,6 +4258,36 @@ create_copied_types_hash (struct objfile *objfile)
 			       dummy_obstack_deallocate);
 }
 
+/* Recursively copy (deep copy) a dynamic attribute list of a type.  */
+
+static struct dynamic_prop_list *
+copy_dynamic_prop_list (struct obstack *objfile_obstack,
+                        struct dynamic_prop_list *list)
+{
+  struct dynamic_prop_list *temp, *node = list, *copy = NULL, *prev = NULL;
+
+  while (node != NULL)
+    {
+      temp = obstack_alloc (objfile_obstack,
+               sizeof (struct dynamic_prop_list));
+      *temp = *node;
+      temp->prop = obstack_alloc (objfile_obstack,
+                     sizeof (struct dynamic_prop));
+      *(temp->prop) = *(node->prop);
+      temp->prop_kind = node->prop_kind;
+
+      if (prev == NULL)
+        copy = temp;
+      else
+        prev->next = temp;
+
+      prev = temp;
+      node = node->next;
+    }
+
+  return copy;
+}
+
 /* Recursively copy (deep copy) TYPE, if it is associated with
    OBJFILE.  Return a new type allocated using malloc, a saved type if
    we have already visited TYPE (using COPIED_TYPES), or TYPE if it is
@@ -4325,14 +4391,11 @@ copy_type_recursive (struct objfile *objfile,
       *TYPE_RANGE_DATA (new_type) = *TYPE_RANGE_DATA (type);
     }
 
-  /* Copy the data location information.  */
-  if (TYPE_DATA_LOCATION (type) != NULL)
-    {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
-    }
+  if (TYPE_DYN_PROP_LIST (type) != NULL)
+    TYPE_DYN_PROP_LIST (new_type)
+      = copy_dynamic_prop_list (&objfile->objfile_obstack,
+          TYPE_DYN_PROP_LIST (type));
+
 
   /* Copy pointers to other types.  */
   if (TYPE_TARGET_TYPE (type))
@@ -4396,13 +4459,10 @@ copy_type (const struct type *type)
   TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
   memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
 	  sizeof (struct main_type));
-  if (TYPE_DATA_LOCATION (type) != NULL)
-    {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
-    }
+  if (TYPE_DYN_PROP_LIST (type) != NULL)
+    TYPE_DYN_PROP_LIST (new_type)
+      = copy_dynamic_prop_list (&TYPE_OBJFILE (type) -> objfile_obstack,
+          TYPE_DYN_PROP_LIST (type));
 
   return new_type;
 }
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 2c5ccf4..12c393f 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -434,6 +434,21 @@ struct dynamic_prop
   union dynamic_prop_data data;
 };
 
+/* * Define a type's dynamic property node kind.  */
+enum dynamic_prop_node_kind
+{
+  /* A property providing a type's data location.
+     Evaluating this field yields to the location of an object's data.  */
+  DYN_ATTR_DATA_LOCATION,
+};
+
+/* * List for dynamic type attributes.  */
+struct dynamic_prop_list
+{
+  enum dynamic_prop_node_kind prop_kind;
+  struct dynamic_prop *prop;
+  struct dynamic_prop_list *next;
+};
 
 /* * Determine which field of the union main_type.fields[x].loc is
    used.  */
@@ -719,10 +734,8 @@ struct main_type
 
   union type_specific type_specific;
 
-  /* * Contains a location description value for the current type. Evaluating
-     this field yields to the location of the data for an object.  */
-
-  struct dynamic_prop *data_location;
+  /* * Contains all dynamic type properties.  */
+  struct dynamic_prop_list *dyn_prop_list;
 };
 
 /* * A ``struct type'' describes a particular instance of a type, with
@@ -1238,9 +1251,9 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_LOW_BOUND_KIND(range_type) \
   TYPE_RANGE_DATA(range_type)->low.kind
 
-/* Attribute accessors for the type data location.  */
+/* Property accessors for the type data location.  */
 #define TYPE_DATA_LOCATION(thistype) \
-  TYPE_MAIN_TYPE(thistype)->data_location
+  get_dyn_prop (DYN_ATTR_DATA_LOCATION, thistype)
 #define TYPE_DATA_LOCATION_BATON(thistype) \
   TYPE_DATA_LOCATION (thistype)->data.baton
 #define TYPE_DATA_LOCATION_ADDR(thistype) \
@@ -1248,6 +1261,17 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_DATA_LOCATION_KIND(thistype) \
   TYPE_DATA_LOCATION (thistype)->kind
 
+/* Attribute accessors for dynamic properties.  */
+#define TYPE_DYN_PROP_LIST(thistype) \
+  TYPE_MAIN_TYPE(thistype)->dyn_prop_list
+#define TYPE_DYN_PROP_BATON(dynprop) \
+  dynprop->data.baton
+#define TYPE_DYN_PROP_ADDR(dynprop) \
+  dynprop->data.const_val
+#define TYPE_DYN_PROP_KIND(dynprop) \
+  dynprop->kind
+
+
 /* Moto-specific stuff for FORTRAN arrays.  */
 
 #define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
@@ -1767,6 +1791,16 @@ extern struct type *resolve_dynamic_type (struct type *type, CORE_ADDR addr);
 /* * Predicate if the type has dynamic values, which are not resolved yet.  */
 extern int is_dynamic_type (struct type *type);
 
+/* * Fetch a dynamic property of a type from the dynamic attribute
+   list.  */
+extern struct dynamic_prop *get_dyn_prop
+  (enum dynamic_prop_node_kind kind, const struct type *type);
+
+/* * Add a dynamic property to a type.  */
+extern void add_dyn_prop
+  (enum dynamic_prop_node_kind kind, struct dynamic_prop prop,
+   struct type *type, struct objfile *objfile);
+
 extern struct type *check_typedef (struct type *);
 
 #define CHECK_TYPEDEF(TYPE)			\
-- 
1.7.9.5


Thanks,
Keven

Re: [PATCH] Introduce linked list for dynamic attributes

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1553 bytes --]

> Yes, I executed the test suite on the following systems:
> Ubuntu 12.04 64bit
> Ubuntu 14.04 64bit
> Fedora 17 64bit
> 
> No regressions on these systems. However, I see some flaky tests with and without my patch.
> 
> Any other recommended systems I should try?

No, that's good  enough. Some tests are indeed still a little flaky
for me too (Eg: attach-many-short-lived-threads.exp).

Here is an updated version to your patch. Here are the list of
changes I made:

  . I fixed the formatting in a number of function calls where
    the subsequent parameters where misaligned. Eg:

       |   temp = obstack_alloc (&objfile->objfile_obstack,
       |-    sizeof (struct dynamic_prop_list));
       |+                        sizeof (struct dynamic_prop_list));

    (the second argument should be aligned to the first one).

  . I simplified add_dyn_prop by using obstack_copy.

  . I rewrote copy_dynamic_prop_list in a simpler way. But I'd like
    you to review my implementation, in case there is a flaw in my
    logic.

  . I swapped the order of the parameters in copy_dynamic_prop_list.

  . I added more documentation to to the various types we are adding.
    It's fairly trivial, but allows us to more closely follow the
    project's guidelines.

If you are happy with that, and you agree with copy_dynamic_prop_list's
new implementation, then why don't you write the revision log and
ChangeLog and resubmit. I'll take one last look and we can move on
to the next phase.

I've tested this version on x86_64-linux, no regression.

-- 
Joel

[-- Attachment #2: 0001-WIP-Introduce-linked-list-for-dynamic-attributes.patch --]
[-- Type: text/x-diff, Size: 9212 bytes --]

From 8b0030afcfc11d35e040cf999d5bbe4705cf2b7c Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 5 Mar 2015 08:18:43 -0800
Subject: [PATCH] WIP: Introduce linked list for dynamic attributes

---
 gdb/dwarf2read.c |  6 +---
 gdb/gdbtypes.c   | 96 +++++++++++++++++++++++++++++++++++++++++++-------------
 gdb/gdbtypes.h   | 57 +++++++++++++++++++++++++++++----
 3 files changed, 127 insertions(+), 32 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 071f97b..370143d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -22084,11 +22084,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   /* Read DW_AT_data_location and set in type.  */
   attr = dwarf2_attr (die, DW_AT_data_location, cu);
   if (attr_to_dynamic_prop (attr, die, cu, &prop))
-    {
-      TYPE_DATA_LOCATION (type)
-        = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
-      *TYPE_DATA_LOCATION (type) = prop;
-    }
+    add_dyn_prop (DYN_ATTR_DATA_LOCATION, prop, type, objfile);
 
   if (dwarf2_per_objfile->die_type_hash == NULL)
     {
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 24f64be..67d29d7 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2025,7 +2025,7 @@ resolve_dynamic_type_internal (struct type *type,
 {
   struct type *real_type = check_typedef (type);
   struct type *resolved_type = type;
-  const struct dynamic_prop *prop;
+  struct dynamic_prop *prop;
   CORE_ADDR value;
 
   if (!is_dynamic_type_internal (real_type, top_level))
@@ -2080,13 +2080,11 @@ resolve_dynamic_type_internal (struct type *type,
 
   /* Resolve data_location attribute.  */
   prop = TYPE_DATA_LOCATION (resolved_type);
-  if (dwarf2_evaluate_property (prop, addr_stack, &value))
+  if (prop != NULL && dwarf2_evaluate_property (prop, addr_stack, &value))
     {
-      TYPE_DATA_LOCATION_ADDR (resolved_type) = value;
-      TYPE_DATA_LOCATION_KIND (resolved_type) = PROP_CONST;
+      TYPE_DYN_PROP_ADDR (prop) = value;
+      TYPE_DYN_PROP_KIND (prop) = PROP_CONST;
     }
-  else
-    TYPE_DATA_LOCATION (resolved_type) = NULL;
 
   return resolved_type;
 }
@@ -2101,6 +2099,42 @@ resolve_dynamic_type (struct type *type, CORE_ADDR addr)
   return resolve_dynamic_type_internal (type, &pinfo, 1);
 }
 
+/* See gdbtypes.h  */
+
+struct dynamic_prop *
+get_dyn_prop (enum dynamic_prop_node_kind prop_kind, const struct type *type)
+{
+  struct dynamic_prop_list *node = TYPE_DYN_PROP_LIST (type);
+
+  while (node != NULL)
+    {
+      if (node->prop_kind == prop_kind)
+        return node->prop;
+      node = node->next;
+    }
+  return NULL;
+}
+
+/* See gdbtypes.h  */
+
+void
+add_dyn_prop (enum dynamic_prop_node_kind prop_kind, struct dynamic_prop prop,
+              struct type *type, struct objfile *objfile)
+{
+  struct dynamic_prop_list *temp;
+
+  gdb_assert (TYPE_OBJFILE_OWNED (type));
+
+  temp = obstack_alloc (&objfile->objfile_obstack,
+			sizeof (struct dynamic_prop_list));
+  temp->prop_kind = prop_kind;
+  temp->prop = obstack_copy (&objfile->objfile_obstack, &prop, sizeof (prop));
+  temp->next = TYPE_DYN_PROP_LIST (type);
+
+  TYPE_DYN_PROP_LIST (type) = temp;
+}
+
+
 /* Find the real type of TYPE.  This function returns the real type,
    after removing all layers of typedefs, and completing opaque or stub
    types.  Completion changes the TYPE argument, but stripping of
@@ -4222,6 +4256,32 @@ create_copied_types_hash (struct objfile *objfile)
 			       dummy_obstack_deallocate);
 }
 
+/* Recursively copy (deep copy) LIST on the given OBJFILE_OBSTACK.
+   Return the address of the copy.  */
+
+static struct dynamic_prop_list *
+copy_dynamic_prop_list (struct dynamic_prop_list *list,
+			struct obstack *objfile_obstack)
+{
+  struct dynamic_prop_list *copy = list;
+  struct dynamic_prop_list **node_ptr = &copy;
+
+  while (*node_ptr != NULL)
+    {
+      struct dynamic_prop_list *node_copy;
+
+      node_copy = obstack_copy (objfile_obstack, *node_ptr,
+				sizeof (struct dynamic_prop_list));
+      node_copy->prop = obstack_copy (objfile_obstack, (*node_ptr)->prop,
+				      sizeof (struct dynamic_prop));
+      *node_ptr = node_copy;
+
+      node_ptr = &node_copy->next;
+    }
+
+  return copy;
+}
+
 /* Recursively copy (deep copy) TYPE, if it is associated with
    OBJFILE.  Return a new type allocated using malloc, a saved type if
    we have already visited TYPE (using COPIED_TYPES), or TYPE if it is
@@ -4325,14 +4385,11 @@ copy_type_recursive (struct objfile *objfile,
       *TYPE_RANGE_DATA (new_type) = *TYPE_RANGE_DATA (type);
     }
 
-  /* Copy the data location information.  */
-  if (TYPE_DATA_LOCATION (type) != NULL)
-    {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
-    }
+  if (TYPE_DYN_PROP_LIST (type) != NULL)
+    TYPE_DYN_PROP_LIST (new_type)
+      = copy_dynamic_prop_list (TYPE_DYN_PROP_LIST (type),
+				&objfile->objfile_obstack);
+
 
   /* Copy pointers to other types.  */
   if (TYPE_TARGET_TYPE (type))
@@ -4396,13 +4453,10 @@ copy_type (const struct type *type)
   TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
   memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
 	  sizeof (struct main_type));
-  if (TYPE_DATA_LOCATION (type) != NULL)
-    {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
-    }
+  if (TYPE_DYN_PROP_LIST (type) != NULL)
+    TYPE_DYN_PROP_LIST (new_type)
+      = copy_dynamic_prop_list (TYPE_DYN_PROP_LIST (type),
+				&TYPE_OBJFILE (type)->objfile_obstack);
 
   return new_type;
 }
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 2c5ccf4..ceb77af 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -434,6 +434,26 @@ struct dynamic_prop
   union dynamic_prop_data data;
 };
 
+/* * Define a type's dynamic property node kind.  */
+enum dynamic_prop_node_kind
+{
+  /* A property providing a type's data location.
+     Evaluating this field yields to the location of an object's data.  */
+  DYN_ATTR_DATA_LOCATION,
+};
+
+/* * A list of dynamic properties for a given type.  */
+struct dynamic_prop_list
+{
+  /* The kind of dynamic prop in this node.  */
+  enum dynamic_prop_node_kind prop_kind;
+
+  /* The dynamic property itself.  */
+  struct dynamic_prop *prop;
+
+  /* A pointer to the next dynamic property.  */
+  struct dynamic_prop_list *next;
+};
 
 /* * Determine which field of the union main_type.fields[x].loc is
    used.  */
@@ -719,10 +739,8 @@ struct main_type
 
   union type_specific type_specific;
 
-  /* * Contains a location description value for the current type. Evaluating
-     this field yields to the location of the data for an object.  */
-
-  struct dynamic_prop *data_location;
+  /* * Contains all dynamic type properties.  */
+  struct dynamic_prop_list *dyn_prop_list;
 };
 
 /* * A ``struct type'' describes a particular instance of a type, with
@@ -1238,9 +1256,9 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_LOW_BOUND_KIND(range_type) \
   TYPE_RANGE_DATA(range_type)->low.kind
 
-/* Attribute accessors for the type data location.  */
+/* Property accessors for the type data location.  */
 #define TYPE_DATA_LOCATION(thistype) \
-  TYPE_MAIN_TYPE(thistype)->data_location
+  get_dyn_prop (DYN_ATTR_DATA_LOCATION, thistype)
 #define TYPE_DATA_LOCATION_BATON(thistype) \
   TYPE_DATA_LOCATION (thistype)->data.baton
 #define TYPE_DATA_LOCATION_ADDR(thistype) \
@@ -1248,6 +1266,17 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_DATA_LOCATION_KIND(thistype) \
   TYPE_DATA_LOCATION (thistype)->kind
 
+/* Attribute accessors for dynamic properties.  */
+#define TYPE_DYN_PROP_LIST(thistype) \
+  TYPE_MAIN_TYPE(thistype)->dyn_prop_list
+#define TYPE_DYN_PROP_BATON(dynprop) \
+  dynprop->data.baton
+#define TYPE_DYN_PROP_ADDR(dynprop) \
+  dynprop->data.const_val
+#define TYPE_DYN_PROP_KIND(dynprop) \
+  dynprop->kind
+
+
 /* Moto-specific stuff for FORTRAN arrays.  */
 
 #define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
@@ -1767,6 +1796,22 @@ extern struct type *resolve_dynamic_type (struct type *type, CORE_ADDR addr);
 /* * Predicate if the type has dynamic values, which are not resolved yet.  */
 extern int is_dynamic_type (struct type *type);
 
+/* * Return the dynamic property of the requested KIND from TYPE's
+   list of dynamic properties.
+
+   Return NULL if TYPE does not have such dynamic property.  */
+extern struct dynamic_prop *get_dyn_prop
+  (enum dynamic_prop_node_kind kind, const struct type *type);
+
+/* * Given a dynamic property PROP of a given KIND, add this dynamic
+   property to the given TYPE.
+
+   This function assumes that TYPE is objfile-owned, and that OBJFILE
+   is the TYPE's objfile.  */
+extern void add_dyn_prop
+  (enum dynamic_prop_node_kind kind, struct dynamic_prop prop,
+   struct type *type, struct objfile *objfile);
+
 extern struct type *check_typedef (struct type *);
 
 #define CHECK_TYPEDEF(TYPE)			\
-- 
1.9.1

Re: [PATCH] Introduce linked list for dynamic attributes

[Keven Boell]

On 05.03.2015 19:13, Joel Brobecker wrote:
>> Yes, I executed the test suite on the following systems:
>> Ubuntu 12.04 64bit
>> Ubuntu 14.04 64bit
>> Fedora 17 64bit
>>
>> No regressions on these systems. However, I see some flaky tests with and without my patch.
>>
>> Any other recommended systems I should try?
> 
> No, that's good  enough. Some tests are indeed still a little flaky
> for me too (Eg: attach-many-short-lived-threads.exp).
> 
> Here is an updated version to your patch. Here are the list of
> changes I made:

Thanks!

> 
>   . I fixed the formatting in a number of function calls where
>     the subsequent parameters where misaligned. Eg:
> 
>        |   temp = obstack_alloc (&objfile->objfile_obstack,
>        |-    sizeof (struct dynamic_prop_list));
>        |+                        sizeof (struct dynamic_prop_list));
> 
>     (the second argument should be aligned to the first one).
> 
>   . I simplified add_dyn_prop by using obstack_copy.
> 
>   . I rewrote copy_dynamic_prop_list in a simpler way. But I'd like
>     you to review my implementation, in case there is a flaw in my
>     logic.

Looks good to me.

> 
>   . I swapped the order of the parameters in copy_dynamic_prop_list.

I would like to keep the order as is as, e.g. copy_type_recursive has the same order.

> 
>   . I added more documentation to to the various types we are adding.
>     It's fairly trivial, but allows us to more closely follow the
>     project's guidelines.
> 
> If you are happy with that, and you agree with copy_dynamic_prop_list's
> new implementation, then why don't you write the revision log and
> ChangeLog and resubmit. I'll take one last look and we can move on
> to the next phase.
> 
> I've tested this version on x86_64-linux, no regression.
> 

Updated patch:
--
From: Keven Boell <keven.boell@intel.com>
Date: Mon, 23 Feb 2015 15:45:06 +0100
Subject: [PATCH] Introduce linked list for dynamic attributes

This patch introduces a linked list for dynamic
attributes of a type. This is a pre-work for
the Fortran dynamic array support. The Fortran
dynamic array support will add more dynamic
attributes to a type. As only a few types
will have such dynamic attributes set, a linked
list is more efficient in terms of memory
consumption than adding multiple attributes
to main_type.

Transformed the data_location dynamic attribute
to use the linked list.

2015-02-23  Keven Boell  <keven.boell@intel.com>

	* gdbtypes.c (resolve_dynamic_type_internal):
	Adapt data_location usage to linked list.
	(resolve_dynamic_type_internal): Adapt
	data_location usage to linked list.
	(get_dyn_prop): New function.
	(add_dyn_prop): New function.
	(copy_dynamic_prop_list): New function.
	(copy_type_recursive): Add copy of linked
	list.
	(copy_type): Add copy of linked list.
	* gdbtypes.h (enum dynamic_prop_node_kind):
	Kind of the dynamic attribute in linked list.
	(struct dynamic_prop_list): Dynamic list
	node.
	* dwarf2read.c (set_die_type): Add data_location
	data to linked list using helper functions.
---
 gdb/ChangeLog    |   19 +++++++++++
 gdb/dwarf2read.c |    6 +---
 gdb/gdbtypes.c   |   95 ++++++++++++++++++++++++++++++++++++++++++------------
 gdb/gdbtypes.h   |   55 +++++++++++++++++++++++++++----
 4 files changed, 143 insertions(+), 32 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1db98ff..b3d39cb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,22 @@
+2015-03-10  Keven Boell  <keven.boell@intel.com>
+
+	* gdbtypes.c (resolve_dynamic_type_internal):
+	Adapt data_location usage to linked list.
+	(resolve_dynamic_type_internal): Adapt
+	data_location usage to linked list.
+	(get_dyn_prop): New function.
+	(add_dyn_prop): New function.
+	(copy_dynamic_prop_list): New function.
+	(copy_type_recursive): Add copy of linked
+	list.
+	(copy_type): Add copy of linked list.
+	* gdbtypes.h (enum dynamic_prop_node_kind):
+	Kind of the dynamic attribute in linked list.
+	(struct dynamic_prop_list): Dynamic list
+	node.
+	* dwarf2read.c (set_die_type): Add data_location
+	data to linked list using helper functions.
+
 2015-03-03  Markus Metzger  <markus.t.metzger@intel.com>
 
 	* nat/linux-btrace.c: Include sys/utsname.h.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 071f97b..370143d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -22084,11 +22084,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   /* Read DW_AT_data_location and set in type.  */
   attr = dwarf2_attr (die, DW_AT_data_location, cu);
   if (attr_to_dynamic_prop (attr, die, cu, &prop))
-    {
-      TYPE_DATA_LOCATION (type)
-        = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
-      *TYPE_DATA_LOCATION (type) = prop;
-    }
+    add_dyn_prop (DYN_ATTR_DATA_LOCATION, prop, type, objfile);
 
   if (dwarf2_per_objfile->die_type_hash == NULL)
     {
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 24f64be..fd0ae66 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2025,7 +2025,7 @@ resolve_dynamic_type_internal (struct type *type,
 {
   struct type *real_type = check_typedef (type);
   struct type *resolved_type = type;
-  const struct dynamic_prop *prop;
+  struct dynamic_prop *prop;
   CORE_ADDR value;
 
   if (!is_dynamic_type_internal (real_type, top_level))
@@ -2080,13 +2080,11 @@ resolve_dynamic_type_internal (struct type *type,
 
   /* Resolve data_location attribute.  */
   prop = TYPE_DATA_LOCATION (resolved_type);
-  if (dwarf2_evaluate_property (prop, addr_stack, &value))
+  if (prop != NULL && dwarf2_evaluate_property (prop, addr_stack, &value))
     {
-      TYPE_DATA_LOCATION_ADDR (resolved_type) = value;
-      TYPE_DATA_LOCATION_KIND (resolved_type) = PROP_CONST;
+      TYPE_DYN_PROP_ADDR (prop) = value;
+      TYPE_DYN_PROP_KIND (prop) = PROP_CONST;
     }
-  else
-    TYPE_DATA_LOCATION (resolved_type) = NULL;
 
   return resolved_type;
 }
@@ -2101,6 +2099,42 @@ resolve_dynamic_type (struct type *type, CORE_ADDR addr)
   return resolve_dynamic_type_internal (type, &pinfo, 1);
 }
 
+/* See gdbtypes.h  */
+
+struct dynamic_prop *
+get_dyn_prop (enum dynamic_prop_node_kind prop_kind, const struct type *type)
+{
+  struct dynamic_prop_list *node = TYPE_DYN_PROP_LIST (type);
+
+  while (node != NULL)
+    {
+      if (node->prop_kind == prop_kind)
+        return node->prop;
+      node = node->next;
+    }
+  return NULL;
+}
+
+/* See gdbtypes.h  */
+
+void
+add_dyn_prop (enum dynamic_prop_node_kind prop_kind, struct dynamic_prop prop,
+              struct type *type, struct objfile *objfile)
+{
+  struct dynamic_prop_list *temp;
+
+  gdb_assert (TYPE_OBJFILE_OWNED (type));
+
+  temp = obstack_alloc (&objfile->objfile_obstack,
+			sizeof (struct dynamic_prop_list));
+  temp->prop_kind = prop_kind;
+  temp->prop = obstack_copy (&objfile->objfile_obstack, &prop, sizeof (prop));
+  temp->next = TYPE_DYN_PROP_LIST (type);
+
+  TYPE_DYN_PROP_LIST (type) = temp;
+}
+
+
 /* Find the real type of TYPE.  This function returns the real type,
    after removing all layers of typedefs, and completing opaque or stub
    types.  Completion changes the TYPE argument, but stripping of
@@ -4222,6 +4256,31 @@ create_copied_types_hash (struct objfile *objfile)
 			       dummy_obstack_deallocate);
 }
 
+/* Recursively copy (deep copy) a dynamic attribute list of a type.  */
+
+static struct dynamic_prop_list *
+copy_dynamic_prop_list (struct obstack *objfile_obstack,
+			struct dynamic_prop_list *list)
+{
+  struct dynamic_prop_list *copy = list;
+  struct dynamic_prop_list **node_ptr = &copy;
+
+  while (*node_ptr != NULL)
+    {
+      struct dynamic_prop_list *node_copy;
+
+      node_copy = obstack_copy (objfile_obstack, *node_ptr,
+				sizeof (struct dynamic_prop_list));
+      node_copy->prop = obstack_copy (objfile_obstack, (*node_ptr)->prop,
+				      sizeof (struct dynamic_prop));
+      *node_ptr = node_copy;
+
+      node_ptr = &node_copy->next;
+    }
+
+  return copy;
+}
+
 /* Recursively copy (deep copy) TYPE, if it is associated with
    OBJFILE.  Return a new type allocated using malloc, a saved type if
    we have already visited TYPE (using COPIED_TYPES), or TYPE if it is
@@ -4325,14 +4384,11 @@ copy_type_recursive (struct objfile *objfile,
       *TYPE_RANGE_DATA (new_type) = *TYPE_RANGE_DATA (type);
     }
 
-  /* Copy the data location information.  */
-  if (TYPE_DATA_LOCATION (type) != NULL)
-    {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
-    }
+  if (TYPE_DYN_PROP_LIST (type) != NULL)
+    TYPE_DYN_PROP_LIST (new_type)
+      = copy_dynamic_prop_list (&objfile->objfile_obstack,
+				TYPE_DYN_PROP_LIST (type));
+
 
   /* Copy pointers to other types.  */
   if (TYPE_TARGET_TYPE (type))
@@ -4396,13 +4452,10 @@ copy_type (const struct type *type)
   TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
   memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
 	  sizeof (struct main_type));
-  if (TYPE_DATA_LOCATION (type) != NULL)
-    {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
-    }
+  if (TYPE_DYN_PROP_LIST (type) != NULL)
+    TYPE_DYN_PROP_LIST (new_type)
+      = copy_dynamic_prop_list (&TYPE_OBJFILE (type) -> objfile_obstack,
+				TYPE_DYN_PROP_LIST (type));
 
   return new_type;
 }
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 2c5ccf4..79d72df 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -434,6 +434,26 @@ struct dynamic_prop
   union dynamic_prop_data data;
 };
 
+/* * Define a type's dynamic property node kind.  */
+enum dynamic_prop_node_kind
+{
+  /* A property providing a type's data location.
+     Evaluating this field yields to the location of an object's data.  */
+  DYN_ATTR_DATA_LOCATION,
+};
+
+/* * List for dynamic type attributes.  */
+struct dynamic_prop_list
+{
+  /* The kind of dynamic prop in this node.  */
+  enum dynamic_prop_node_kind prop_kind;
+
+  /* The dynamic property itself.  */
+  struct dynamic_prop *prop;
+
+  /* A pointer to the next dynamic property.  */
+  struct dynamic_prop_list *next;
+};
 
 /* * Determine which field of the union main_type.fields[x].loc is
    used.  */
@@ -719,10 +739,8 @@ struct main_type
 
   union type_specific type_specific;
 
-  /* * Contains a location description value for the current type. Evaluating
-     this field yields to the location of the data for an object.  */
-
-  struct dynamic_prop *data_location;
+  /* * Contains all dynamic type properties.  */
+  struct dynamic_prop_list *dyn_prop_list;
 };
 
 /* * A ``struct type'' describes a particular instance of a type, with
@@ -1238,9 +1256,9 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_LOW_BOUND_KIND(range_type) \
   TYPE_RANGE_DATA(range_type)->low.kind
 
-/* Attribute accessors for the type data location.  */
+/* Property accessors for the type data location.  */
 #define TYPE_DATA_LOCATION(thistype) \
-  TYPE_MAIN_TYPE(thistype)->data_location
+  get_dyn_prop (DYN_ATTR_DATA_LOCATION, thistype)
 #define TYPE_DATA_LOCATION_BATON(thistype) \
   TYPE_DATA_LOCATION (thistype)->data.baton
 #define TYPE_DATA_LOCATION_ADDR(thistype) \
@@ -1248,6 +1266,17 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_DATA_LOCATION_KIND(thistype) \
   TYPE_DATA_LOCATION (thistype)->kind
 
+/* Attribute accessors for dynamic properties.  */
+#define TYPE_DYN_PROP_LIST(thistype) \
+  TYPE_MAIN_TYPE(thistype)->dyn_prop_list
+#define TYPE_DYN_PROP_BATON(dynprop) \
+  dynprop->data.baton
+#define TYPE_DYN_PROP_ADDR(dynprop) \
+  dynprop->data.const_val
+#define TYPE_DYN_PROP_KIND(dynprop) \
+  dynprop->kind
+
+
 /* Moto-specific stuff for FORTRAN arrays.  */
 
 #define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
@@ -1767,6 +1796,20 @@ extern struct type *resolve_dynamic_type (struct type *type, CORE_ADDR addr);
 /* * Predicate if the type has dynamic values, which are not resolved yet.  */
 extern int is_dynamic_type (struct type *type);
 
+/* * Return the dynamic property of the requested KIND from TYPE's
+   list of dynamic properties.  */
+extern struct dynamic_prop *get_dyn_prop
+  (enum dynamic_prop_node_kind kind, const struct type *type);
+
+/* * Given a dynamic property PROP of a given KIND, add this dynamic
+   property to the given TYPE.
+
+   This function assumes that TYPE is objfile-owned, and that OBJFILE
+   is the TYPE's objfile.  */
+extern void add_dyn_prop
+  (enum dynamic_prop_node_kind kind, struct dynamic_prop prop,
+   struct type *type, struct objfile *objfile);
+
 extern struct type *check_typedef (struct type *);
 
 #define CHECK_TYPEDEF(TYPE)			\
-- 
1.7.9.5

Re: [PATCH] Introduce linked list for dynamic attributes

[Joel Brobecker]

> From: Keven Boell <keven.boell@intel.com>
> Date: Mon, 23 Feb 2015 15:45:06 +0100
> Subject: [PATCH] Introduce linked list for dynamic attributes
> 
> This patch introduces a linked list for dynamic
> attributes of a type. This is a pre-work for
> the Fortran dynamic array support. The Fortran
> dynamic array support will add more dynamic
> attributes to a type. As only a few types
> will have such dynamic attributes set, a linked
> list is more efficient in terms of memory
> consumption than adding multiple attributes
> to main_type.
> 
> Transformed the data_location dynamic attribute
> to use the linked list.
> 
> 2015-02-23  Keven Boell  <keven.boell@intel.com>
> 
> 	* gdbtypes.c (resolve_dynamic_type_internal):
> 	Adapt data_location usage to linked list.
> 	(resolve_dynamic_type_internal): Adapt
> 	data_location usage to linked list.
> 	(get_dyn_prop): New function.
> 	(add_dyn_prop): New function.
> 	(copy_dynamic_prop_list): New function.
> 	(copy_type_recursive): Add copy of linked
> 	list.
> 	(copy_type): Add copy of linked list.
> 	* gdbtypes.h (enum dynamic_prop_node_kind):
> 	Kind of the dynamic attribute in linked list.
> 	(struct dynamic_prop_list): Dynamic list
> 	node.
> 	* dwarf2read.c (set_die_type): Add data_location
> 	data to linked list using helper functions.

OK to push, but before  you do, make sure you fix up the date
in the ChangeLog entry both in the revision history (the one quoted
above) and the one in the ChangeLog file.

Also, would you mind reformatting the revision log to something that
uses a slightly longer line length? Something like 70 characters will
allow us to use fewer lines, and make the revision log a little
shorter (as well as more readable, IMO, but that may be just me).

And while looking at the contents of the ChangeLog one more time,
I think I missed on some suggestions. So below is what I suggest.
Please take a look to see what the changes are and whether you
agree with them. Basically, the ChangeLog says what you did in
concise terms, now what the different new elements are for.

| This patch introduces a linked list for dynamic attributes of a type.
| This is a pre-work for the Fortran dynamic array support. The Fortran
| dynamic array support will add more dynamic attributes to a type.
| As only a few types will have such dynamic attributes set, a linked
| list is more efficient in terms of memory consumption than adding
| multiple attributes to main_type.
| 
| 2015-03-16  Keven Boell  <keven.boell@intel.com>
| 
| 	* gdbtypes.c (resolve_dynamic_type_internal): Adapt
| 	data_location usage to linked list.
| 	(resolve_dynamic_type_internal): Adapt data_location to
| 	linked list.
| 	(get_dyn_prop, add_dyn_prop, copy_dynamic_prop_list): New function.
| 	(copy_type_recursive, copy_type): Add copy of linked list.
| 	* gdbtypes.h (enum dynamic_prop_node_kind): New enum.
| 	(struct dynamic_prop_list): New struct.
| 	* dwarf2read.c (set_die_type): Set data_location data.

Please make sure that this ChangeLog entry matches what you put
in gdb/ChangeLog.

Other than that, well done, and thank you for the patch. We can now
move to the next phase of your patch series!

-- 
Joel

Re: [PATCH] Introduce linked list for dynamic attributes

[Keven Boell]

On 16.03.2015 20:55, Joel Brobecker wrote:
>> From: Keven Boell <keven.boell@intel.com>
>> Date: Mon, 23 Feb 2015 15:45:06 +0100
>> Subject: [PATCH] Introduce linked list for dynamic attributes
>>
>> This patch introduces a linked list for dynamic
>> attributes of a type. This is a pre-work for
>> the Fortran dynamic array support. The Fortran
>> dynamic array support will add more dynamic
>> attributes to a type. As only a few types
>> will have such dynamic attributes set, a linked
>> list is more efficient in terms of memory
>> consumption than adding multiple attributes
>> to main_type.
>>
>> Transformed the data_location dynamic attribute
>> to use the linked list.
>>
>> 2015-02-23  Keven Boell  <keven.boell@intel.com>
>>
>> 	* gdbtypes.c (resolve_dynamic_type_internal):
>> 	Adapt data_location usage to linked list.
>> 	(resolve_dynamic_type_internal): Adapt
>> 	data_location usage to linked list.
>> 	(get_dyn_prop): New function.
>> 	(add_dyn_prop): New function.
>> 	(copy_dynamic_prop_list): New function.
>> 	(copy_type_recursive): Add copy of linked
>> 	list.
>> 	(copy_type): Add copy of linked list.
>> 	* gdbtypes.h (enum dynamic_prop_node_kind):
>> 	Kind of the dynamic attribute in linked list.
>> 	(struct dynamic_prop_list): Dynamic list
>> 	node.
>> 	* dwarf2read.c (set_die_type): Add data_location
>> 	data to linked list using helper functions.
> 
> OK to push, but before  you do, make sure you fix up the date
> in the ChangeLog entry both in the revision history (the one quoted
> above) and the one in the ChangeLog file.

I aligned the dates in the git log and the ChangeLog file. I hope this
is what I meant.

> 
> Also, would you mind reformatting the revision log to something that
> uses a slightly longer line length? Something like 70 characters will
> allow us to use fewer lines, and make the revision log a little
> shorter (as well as more readable, IMO, but that may be just me).

Done.

> 
> And while looking at the contents of the ChangeLog one more time,
> I think I missed on some suggestions. So below is what I suggest.
> Please take a look to see what the changes are and whether you
> agree with them. Basically, the ChangeLog says what you did in
> concise terms, now what the different new elements are for.
> 
> | This patch introduces a linked list for dynamic attributes of a type.
> | This is a pre-work for the Fortran dynamic array support. The Fortran
> | dynamic array support will add more dynamic attributes to a type.
> | As only a few types will have such dynamic attributes set, a linked
> | list is more efficient in terms of memory consumption than adding
> | multiple attributes to main_type.
> | 
> | 2015-03-16  Keven Boell  <keven.boell@intel.com>
> | 
> | 	* gdbtypes.c (resolve_dynamic_type_internal): Adapt
> | 	data_location usage to linked list.
> | 	(resolve_dynamic_type_internal): Adapt data_location to
> | 	linked list.
> | 	(get_dyn_prop, add_dyn_prop, copy_dynamic_prop_list): New function.
> | 	(copy_type_recursive, copy_type): Add copy of linked list.
> | 	* gdbtypes.h (enum dynamic_prop_node_kind): New enum.
> | 	(struct dynamic_prop_list): New struct.
> | 	* dwarf2read.c (set_die_type): Set data_location data.
> 
> Please make sure that this ChangeLog entry matches what you put
> in gdb/ChangeLog.

Aligned them as well.

> 
> Other than that, well done, and thank you for the patch. We can now
> move to the next phase of your patch series!
> 

Thanks! How do I get read/write access for pushing this patch?

Thanks again,
Keven


--
From: Keven Boell <keven.boell@intel.com>
Date: Mon, 23 Feb 2015 15:45:06 +0100
Subject: [PATCH] Introduce linked list for dynamic attributes

This patch introduces a linked list for dynamic attributes of a
type. This is a pre-work for the Fortran dynamic array support.
The Fortran dynamic array support will add more dynamic attributes
to a type. As only a few types will have such dynamic attributes
set, a linked list is more efficient in terms of memory consumption
than adding multiple attributes to main_type.

Transformed the data_location dynamic attribute to use the linked
list.

2015-02-23  Keven Boell  <keven.boell@intel.com>

	* gdbtypes.c (resolve_dynamic_type_internal): Adapt
	data_location usage to linked list.
	(resolve_dynamic_type_internal): Adapt data_location usage
	to linked list.
	(get_dyn_prop): New function.
	(add_dyn_prop): New function.
	(copy_dynamic_prop_list): New function.
	(copy_type_recursive): Add copy of linked list.
	(copy_type): Add copy of linked list.
	* gdbtypes.h (enum dynamic_prop_node_kind): Kind of the
	dynamic attribute in linked list.
	(struct dynamic_prop_list): Dynamic list node.
	* dwarf2read.c (set_die_type): Add data_location data to
	linked list using helper functions.
---
 gdb/ChangeLog    |   17 ++++++++++
 gdb/dwarf2read.c |    6 +---
 gdb/gdbtypes.c   |   95 ++++++++++++++++++++++++++++++++++++++++++------------
 gdb/gdbtypes.h   |   55 +++++++++++++++++++++++++++----
 4 files changed, 141 insertions(+), 32 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 10abae0..37ec688 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,20 @@
+2015-02-23  Keven Boell  <keven.boell@intel.com>
+
+	* gdbtypes.c (resolve_dynamic_type_internal): Adapt
+	data_location usage to linked list.
+	(resolve_dynamic_type_internal): Adapt data_location usage
+	to linked list.
+	(get_dyn_prop): New function.
+	(add_dyn_prop): New function.
+	(copy_dynamic_prop_list): New function.
+	(copy_type_recursive): Add copy of linked list.
+	(copy_type): Add copy of linked list.
+	* gdbtypes.h (enum dynamic_prop_node_kind): Kind of the
+	dynamic attribute in linked list.
+	(struct dynamic_prop_list): Dynamic list node.
+	* dwarf2read.c (set_die_type): Add data_location data to
+	linked list using helper functions.
+
 2015-03-17  Gary Benson <gbenson@redhat.com>
 	    Luke Allardyce <lukeallardyce@gmail.com>
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index c185d51..898706f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -22085,11 +22085,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   /* Read DW_AT_data_location and set in type.  */
   attr = dwarf2_attr (die, DW_AT_data_location, cu);
   if (attr_to_dynamic_prop (attr, die, cu, &prop))
-    {
-      TYPE_DATA_LOCATION (type)
-        = obstack_alloc (&objfile->objfile_obstack, sizeof (prop));
-      *TYPE_DATA_LOCATION (type) = prop;
-    }
+    add_dyn_prop (DYN_ATTR_DATA_LOCATION, prop, type, objfile);
 
   if (dwarf2_per_objfile->die_type_hash == NULL)
     {
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 4cbbe95..19579af 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2025,7 +2025,7 @@ resolve_dynamic_type_internal (struct type *type,
 {
   struct type *real_type = check_typedef (type);
   struct type *resolved_type = type;
-  const struct dynamic_prop *prop;
+  struct dynamic_prop *prop;
   CORE_ADDR value;
 
   if (!is_dynamic_type_internal (real_type, top_level))
@@ -2080,13 +2080,11 @@ resolve_dynamic_type_internal (struct type *type,
 
   /* Resolve data_location attribute.  */
   prop = TYPE_DATA_LOCATION (resolved_type);
-  if (dwarf2_evaluate_property (prop, addr_stack, &value))
+  if (prop != NULL && dwarf2_evaluate_property (prop, addr_stack, &value))
     {
-      TYPE_DATA_LOCATION_ADDR (resolved_type) = value;
-      TYPE_DATA_LOCATION_KIND (resolved_type) = PROP_CONST;
+      TYPE_DYN_PROP_ADDR (prop) = value;
+      TYPE_DYN_PROP_KIND (prop) = PROP_CONST;
     }
-  else
-    TYPE_DATA_LOCATION (resolved_type) = NULL;
 
   return resolved_type;
 }
@@ -2101,6 +2099,42 @@ resolve_dynamic_type (struct type *type, CORE_ADDR addr)
   return resolve_dynamic_type_internal (type, &pinfo, 1);
 }
 
+/* See gdbtypes.h  */
+
+struct dynamic_prop *
+get_dyn_prop (enum dynamic_prop_node_kind prop_kind, const struct type *type)
+{
+  struct dynamic_prop_list *node = TYPE_DYN_PROP_LIST (type);
+
+  while (node != NULL)
+    {
+      if (node->prop_kind == prop_kind)
+        return node->prop;
+      node = node->next;
+    }
+  return NULL;
+}
+
+/* See gdbtypes.h  */
+
+void
+add_dyn_prop (enum dynamic_prop_node_kind prop_kind, struct dynamic_prop prop,
+              struct type *type, struct objfile *objfile)
+{
+  struct dynamic_prop_list *temp;
+
+  gdb_assert (TYPE_OBJFILE_OWNED (type));
+
+  temp = obstack_alloc (&objfile->objfile_obstack,
+			sizeof (struct dynamic_prop_list));
+  temp->prop_kind = prop_kind;
+  temp->prop = obstack_copy (&objfile->objfile_obstack, &prop, sizeof (prop));
+  temp->next = TYPE_DYN_PROP_LIST (type);
+
+  TYPE_DYN_PROP_LIST (type) = temp;
+}
+
+
 /* Find the real type of TYPE.  This function returns the real type,
    after removing all layers of typedefs, and completing opaque or stub
    types.  Completion changes the TYPE argument, but stripping of
@@ -4230,6 +4264,31 @@ create_copied_types_hash (struct objfile *objfile)
 			       dummy_obstack_deallocate);
 }
 
+/* Recursively copy (deep copy) a dynamic attribute list of a type.  */
+
+static struct dynamic_prop_list *
+copy_dynamic_prop_list (struct obstack *objfile_obstack,
+			struct dynamic_prop_list *list)
+{
+  struct dynamic_prop_list *copy = list;
+  struct dynamic_prop_list **node_ptr = &copy;
+
+  while (*node_ptr != NULL)
+    {
+      struct dynamic_prop_list *node_copy;
+
+      node_copy = obstack_copy (objfile_obstack, *node_ptr,
+				sizeof (struct dynamic_prop_list));
+      node_copy->prop = obstack_copy (objfile_obstack, (*node_ptr)->prop,
+				      sizeof (struct dynamic_prop));
+      *node_ptr = node_copy;
+
+      node_ptr = &node_copy->next;
+    }
+
+  return copy;
+}
+
 /* Recursively copy (deep copy) TYPE, if it is associated with
    OBJFILE.  Return a new type allocated using malloc, a saved type if
    we have already visited TYPE (using COPIED_TYPES), or TYPE if it is
@@ -4333,14 +4392,11 @@ copy_type_recursive (struct objfile *objfile,
       *TYPE_RANGE_DATA (new_type) = *TYPE_RANGE_DATA (type);
     }
 
-  /* Copy the data location information.  */
-  if (TYPE_DATA_LOCATION (type) != NULL)
-    {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
-    }
+  if (TYPE_DYN_PROP_LIST (type) != NULL)
+    TYPE_DYN_PROP_LIST (new_type)
+      = copy_dynamic_prop_list (&objfile->objfile_obstack,
+				TYPE_DYN_PROP_LIST (type));
+
 
   /* Copy pointers to other types.  */
   if (TYPE_TARGET_TYPE (type))
@@ -4404,13 +4460,10 @@ copy_type (const struct type *type)
   TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
   memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
 	  sizeof (struct main_type));
-  if (TYPE_DATA_LOCATION (type) != NULL)
-    {
-      TYPE_DATA_LOCATION (new_type)
-	= TYPE_ALLOC (new_type, sizeof (struct dynamic_prop));
-      memcpy (TYPE_DATA_LOCATION (new_type), TYPE_DATA_LOCATION (type),
-	      sizeof (struct dynamic_prop));
-    }
+  if (TYPE_DYN_PROP_LIST (type) != NULL)
+    TYPE_DYN_PROP_LIST (new_type)
+      = copy_dynamic_prop_list (&TYPE_OBJFILE (type) -> objfile_obstack,
+				TYPE_DYN_PROP_LIST (type));
 
   return new_type;
 }
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 2c5ccf4..79d72df 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -434,6 +434,26 @@ struct dynamic_prop
   union dynamic_prop_data data;
 };
 
+/* * Define a type's dynamic property node kind.  */
+enum dynamic_prop_node_kind
+{
+  /* A property providing a type's data location.
+     Evaluating this field yields to the location of an object's data.  */
+  DYN_ATTR_DATA_LOCATION,
+};
+
+/* * List for dynamic type attributes.  */
+struct dynamic_prop_list
+{
+  /* The kind of dynamic prop in this node.  */
+  enum dynamic_prop_node_kind prop_kind;
+
+  /* The dynamic property itself.  */
+  struct dynamic_prop *prop;
+
+  /* A pointer to the next dynamic property.  */
+  struct dynamic_prop_list *next;
+};
 
 /* * Determine which field of the union main_type.fields[x].loc is
    used.  */
@@ -719,10 +739,8 @@ struct main_type
 
   union type_specific type_specific;
 
-  /* * Contains a location description value for the current type. Evaluating
-     this field yields to the location of the data for an object.  */
-
-  struct dynamic_prop *data_location;
+  /* * Contains all dynamic type properties.  */
+  struct dynamic_prop_list *dyn_prop_list;
 };
 
 /* * A ``struct type'' describes a particular instance of a type, with
@@ -1238,9 +1256,9 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_LOW_BOUND_KIND(range_type) \
   TYPE_RANGE_DATA(range_type)->low.kind
 
-/* Attribute accessors for the type data location.  */
+/* Property accessors for the type data location.  */
 #define TYPE_DATA_LOCATION(thistype) \
-  TYPE_MAIN_TYPE(thistype)->data_location
+  get_dyn_prop (DYN_ATTR_DATA_LOCATION, thistype)
 #define TYPE_DATA_LOCATION_BATON(thistype) \
   TYPE_DATA_LOCATION (thistype)->data.baton
 #define TYPE_DATA_LOCATION_ADDR(thistype) \
@@ -1248,6 +1266,17 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_DATA_LOCATION_KIND(thistype) \
   TYPE_DATA_LOCATION (thistype)->kind
 
+/* Attribute accessors for dynamic properties.  */
+#define TYPE_DYN_PROP_LIST(thistype) \
+  TYPE_MAIN_TYPE(thistype)->dyn_prop_list
+#define TYPE_DYN_PROP_BATON(dynprop) \
+  dynprop->data.baton
+#define TYPE_DYN_PROP_ADDR(dynprop) \
+  dynprop->data.const_val
+#define TYPE_DYN_PROP_KIND(dynprop) \
+  dynprop->kind
+
+
 /* Moto-specific stuff for FORTRAN arrays.  */
 
 #define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
@@ -1767,6 +1796,20 @@ extern struct type *resolve_dynamic_type (struct type *type, CORE_ADDR addr);
 /* * Predicate if the type has dynamic values, which are not resolved yet.  */
 extern int is_dynamic_type (struct type *type);
 
+/* * Return the dynamic property of the requested KIND from TYPE's
+   list of dynamic properties.  */
+extern struct dynamic_prop *get_dyn_prop
+  (enum dynamic_prop_node_kind kind, const struct type *type);
+
+/* * Given a dynamic property PROP of a given KIND, add this dynamic
+   property to the given TYPE.
+
+   This function assumes that TYPE is objfile-owned, and that OBJFILE
+   is the TYPE's objfile.  */
+extern void add_dyn_prop
+  (enum dynamic_prop_node_kind kind, struct dynamic_prop prop,
+   struct type *type, struct objfile *objfile);
+
 extern struct type *check_typedef (struct type *);
 
 #define CHECK_TYPEDEF(TYPE)			\
-- 
1.7.9.5

Re: [PATCH] Introduce linked list for dynamic attributes

[Joel Brobecker]

> 2015-02-23  Keven Boell  <keven.boell@intel.com>
> 
> 	* gdbtypes.c (resolve_dynamic_type_internal): Adapt
> 	data_location usage to linked list.
> 	(resolve_dynamic_type_internal): Adapt data_location usage
> 	to linked list.
> 	(get_dyn_prop): New function.
> 	(add_dyn_prop): New function.
> 	(copy_dynamic_prop_list): New function.
> 	(copy_type_recursive): Add copy of linked list.
> 	(copy_type): Add copy of linked list.
> 	* gdbtypes.h (enum dynamic_prop_node_kind): Kind of the
> 	dynamic attribute in linked list.
> 	(struct dynamic_prop_list): Dynamic list node.
> 	* dwarf2read.c (set_die_type): Add data_location data to
> 	linked list using helper functions.

I sent Keven the instructions on how to get a sourceware account
a while ago now. In the meantime, I've pushed this patch to master
(after having re-tested it on x86_64-linux, just in case).

-- 
Joel