From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id J036K37WpV9KHwAAWB0awg (envelope-from ) for ; Fri, 06 Nov 2020 18:04:30 -0500 Received: by simark.ca (Postfix, from userid 112) id AB76B1F08B; Fri, 6 Nov 2020 18:04:30 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 6CD601E58D for ; Fri, 6 Nov 2020 18:04:29 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2E9AC3A6A03E; Fri, 6 Nov 2020 23:04:29 +0000 (GMT) Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by sourceware.org (Postfix) with ESMTPS id 07752386EC23 for ; Fri, 6 Nov 2020 23:04:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 07752386EC23 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x32f.google.com with SMTP id h2so2962882wmm.0 for ; Fri, 06 Nov 2020 15:04:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=HjbF8ruITL9iXhFxMGNgu1htKnIXRWsSoJaqH9JpAoU=; b=KLPM5zAGO+nApiSjZU3iL5Cph3C3GZAFCFKUOzwo53xxBaTOV7Ew5BX+CJaYFTuIpb HvIxB7CJsFKM5F0YyJYWljL+ZF8LCR8A6pk5VeBTnNJlBzIMu0wc8GQdpmGf5mxnGSUI OuwUH4M/2RDp5Ken1cPOe1016vsxDWkLJq1XcU5dVR/UTM+v+ZJyVMhc7/jZLps4b1ou P/V7rqtFQn4GrWXwxONevioVPZQgHDI0Lb+AvtHaVcAALi4oHz208+pbRKrNSd6Dm5+T b8Mz5IeB9RCPcyERSMXnuZxOk8OecsUSrROVQZmCdladJyN9RqE3PU9kvg1D16SA8MlX Oz2Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=HjbF8ruITL9iXhFxMGNgu1htKnIXRWsSoJaqH9JpAoU=; b=C+cHrl0y+vGdhP/ZTtweFZ7P1pwzJADkzB7+rkPehG50TEg/FqN96V214q3c/bU5sH nNJX0xeGgZnl14Q5kNRGxA5x9RU7uEj48nX80GARulio+9j8e0saIAfs+xqRCQZ4LOQe tq6MDO69prsSOZhH7Nlad/DeuhhlAL22hi5ZqPWMsfc6Yk3cSphKXEJKefINPFTp5U2k FnWlCiM7WxwUbDPNcVd6proAvjMyNd/hTLHW2qhf8HV4xV5AA6Kf2l8+8oZ9/pGn/dz+ bw+5PsvohlPDbPINYdE5+wEaDb9zKSFrg/WHirRonnW91M+C/A9ZoHFjh2k9OOjib9OY LIlA== X-Gm-Message-State: AOAM5325d/XI3ODUZ08fk7ExMYQnZL53Ef/7npA2BDXCsQ9D4C8V3ywD XUCFJEj+p5/pd5y3kYXHv5vRiVjw1YeSBQ== X-Google-Smtp-Source: ABdhPJySx+GWvVc3kJtHuWk3Knke31HxJGnSQPiaL8IlYv5BLNAJmgF789ewS6ID3Kv4sTh5hvf6Og== X-Received: by 2002:a7b:c2ef:: with SMTP id e15mr1960948wmk.180.1604703863805; Fri, 06 Nov 2020 15:04:23 -0800 (PST) Received: from localhost (host109-154-72-197.range109-154.btcentralplus.com. [109.154.72.197]) by smtp.gmail.com with ESMTPSA id x7sm4062695wrt.78.2020.11.06.15.04.23 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Nov 2020 15:04:23 -0800 (PST) Date: Fri, 6 Nov 2020 23:04:22 +0000 From: Andrew Burgess To: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb: user variables with components of dynamic type Message-ID: <20201106230422.GK2729@embecosm.com> References: <20201022153238.1947197-1-andrew.burgess@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201022153238.1947197-1-andrew.burgess@embecosm.com> X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 23:03:43 up 12 days, 14:06, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" Ping! Any thoughts? Thanks, Andrew * Andrew Burgess [2020-10-22 16:32:38 +0100]: > Consider this Fortran type: > > type :: some_type > integer, allocatable :: array_one (:,:) > integer :: a_field > integer, allocatable :: array_two (:,:) > end type some_type > > And a variable declared: > > type(some_type) :: some_var > > Now within GDB we try this: > > (gdb) set $a = some_var > (gdb) p $a > $1 = ( array_one = > ../../src/gdb/value.c:3968: internal-error: Unexpected lazy value type. > > Normally, when an internalvar ($a in this case) is created, it is > non-lazy, the value is immediately copied out of the inferior into > GDB's memory. > > When printing the internalvar ($a) GDB will extract each field in > turn, so in this case `array_one`. As the original internalvar is > non-lazy then the extracted field will also be non-lazy, with its > contents immediately copied from the parent internalvar. > > However, when the field has a dynamic type this is not the case, > value_primitive_field we see that any field with dynamic type is > always created lazy. Further, the content of this field will usually > not have been captured in the contents buffer of the original value, a > field with dynamic location is effectively a pointer value contained > within the parent value, with rules in the DWARF for how to > dereference the pointer. > > So, we end up with a lazy lval_internalvar_component representing a > field within an lval_internalvar. This eventually ends up in > value_fetch_lazy, which currently does not support > lval_internalvar_component, and we see the error above. > > My original plan for how to handle this involved extending > value_fetch_lazy to handle lval_internalvar_component. However, when > I did this I ran into another error: > > (gdb) set $a = some_var > (gdb) p $a > $1 = ( array_one = ((1, 1) (1, 1) (1, 1)), a_field = 5, array_two = ((0, 0, 0) (0, 0, 0)) ) > (gdb) p $a%array_one > $2 = ((1, 1) (1, 1) (1, 1)) > (gdb) p $a%array_one(1,1) > ../../src/gdb/value.c:1547: internal-error: void set_value_address(value*, CORE_ADDR): Assertion `value->lval == lval_memory' failed. > > The problem now is some patch up code inside > set_value_component_location, where we attempt to set the address for > a component, if the original parent value has a dynamic location. GDB > does not expect to ever set the address on anything other than an > lval_memory value (which seems reasonable). > > In order to resolve this issue I initially thought about how an > internalvar should "capture" the value of a program variable at the > moment the var is created. In an ideal world (I think) GDB would be > able to do this even for values with dynamic type. So in our above > example doing `set $a = some_var` would capture the content of > 'some_var', but also the content of 'array_one', and also 'array_two', > even these content regions are not contained within the region of > 'some_var'. > > Supporting this would require GDB values to be able to carry around > multiple non-contiguous regions of memory at content in some way, > which sounds like a pretty huge change to a core part of GDB. > > So, I wondered if there was some other solution that wouldn't require > such a huge change. > > What if values with a dynamic location were though of like points with > automatic dereferencing? Given this C structure: > > struct foo_t { > int *val; > } > > struct foo_t my_foo; > > Then in GDB: > > (gdb) $a = my_foo > > We would expect GDB to capture the pointer value in '$a', but not the > value pointed at by the pointer. So maybe it's not that unreasonable > to think that given a dynamically typed field GDB will capture the > address of the content, but not the actual content itself. > > That's what this patch does. > > The approach is to catch this case in set_value_component_location, > when we create a component location (of an lval_internalvar) that has > a dynamic data location, the lval_internalvar_component is changed > into an lval_memory. After this both of the above issues are > resolved. In the first case, the lval_memory is still lazy, but > value_fetch_lazy knows how to handle that. In the second case, when > we access the element of the array we are now accessing an element of > an lval_memory, not an lval_internalvar_component, and calling > set_value_address on an lval_memory is fine. > > gdb/ChangeLog: > > * value.c (set_value_component_location): Adjust the VALUE_LVAL > for internalvar components that have a dynamic location. > > gdb/testsuite/ChangeLog: > > * gdb.fortran/intvar-dynamic-types.exp: New file. > * gdb.fortran/intvar-dynamic-types.f90: New file. > --- > gdb/ChangeLog | 5 + > gdb/testsuite/ChangeLog | 5 + > .../gdb.fortran/intvar-dynamic-types.exp | 97 +++++++++++++++++++ > .../gdb.fortran/intvar-dynamic-types.f90 | 32 ++++++ > gdb/value.c | 29 +++++- > 5 files changed, 166 insertions(+), 2 deletions(-) > create mode 100644 gdb/testsuite/gdb.fortran/intvar-dynamic-types.exp > create mode 100644 gdb/testsuite/gdb.fortran/intvar-dynamic-types.f90 > > diff --git a/gdb/testsuite/gdb.fortran/intvar-dynamic-types.exp b/gdb/testsuite/gdb.fortran/intvar-dynamic-types.exp > new file mode 100644 > index 00000000000..9cf5cee02ff > --- /dev/null > +++ b/gdb/testsuite/gdb.fortran/intvar-dynamic-types.exp > @@ -0,0 +1,97 @@ > +# Copyright 2020 Free Software Foundation, Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# Places a value with components that have dynamic type into a GDB > +# user variable, and then prints the user variable. > + > +standard_testfile ".f90" > +load_lib "fortran.exp" > + > +if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} \ > + {debug f90 quiet}] } { > + return -1 > +} > + > +if ![fortran_runto_main] { > + untested "could not run to main" > + return -1 > +} > + > +gdb_breakpoint [gdb_get_line_number "Break here"] > +gdb_continue_to_breakpoint "Break here" > + > +gdb_test_no_output "set \$a=some_var" "set \$a internal variable" > + > +foreach var { "\$a" "some_var" } { > + with_test_prefix "print $var" { > + gdb_test "print $var" \ > + " = \\( array_one = \\(\\(1, 1\\) \\(1, 1\\) \\(1, 1\\)\\), a_field = 5, array_two = \\(\\(2, 2, 2\\) \\(2, 2, 2\\)\\) \\)" \ > + "print full contents" > + > + gdb_test "print $var%array_one" \ > + " = \\(\\(1, 1\\) \\(1, 1\\) \\(1, 1\\)\\)" \ > + "print array_one field" > + > + gdb_test "print $var%a_field" \ > + " = 5" \ > + "print a_field field" > + > + gdb_test "print $var%array_two" \ > + " = \\(\\(2, 2, 2\\) \\(2, 2, 2\\)\\)" \ > + "print array_two field" > + } > +} > + > +# Create new user variables for the fields of some_var, and show that > +# modifying these variables does not change the original value from > +# the program. > +gdb_test_no_output "set \$b = some_var%array_one" > +gdb_test_no_output "set \$c = some_var%array_two" > +gdb_test "print \$b" \ > + " = \\(\\(1, 1\\) \\(1, 1\\) \\(1, 1\\)\\)" > +gdb_test "print \$c" \ > + " = \\(\\(2, 2, 2\\) \\(2, 2, 2\\)\\)" > +gdb_test_no_output "set \$b(2,2) = 3" > +gdb_test_no_output "set \$c(3,1) = 4" > +gdb_test "print \$b" \ > + " = \\(\\(1, 1\\) \\(1, 3\\) \\(1, 1\\)\\)" \ > + "print \$b after a change" > +gdb_test "print \$c" \ > + " = \\(\\(2, 2, 4\\) \\(2, 2, 2\\)\\)" \ > + "print \$c after a change" > +gdb_test "print some_var%array_one" \ > + " = \\(\\(1, 1\\) \\(1, 1\\) \\(1, 1\\)\\)" > +gdb_test "print some_var%array_two" \ > + " = \\(\\(2, 2, 2\\) \\(2, 2, 2\\)\\)" > + > +# Now modify the user variable '$a', which is a copy of 'some_var', > +# and then check how this change is reflected in the original > +# 'some_var', and the user variable $a. > +# > +# When we change 'a_field' which is a non-dynamic field within the > +# user variable, the change is only visible within the user variable. > +# > +# In contrast, when we change 'array_one' and 'array_two', which are > +# both fields of dynanic type, then the change is visible in both the > +# user variable and the original program variable 'some_var'. This > +# makes sense if you consider the dynamic type as if it was a C > +# pointer with automatic indirection. > +gdb_test_no_output "set \$a%a_field = 3" > +gdb_test_no_output "set \$a%array_one(2,2) = 3" > +gdb_test_no_output "set \$a%array_two(3,1) = 4" > +gdb_test "print \$a" \ > + " = \\( array_one = \\(\\(1, 1\\) \\(1, 3\\) \\(1, 1\\)\\), a_field = 3, array_two = \\(\\(2, 2, 4\\) \\(2, 2, 2\\)\\) \\)" > +gdb_test "print some_var" \ > + " = \\( array_one = \\(\\(1, 1\\) \\(1, 3\\) \\(1, 1\\)\\), a_field = 5, array_two = \\(\\(2, 2, 4\\) \\(2, 2, 2\\)\\) \\)" > diff --git a/gdb/testsuite/gdb.fortran/intvar-dynamic-types.f90 b/gdb/testsuite/gdb.fortran/intvar-dynamic-types.f90 > new file mode 100644 > index 00000000000..ace864812de > --- /dev/null > +++ b/gdb/testsuite/gdb.fortran/intvar-dynamic-types.f90 > @@ -0,0 +1,32 @@ > +! Copyright 2020 Free Software Foundation, Inc. > +! > +! This program is free software; you can redistribute it and/or modify > +! it under the terms of the GNU General Public License as published by > +! the Free Software Foundation; either version 3 of the License, or > +! (at your option) any later version. > +! > +! This program is distributed in the hope that it will be useful, > +! but WITHOUT ANY WARRANTY; without even the implied warranty of > +! MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +! GNU General Public License for more details. > +! > +! You should have received a copy of the GNU General Public License > +! along with this program. If not, see . > + > +program internal_var_test > + type :: some_type > + integer, allocatable :: array_one (:,:) > + integer :: a_field > + integer, allocatable :: array_two (:,:) > + end type some_type > + > + type(some_type) :: some_var > + > + allocate (some_var%array_one (2,3)) > + allocate (some_var%array_two (3,2)) > + some_var%array_one (:,:) = 1 > + some_var%a_field = 5 > + some_var%array_two (:,:) = 2 > + deallocate (some_var%array_one) ! Break here. > + deallocate (some_var%array_two) > +end program internal_var_test > diff --git a/gdb/value.c b/gdb/value.c > index 7c36c31dccd..12b0fd7b48c 100644 > --- a/gdb/value.c > +++ b/gdb/value.c > @@ -1784,12 +1784,37 @@ set_value_component_location (struct value *component, > component->location.computed.closure = funcs->copy_closure (whole); > } > > - /* If type has a dynamic resolved location property > - update it's value address. */ > + /* If either the WHOLE value, or the COMPONENT value has a dynamic > + resolved location property then update the address of the COMPONENT. > + > + If the COMPONENT itself has a dynamic location, and was an > + lval_internalvar_component, then we change this to lval_memory. > + Usually a component of an internalvar is created non-lazy, and has its > + content immediately copied from the parent internalvar. However, > + for components with a dynamic location, the content of the component > + is not contained within the parent, but is instead accessed > + indirectly. Further, the component will be created as a lazy value. > + > + By changing the type of the component to lval_memory we ensure that > + value_fetch_lazy can successfully load the component. */ > type = value_type (whole); > if (NULL != TYPE_DATA_LOCATION (type) > && TYPE_DATA_LOCATION_KIND (type) == PROP_CONST) > set_value_address (component, TYPE_DATA_LOCATION_ADDR (type)); > + > + type = value_type (component); > + if (NULL != TYPE_DATA_LOCATION (type) > + && TYPE_DATA_LOCATION_KIND (type) == PROP_CONST) > + { > + if (VALUE_LVAL (component) == lval_internalvar_component) > + { > + gdb_assert (value_lazy (component)); > + VALUE_LVAL (component) = lval_memory; > + } > + else > + gdb_assert (VALUE_LVAL (component) == lval_memory); > + set_value_address (component, TYPE_DATA_LOCATION_ADDR (type)); > + } > } > > /* Access to the value history. */ > -- > 2.25.4 >