From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 34024 invoked by alias); 4 Apr 2016 13:31:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 34008 invoked by uid 89); 4 Apr 2016 13:31:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-pf0-f182.google.com Received: from mail-pf0-f182.google.com (HELO mail-pf0-f182.google.com) (209.85.192.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 04 Apr 2016 13:31:05 +0000 Received: by mail-pf0-f182.google.com with SMTP id n1so39820872pfn.2 for ; Mon, 04 Apr 2016 06:31:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=QwTWJOQus8GFTZu+/zYQFpU3ePLu1LNK3phSiL7kTkE=; b=Qo/7GOqGTa4mjyT+BMAuh3heiM3bNz06yukLA7bGUPKwb0KQl4iqQZ4MBzvzEpZw+F iXZApkEG9JrVvhJQzqfKMJmcN298rtGU7eiNlExS5tJyo7wUKGEPYmibmCLakJI2scvO qXB6idUyeS9j8GqlNMopCO71rRs5Kroc0O1zAp+p37QbJOqTgGmBX5ziMCz7MvoEgGO4 5uzmmwD9AaCg7MeG2jyph29IO9XEaHaa7Z2VOIicHidoRb4YlZW0qjtnMacE02YYX+hU f9/5rnljey4hwPhaGIExWGZhAzValZgSs+NqSllUUtXRlarkI0kH+4tfkyhFkcgrsI1K WInA== X-Gm-Message-State: AD7BkJLdQPn7NyCIKGTiG2BotzO5WeLdLtEy159N5O6PYEPcXxL1pmnF/E3WiwgL/WfrNQ== X-Received: by 10.98.15.135 with SMTP id 7mr51686465pfp.142.1459776663391; Mon, 04 Apr 2016 06:31:03 -0700 (PDT) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id ml5sm540967pab.2.2016.04.04.06.30.59 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 04 Apr 2016 06:31:02 -0700 (PDT) From: Yao Qi To: Bernhard Heckel Cc: brobecker@adacore.com, gdb-patches@sourceware.org Subject: Re: [PATCH 1/3] fort_dyn_array: Enable dynamic member types inside a structure. References: <1458204189-13267-1-git-send-email-bernhard.heckel@intel.com> <1458204189-13267-2-git-send-email-bernhard.heckel@intel.com> Date: Mon, 04 Apr 2016 13:31:00 -0000 In-Reply-To: <1458204189-13267-2-git-send-email-bernhard.heckel@intel.com> (Bernhard Heckel's message of "Thu, 17 Mar 2016 09:43:07 +0100") Message-ID: <86h9fhpkkz.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-04/txt/msg00036.txt.bz2 Bernhard Heckel writes: > fort_dyn_array: Enable dynamic member types inside a structure. > > 2016-02-24 Bernhard Heckel > 2015-03-20 Keven Boell > > 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 =3D 1 > (gdb) print threev%ivla(5) > $10 =3D 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, >=20=20 First of all, the change in resolve_dynamic_struct isn't mentioned in the CL entry. > pinfo.type =3D check_typedef (TYPE_FIELD_TYPE (type, i)); > pinfo.valaddr =3D addr_stack->valaddr; > - pinfo.addr =3D addr_stack->addr; > + pinfo.addr =3D 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? > pinfo.next =3D addr_stack; >=20=20 > TYPE_FIELD_TYPE (resolved_type, i) > @@ -2090,8 +2091,13 @@ resolve_dynamic_struct (struct type *type, > resolved_type_bit_length =3D new_bit_length; > } >=20=20 > - TYPE_LENGTH (resolved_type) > - =3D (resolved_type_bit_length + TARGET_CHAR_BIT - 1) / TARGET_CHAR_B= IT; > + /* 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? > + if (current_language->la_language !=3D language_fortran) > + TYPE_LENGTH (resolved_type) > + =3D (resolved_type_bit_length + TARGET_CHAR_BIT - 1) / TARGET_CHAR= _BIT; >=20=20 > /* 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_kin= d, struct dynamic_prop prop, > TYPE_DYN_PROP_LIST (type) =3D temp; > } >=20=20 > +/* 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 =3D TYPE_DYN_PROP_LIST (type); > + prev_node =3D NULL; > + > + while (NULL !=3D curr_node) > + { > + if (curr_node->prop_kind =3D=3D 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 =3D=3D prev_node) > + TYPE_DYN_PROP_LIST (type) =3D curr_node->next; > + else > + prev_node->next =3D curr_node->next; > + > + return; > + } > + > + prev_node =3D curr_node; > + curr_node =3D 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 !=3D lval_xcallable); >=20=20 > if (whole->lval =3D=3D lval_internalvar) > @@ -1861,9 +1868,14 @@ set_value_component_location (struct value *compon= ent, > if (funcs->copy_closure) > component->location.computed.closure =3D funcs->copy_closure (wh= ole); > } > + > + /* If type has a dynamic resolved location property update it's value = address. */ > + type =3D value_type (whole); > + if (TYPE_DATA_LOCATION (type) TYPE_DATA_LOCATION (type) !=3D NULL > + && TYPE_DATA_LOCATION_KIND (type) =3D=3D PROP_CONST) > + set_value_address (component, TYPE_DATA_LOCATION_ADDR (type)); > } >=20=20 > -=0C > /* Access to the value history. */ >=20=20 > /* Record a new value in the value history. > @@ -2416,6 +2428,12 @@ set_internalvar (struct internalvar *var, struct v= alue *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? > + > break; > } >=20=20 > @@ -3157,6 +3175,17 @@ value_primitive_field (struct value *arg1, int off= set, > v->offset =3D value_offset (arg1); > v->embedded_offset =3D offset + value_embedded_offset (arg1) + bof= fset; > } > + else if (TYPE_DATA_LOCATION (type)) > + { > + /* Field is a dynamic data member. */ > + > + gdb_assert (0 =3D=3D offset); > + /* We expect an already resolved data location. */ > + gdb_assert (PROP_CONST =3D=3D TYPE_DATA_LOCATION_KIND (type)); > + /* For dynamic data types defer memory allocation > + until we actual access the value. */ > + v =3D allocate_value_lazy (type); Do we need to call set_value_lazy (v, 1)? > + } > else > { > /* Plain old data member */ --=20 Yao (=E9=BD=90=E5=B0=A7)