This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/3] fort_dyn_array: Enable dynamic member types inside a structure.
- From: "Heckel, Bernhard" <bernhard dot heckel at intel dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>
- Cc: brobecker at adacore dot com, gdb-patches at sourceware dot org
- Date: Tue, 5 Apr 2016 09:31:08 +0200
- Subject: Re: [PATCH 1/3] fort_dyn_array: Enable dynamic member types inside a structure.
- Authentication-results: sourceware.org; auth=none
- References: <1458204189-13267-1-git-send-email-bernhard dot heckel at intel dot com> <1458204189-13267-2-git-send-email-bernhard dot heckel at intel dot com> <86h9fhpkkz dot fsf at gmail dot com>
On 04/04/2016 15:30, Yao Qi wrote:
Bernhard Heckel <bernhard.heckel@intel.com> writes:
fort_dyn_array: Enable dynamic member types inside a structure.
2016-02-24 Bernhard Heckel <bernhard.heckel@intel.com>
2015-03-20 Keven Boell <keven.boell@intel.com>
Before:
(gdb) print threev%ivla(1)
Cannot access memory at address 0x3
(gdb) print threev%ivla(5)
no such vector element
After:
(gdb) print threev%ivla(1)
$9 = 1
(gdb) print threev%ivla(5)
$10 = 42
It would be helpful to describe your change, at least people don't know
much about fortran, like me, can understand the change.
gdb/Changelog:
* gdbtypes.c (remove_dyn_prop): New.
* gdbtypes.h: Forward declaration of new function.
* value.c (value_address): Return dynamic resolved location of a value.
(set_value_component_location): Adjust the value address
for single value prints.
(value_primitive_field): Support value types with a dynamic location.
(set_internalvar): Remove dynamic location property of
internal variables.
gdb/testsuite/Changelog:
* gdb.fortran/vla-type.f90: New file.
* gdb.fortran/vla-type.exp: New file.
---
gdb/gdbtypes.c | 43 +++++++++++++--
gdb/gdbtypes.h | 3 ++
gdb/testsuite/gdb.fortran/vla-type.exp | 98 ++++++++++++++++++++++++++++++++++
gdb/testsuite/gdb.fortran/vla-type.f90 | 88 ++++++++++++++++++++++++++++++
gdb/value.c | 35 ++++++++++--
5 files changed, 261 insertions(+), 6 deletions(-)
create mode 100755 gdb/testsuite/gdb.fortran/vla-type.exp
create mode 100755 gdb/testsuite/gdb.fortran/vla-type.f90
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index f129b0e..066fe88 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2064,7 +2064,8 @@ resolve_dynamic_struct (struct type *type,
First of all, the change in resolve_dynamic_struct isn't mentioned in
the CL entry.
pinfo.type = check_typedef (TYPE_FIELD_TYPE (type, i));
pinfo.valaddr = addr_stack->valaddr;
- pinfo.addr = addr_stack->addr;
+ pinfo.addr = addr_stack->addr
+ + (TYPE_FIELD_BITPOS (resolved_type, i) / TARGET_CHAR_BIT);
AFAICS, addr_stack->valaddr and addr_stack->addr can't be used
together. Either of them is used, so why do we set both of them?
Actually, when resolving dynamic types I don't use the valaddr. From
what I understood
I could even return if valaddr is not Null as the TYPE should already be
resolved at that time.
But I was unsure about it and kept the code.
pinfo.next = addr_stack;
TYPE_FIELD_TYPE (resolved_type, i)
@@ -2090,8 +2091,13 @@ resolve_dynamic_struct (struct type *type,
resolved_type_bit_length = new_bit_length;
}
- TYPE_LENGTH (resolved_type)
- = (resolved_type_bit_length + TARGET_CHAR_BIT - 1) / TARGET_CHAR_BIT;
+ /* Type length won't change for fortran. Keep what we got from DWARF.
Two spaces after ".". Multiple instances of this problem.
+ Dynamic fields might change over time but not the struct definition.
+ If we would adapt it we run into problems when
+ calculating the element offset for arrays of structs. */
What is the problem we run into?
Imagine we have a dynamic allocatable array as a member of a structure.
The size of the dynamic array can vary over time.
When we have resolved this allocatable array we don't want to add it's
length to the structure length it belongs to.
Dynamic members are not embedded in the structure itself. Only the
description of the dynamic type is embedded
and the size of the description (address, bounds,...) won't change.
If we add the length of the resolved type we would run in problems when
we resolve arrays of structs
with dynamic types (usually allocatable types) as we would use the wrong
location (offset) for the boundary, size, location,.. attributes -> see
patch 2/3.
+ if (current_language->la_language != language_fortran)
+ TYPE_LENGTH (resolved_type)
+ = (resolved_type_bit_length + TARGET_CHAR_BIT - 1) / TARGET_CHAR_BIT;
/* The Ada language uses this field as a cache for static fixed types: reset
it as RESOLVED_TYPE must have its own static fixed type. */
@@ -2224,6 +2230,37 @@ add_dyn_prop (enum dynamic_prop_node_kind prop_kind, struct dynamic_prop prop,
TYPE_DYN_PROP_LIST (type) = temp;
}
+/* Remove dynamic property from a type in case it exist. */
+
The comment can be
/* Remove dynamic property from TYPE in case it exists. */
+void
+remove_dyn_prop (enum dynamic_prop_node_kind prop_kind,
+ struct type *type)
+{
+ struct dynamic_prop_list *prev_node, *curr_node;
+
+ curr_node = TYPE_DYN_PROP_LIST (type);
+ prev_node = NULL;
+
+ while (NULL != curr_node)
+ {
+ if (curr_node->prop_kind == prop_kind)
+ {
+ /* Upadate the linked list but don't free anything.
+ The property was allocated on objstack and it is not known
+ if we are on top of it. Nevertheless, everything is released
+ when the complete objstack is freed. */
+ if (NULL == prev_node)
+ TYPE_DYN_PROP_LIST (type) = curr_node->next;
+ else
+ prev_node->next = curr_node->next;
+
+ return;
+ }
+
+ prev_node = curr_node;
+ curr_node = curr_node->next;
+ }
+}
CORE_ADDR
@@ -1846,6 +1851,8 @@ void
set_value_component_location (struct value *component,
const struct value *whole)
{
+ struct type *type;
+
gdb_assert (whole->lval != lval_xcallable);
if (whole->lval == lval_internalvar)
@@ -1861,9 +1868,14 @@ set_value_component_location (struct value *component,
if (funcs->copy_closure)
component->location.computed.closure = funcs->copy_closure (whole);
}
+
+ /* If type has a dynamic resolved location property update it's value address. */
+ type = value_type (whole);
+ if (TYPE_DATA_LOCATION (type)
TYPE_DATA_LOCATION (type) != NULL
+ && TYPE_DATA_LOCATION_KIND (type) == PROP_CONST)
+ set_value_address (component, TYPE_DATA_LOCATION_ADDR (type));
}
-
/* Access to the value history. */
/* Record a new value in the value history.
@@ -2416,6 +2428,12 @@ set_internalvar (struct internalvar *var, struct value *val)
call error () until new_data is installed into the var->u to avoid
leaking memory. */
release_value (new_data.value);
+
+ /* Internal variables which are created from values with a dynamic location
+ don't need the location property of the origin anymore.
+ Remove the location property in case it exist. */
+ remove_dyn_prop(DYN_PROP_DATA_LOCATION, value_type(new_data.value));
Space is needed before "(". What is wrong if we don't do so? Do you
have a test case for this?
An internal variable has it's own valaddr to where the copy is located.
If we keep the dynamic location from the origin then this dyn. location
will be used to set
the value component location.
As the internal variable is a copy of an value at a certain point in
time I prefer to get rid off the
dynamic location attribute then to do some "if else if else" constructs
when setting the component location.
There are tests:
gdb.fortran/vla-value.exp: print $myvar set to vla1
gdb.fortran/vla-value.exp: print $myvar(3,6,9)
+
break;
}
@@ -3157,6 +3175,17 @@ value_primitive_field (struct value *arg1, int offset,
v->offset = value_offset (arg1);
v->embedded_offset = offset + value_embedded_offset (arg1) + boffset;
}
+ else if (TYPE_DATA_LOCATION (type))
+ {
+ /* Field is a dynamic data member. */
+
+ gdb_assert (0 == offset);
+ /* We expect an already resolved data location. */
+ gdb_assert (PROP_CONST == TYPE_DATA_LOCATION_KIND (type));
+ /* For dynamic data types defer memory allocation
+ until we actual access the value. */
+ v = allocate_value_lazy (type);
Do we need to call set_value_lazy (v, 1)?
+ }
else
{
/* Plain old data member */
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928