From patchwork Thu Mar 5 18:13:47 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 5481 Received: (qmail 20197 invoked by alias); 5 Mar 2015 18:13:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 20145 invoked by uid 89); 5 Mar 2015 18:13:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 05 Mar 2015 18:13:53 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id B0CA228101; Thu, 5 Mar 2015 13:13:51 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id MRyCl3DGqIa2; Thu, 5 Mar 2015 13:13:51 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 34F7928100; Thu, 5 Mar 2015 13:13:51 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id E5EB940EAD; Thu, 5 Mar 2015 10:13:47 -0800 (PST) Date: Thu, 5 Mar 2015 10:13:47 -0800 From: Joel Brobecker To: Keven Boell Cc: Keven Boell , gdb-patches@sourceware.org Subject: Re: [PATCH] Introduce linked list for dynamic attributes Message-ID: <20150305181347.GB4604@adacore.com> References: <1425281876-20302-1-git-send-email-keven.boell@intel.com> <20150302194959.GD4449@adacore.com> <54F5BA7A.2090308@linux.intel.com> <20150303155457.GA3243@adacore.com> <54F6FA0D.4000103@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <54F6FA0D.4000103@linux.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) > 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. From 8b0030afcfc11d35e040cf999d5bbe4705cf2b7c Mon Sep 17 00:00:00 2001 From: Joel Brobecker 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 = © + + 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