* [PATCH] gdb: user variables with components of dynamic type
@ 2020-10-22 15:32 Andrew Burgess
2020-11-06 23:04 ` Andrew Burgess
2020-11-15 14:24 ` Joel Brobecker
0 siblings, 2 replies; 11+ messages in thread
From: Andrew Burgess @ 2020-10-22 15:32 UTC (permalink / raw)
To: gdb-patches
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 <http://www.gnu.org/licenses/>.
+
+# 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 <http://www.gnu.org/licenses/>.
+
+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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gdb: user variables with components of dynamic type
2020-10-22 15:32 [PATCH] gdb: user variables with components of dynamic type Andrew Burgess
@ 2020-11-06 23:04 ` Andrew Burgess
2020-11-08 10:50 ` Joel Brobecker
2020-11-15 14:24 ` Joel Brobecker
1 sibling, 1 reply; 11+ messages in thread
From: Andrew Burgess @ 2020-11-06 23:04 UTC (permalink / raw)
To: gdb-patches
Ping!
Any thoughts?
Thanks,
Andrew
* Andrew Burgess <andrew.burgess@embecosm.com> [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 <http://www.gnu.org/licenses/>.
> +
> +# 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 <http://www.gnu.org/licenses/>.
> +
> +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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gdb: user variables with components of dynamic type
2020-11-06 23:04 ` Andrew Burgess
@ 2020-11-08 10:50 ` Joel Brobecker
2020-11-12 16:00 ` Andrew Burgess
0 siblings, 1 reply; 11+ messages in thread
From: Joel Brobecker @ 2020-11-08 10:50 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
Hi Andrew,
> * Andrew Burgess <andrew.burgess@embecosm.com> [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.
Is it a pointer, or a reference? From what you are seeing and
what you are reported here, I assume these components are declared
as references? Or perhaps, after written 3 different versions of
a reply to this email, they are actually *neither*, but rather
are described as arrays with location expressions?
> > 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.
I am not surprised. Intuitively, like you said, we expect GDB
to "capture" the value of our variable, so we should have anything
lazy about it, or else this would indicate an incomplete capture.
> 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'.
This would be my understanding as well, provided the arrays are
*references*. For pointers, I think it's fine to continue with
the idea that we capture the target address, but not the target
memory region it points to.
> > 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.
I admit I don't really understand quite how this is all happening,
and how you're trying to deal with the issue.
It's possible that the compromise you suggest (treat dynamic components
the same as pointers) might be the most reasonable way out, but I think
it'll invite confusion on the users' side, and probably bug reports.
At the very least, I think we should warn users when we do this, so
as to be sure to set expectations right, on the spot.
Have you looked at how we handle components which are references?
I wonder how well we handle those...
--
Joel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gdb: user variables with components of dynamic type
2020-11-08 10:50 ` Joel Brobecker
@ 2020-11-12 16:00 ` Andrew Burgess
2020-11-15 14:07 ` Joel Brobecker
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Burgess @ 2020-11-12 16:00 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
Joel,
Thanks for your feedback.
> * Joel Brobecker <brobecker@adacore.com> [2020-11-08 14:50:59 +0400]:
>
> Hi Andrew,
>
> > * Andrew Burgess <andrew.burgess@embecosm.com> [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.
>
> Is it a pointer, or a reference? From what you are seeing and
> what you are reported here, I assume these components are declared
> as references? Or perhaps, after written 3 different versions of
> a reply to this email, they are actually *neither*, but rather
> are described as arrays with location expressions?
If we just look at 'some_var%array_one', here's it's type information:
<1><3c>: Abbrev Number: 5 (DW_TAG_structure_type)
<3d> DW_AT_name : (indirect string, offset: 0x0): some_type
<41> DW_AT_byte_size : 184
<42> DW_AT_decl_file : 1
<43> DW_AT_decl_line : 16
<44> DW_AT_sibling : <0x6d>
<2><48>: Abbrev Number: 6 (DW_TAG_member)
<49> DW_AT_name : (indirect string, offset: 0x5f): array_one
<4d> DW_AT_decl_file : 1
<4e> DW_AT_decl_line : 18
<4f> DW_AT_type : <0x6d>
<53> DW_AT_data_member_location: 0
<2><54>: Abbrev Number: 6 (DW_TAG_member)
<55> DW_AT_name : (indirect string, offset: 0x79): a_field
<59> DW_AT_decl_file : 1
<5a> DW_AT_decl_line : 19
<5b> DW_AT_type : <0xaa>
<5f> DW_AT_data_member_location: 88
<2><60>: Abbrev Number: 6 (DW_TAG_member)
<61> DW_AT_name : (indirect string, offset: 0x4c): array_two
<65> DW_AT_decl_file : 1
<66> DW_AT_decl_line : 20
<67> DW_AT_type : <0xb6>
<6b> DW_AT_data_member_location: 96
<2><6c>: Abbrev Number: 0
<1><6d>: Abbrev Number: 7 (DW_TAG_array_type)
<6e> DW_AT_ordering : 1 (column major)
<6f> DW_AT_data_location: 2 byte block: 97 6 (DW_OP_push_object_address; DW_OP_deref)
<72> DW_AT_allocated : 4 byte block: 97 6 30 2e (DW_OP_push_object_address; DW_OP_deref; DW_OP_lit0; DW_OP_ne)
<77> DW_AT_type : <0xaa> [ APB: This is signed 4-byte integer. ]
<7b> DW_AT_sibling : <0xaa> [ APB: This is signed 4-byte integer. ]
<2><7f>: Abbrev Number: 8 (DW_TAG_subrange_type)
<80> DW_AT_lower_bound : 4 byte block: 97 23 30 6 (DW_OP_push_object_address; DW_OP_plus_uconst: 48; DW_OP_deref)
<85> DW_AT_upper_bound : 4 byte block: 97 23 38 6 (DW_OP_push_object_address; DW_OP_plus_uconst: 56; DW_OP_deref)
<8a> DW_AT_byte_stride : 9 byte block: 97 23 28 6 97 23 20 6 1e (DW_OP_push_object_address; DW_OP_plus_uconst: 40; DW_OP_deref; DW_OP_push_object_address; DW_OP_plus_uconst: 32; DW_OP_deref; DW_OP_mul)
<2><94>: Abbrev Number: 8 (DW_TAG_subrange_type)
<95> DW_AT_lower_bound : 4 byte block: 97 23 48 6 (DW_OP_push_object_address; DW_OP_plus_uconst: 72; DW_OP_deref)
<9a> DW_AT_upper_bound : 4 byte block: 97 23 50 6 (DW_OP_push_object_address; DW_OP_plus_uconst: 80; DW_OP_deref)
<9f> DW_AT_byte_stride : 9 byte block: 97 23 40 6 97 23 20 6 1e (DW_OP_push_object_address; DW_OP_plus_uconst: 64; DW_OP_deref; DW_OP_push_object_address; DW_OP_plus_uconst: 32; DW_OP_deref; DW_OP_mul)
<2><a9>: Abbrev Number: 0
So your third choice was the winner, the array has dynamic type and
includes a computed data location.
>
> > > 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.
>
> I am not surprised. Intuitively, like you said, we expect GDB
> to "capture" the value of our variable, so we should have anything
> lazy about it, or else this would indicate an incomplete capture.
Agreed.
>
> > 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'.
>
> This would be my understanding as well, provided the arrays are
*> references*. For pointers, I think it's fine to continue with
> the idea that we capture the target address, but not the target
> memory region it points to.
Again, I think we agree.
The problem in terms of implementation is that really everything is
either a real inline value, or a pointer. All the other words are
just language sugar on top of these two choices.
In C then things are dead simple, something is either a pointer or is
the actual contents of the value, but the language exposes all this to
the programmer, so there's little room for surprise.
When we look at C++ references (basically pointers + automatic
dereferencing), or Fortran allocatable variables (same again) things
are less clear, we capture the underlying pointer, but can (especially
for Fortran) display the value with automatic dereferencing.
You specifically asked about references, I'm taking this to mean C++
references. Consider this test program:
#include <cstdio>
struct xxx
{
int &val;
};
void
func (xxx x)
{
printf ("Got: %d\n", x.val);
x.val = 0;
}
int
main ()
{
int i = 3;
xxx x = { i };
func (x); /* Break 1. */
printf ("Returning: %d\n", i); /* Break 2. */
return i;
}
Now our GDB session:
Breakpoint 1, main () at ref.cc:20
20 func (x);
(gdb) p x
$1 = {
val = @0x7fffffffb55c
}
(gdb) p x.val
$2 = (int &) @0x7fffffffb55c: 3
(gdb) set $foo = x
(gdb) p $foo
$3 = {
val = @0x7fffffffb55c
}
(gdb) p $foo.val
$4 = (int &) @0x7fffffffb55c: 3
(gdb) next
Got: 3
21 printf ("Returning: %d\n", i);
(gdb) p $foo.val
$5 = (int &) @0x7fffffffb55c: 0
(gdb) p x.val
$6 = (int &) @0x7fffffffb55c: 0
(gdb)
So we get the behaviour we might expect, the pointer value underlying
the reference is preserved, but the value pointed too is not.
Interestingly the choice was made to not automatically dereference the
C++ references, so they are displayed in a semi-pointer fashion, the
type prefix and the '@' symbol being what tells them apart from
regular pointers.
>
> > > 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.
>
> I admit I don't really understand quite how this is all happening,
> and how you're trying to deal with the issue.
I'm not sure which bit you don't understand, as in the next paragraph
you give an accurate description of what I'm proposing...
> It's possible that the compromise you suggest (treat dynamic components
> the same as pointers) might be the most reasonable way out, but I think
> it'll invite confusion on the users' side, and probably bug reports.
> At the very least, I think we should warn users when we do this, so
> as to be sure to set expectations right, on the spot.
Adding a warning would be reasonably simple, we can start with (in
value.c:set_internalvar):
if (is_dynamic_type (value_type (new_data.value)))
warning ("some warning text here...");
There's two problems, the first is easy enough to solve: if the top
level value being captured is dynamic, then we do capture the _actual_
value, it's only when a sub-component is dynamic that we have
problems. The above check will trigger if only the top-level value is
dynamic, so it warns in too many places.
As a concrete example, given this Fortran type:
type :: some_type
integer, allocatable :: array_one (:,:)
integer :: a_field
integer, allocatable :: array_two (:,:)
end type some_type
type(some_type) :: some_var
Then in GDB:
(gdb) set $foo = some_var
We capture the contents of the some_type struct, including the
pointers to the dynamic objects array_one and array_two. But if
instead we do:
(gdb) set $bar = some_var%array_one
Now we capture the full contents of array_one, there's no further
dynamic type resolution required. Changing 'some_var%array_one' will
not change the value of $bar, but the change would be see in $foo.
The harder problem is, what warning do we print?? I initially went
with:
components of dynamically typed values are not currently captured within internal variables
despite being a bit long, it's not immediately clear if a user will
know what 'dynamically typed values' means? Maybe we end up needing a
language specific warning, so for Fortran:
the values of allocatable fields are not currently captured within internal variables
thoughts or suggestions are welcome...
>
> Have you looked at how we handle components which are references?
> I wonder how well we handle those...
As above we treat them as pointers, but guard against possible
confusion by displaying them as pointers.
I would not like to change Fortran from displaying dynamnic types as
their actual value (and instead just display a pointer) as that seems
like a really bad change just to work around a limitation with
internal variables.
What I think is super interesting is how this all interacts with
pretty-printers. So, if I start with this test program:
#include <vector>
struct xxx
{
std::vector<int> lst;
};
static void
update (xxx &x)
{
x.lst.clear ();
x.lst.push_back (4);
x.lst.push_back (5);
x.lst.push_back (6);
}
int
main ()
{
xxx x = { { 1, 2, 3 } };
update (x);
return 0;
}
Then this is my GDB session (making use of C++ pretty-printers):
Temporary breakpoint 1, main () at lst.cc:20
20 xxx x = { { 1, 2, 3 } };
(gdb) n
21 update (x);
(gdb) set $foo=x
(gdb) p $foo
$1 = {
lst = std::vector of length 3, capacity 3 = {1, 2, 3}
}
(gdb) n
22 return 0;
(gdb) p $foo
$2 = {
lst = std::vector of length 3, capacity 3 = {4, 5, 6}
}
Notice that the contents of the std::vector changed in the variable
$foo. This I think is the closest to the Fortran case. For Fortran
GDB itself is providing the pretty-printing (it prints the dynamic
value rather than just displaying a pointer), and like with the
std::vector case above, the actual value is not captured, but just
printed.
I wonder if this problem should just be solved (at least in the
short/medium term) by improving the documentation for internal
variables?
Thanks,
Andrew
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gdb: user variables with components of dynamic type
2020-11-12 16:00 ` Andrew Burgess
@ 2020-11-15 14:07 ` Joel Brobecker
2020-12-03 11:04 ` Andrew Burgess
0 siblings, 1 reply; 11+ messages in thread
From: Joel Brobecker @ 2020-11-15 14:07 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
Hi Andrew,
> So your third choice was the winner, the array has dynamic type and
> includes a computed data location.
Thanks for providing the additional details.
> The problem in terms of implementation is that really everything is
> either a real inline value, or a pointer. All the other words are
> just language sugar on top of these two choices.
Agreed.
> When we look at C++ references (basically pointers + automatic
> dereferencing),
Thanks for sharing the experiment. This was actually a good refresher
for me.
> So we get the behaviour we might expect, the pointer value underlying
> the reference is preserved, but the value pointed too is not.
Yeah. What I took away from this experiment is that I think
the syntax in printing the value gives some clues as to why
some of the underlying value is not captured by the value history
mechanism. Whether this is enough or not will likely depend on
the level of the user, but I tend to think that a reasonably sharp
user seeing @<some-address>: can infer that the value printed
from it is the current value at the address shown. So good enough
for me.
> > It's possible that the compromise you suggest (treat dynamic components
> > the same as pointers) might be the most reasonable way out, but I think
> > it'll invite confusion on the users' side, and probably bug reports.
> > At the very least, I think we should warn users when we do this, so
> > as to be sure to set expectations right, on the spot.
>
> Adding a warning would be reasonably simple, we can start with (in
> value.c:set_internalvar):
>
> if (is_dynamic_type (value_type (new_data.value)))
> warning ("some warning text here...");
>
> There's two problems, the first is easy enough to solve: if the top
> level value being captured is dynamic, then we do capture the _actual_
> value, it's only when a sub-component is dynamic that we have
> problems. The above check will trigger if only the top-level value is
> dynamic, so it warns in too many places.
>
> As a concrete example, given this Fortran type:
>
> type :: some_type
> integer, allocatable :: array_one (:,:)
> integer :: a_field
> integer, allocatable :: array_two (:,:)
> end type some_type
>
> type(some_type) :: some_var
>
> Then in GDB:
>
> (gdb) set $foo = some_var
>
> We capture the contents of the some_type struct, including the
> pointers to the dynamic objects array_one and array_two. But if
> instead we do:
>
> (gdb) set $bar = some_var%array_one
>
> Now we capture the full contents of array_one, there's no further
> dynamic type resolution required. Changing 'some_var%array_one' will
> not change the value of $bar, but the change would be see in $foo.
>
> The harder problem is, what warning do we print?? I initially went
> with:
>
> components of dynamically typed values are not currently captured within internal variables
>
> despite being a bit long, it's not immediately clear if a user will
> know what 'dynamically typed values' means? Maybe we end up needing a
> language specific warning, so for Fortran:
>
> the values of allocatable fields are not currently captured within internal variables
>
> thoughts or suggestions are welcome...
I admit I'm having minor headaches trying to wrap my head around
the various scenarios and how we would want to deal with the situation.
For instance, do we auto-dereference [1] the same way for all languages?
For Ada, for instance, I think we always auto-derefrence, regardless
if the data we're printing is top-level or whether it's part of
an aggregate. It sounds like the same is True for Fortran as well.
But it does look from your experiments that C++ only dereferences
in some of the cases (e.g. when printing a struct with a field being
a reference, the field's value is the address, without the actual
underlying value). This question would have an impact on the warning,
since we would warn only when there is auto-dereferencing.
For the timing of the warning, at least, it seems clear to me that
it would be printed when printing the value. Do we want to print it
when accessing the value too? It would seem logical, I think, but
are there scenarios where this could be counter-productive?
There's also the question of avoiding to get the warning printed
in the middle of the value itself. This might be a bit minor, but
it's something I've always found to be distracting and hard to read,
when a warning is lost with some other data being printed.
> > Have you looked at how we handle components which are references?
> > I wonder how well we handle those...
>
> As above we treat them as pointers, but guard against possible
> confusion by displaying them as pointers.
>
> I would not like to change Fortran from displaying dynamnic types as
> their actual value (and instead just display a pointer) as that seems
> like a really bad change just to work around a limitation with
> internal variables.
100% agreed.
> What I think is super interesting is how this all interacts with
> pretty-printers. So, if I start with this test program:
Interesting indeed.
> I wonder if this problem should just be solved (at least in the
> short/medium term) by improving the documentation for internal
> variables?
For me, if I had to summarize my current thinking thanks to our
discussions so far, I would say:
(a) The case of fields whose type is dynamic is really very
similar to the case of references.
(b) Since we handle references the same as pointers, I think
it's fine as a first instance to fix our immediate issue,
which is the internal error, by providing the exact same
kind of behavior for those dynamic fields as we provide
for references.
(c) Finding a way to make this clearer for users would be a nice
enhancement, but this would be a general enhancement, not
something required in the context of your patch.
So you and I are converging towards the same solution in the code,
and I agree that a documentation update might be useful as well.
Going back to your original patch, I would need more research
than what I have time for to determine whether I'd fix it
the same way you did at the location you did. After much staring
with the entire function's implementation as a context, the patch
does make sense to me, especially if I ignore a bit the asserts
for a minute. Hopefully others with a more complete knowledge of
the area of value-saving for the value history can chime in.
I wish I could be more help! :-/
--
Joel
[1] I know your situation has to do with dynamic types rather than
references, but I think both cases are in the same boat and could
be treated the same.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gdb: user variables with components of dynamic type
2020-10-22 15:32 [PATCH] gdb: user variables with components of dynamic type Andrew Burgess
2020-11-06 23:04 ` Andrew Burgess
@ 2020-11-15 14:24 ` Joel Brobecker
2021-01-08 11:56 ` Andrew Burgess
1 sibling, 1 reply; 11+ messages in thread
From: Joel Brobecker @ 2020-11-15 14:24 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
Hi Andrew,
> - /* 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));
> + }
I have a suggestion, but I am not sure it might be right for
everyone, as perhaps other people's brains might be thinking
differently.
In your patch you architected it with one large comment at beginning
followed by a number of conditinal branches, with the comment explaining
the various scenarios that we're about to handle. If your brain works
like mine, I would find the following approach to make it easier for me
to understand the code:
type = value_type (component);
if (NULL != TYPE_DATA_LOCATION (type)
&& TYPE_DATA_LOCATION_KIND (type) == PROP_CONST)
{
/* COMPONENT's type has a dynamic location, so we need
to update our component's address to reflect the actual
location after resolution. */
if (VALUE_LVAL (component) == lval_internalvar_component)
{
/* This happens when [...].
We have to change the lval to lval_memory because ... */
gdb_assert (value_lazy (component));
VALUE_LVAL (component) = lval_memory;
}
/* At this point, we assume that COMPONENT is now an lval_memory,
and we can now set it address. */
gdb_assert (VALUE_LVAL (component) == lval_memory);
set_value_address (component, TYPE_DATA_LOCATION_ADDR (type));
}
As I mentioned, maybe you don't think the read code the same way
as I do, and so it would be absolutely fine with me if you don't
agree with the suggestion ;-).
--
Joel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gdb: user variables with components of dynamic type
2020-11-15 14:07 ` Joel Brobecker
@ 2020-12-03 11:04 ` Andrew Burgess
2020-12-06 9:59 ` Joel Brobecker
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Burgess @ 2020-12-03 11:04 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
Hi Joel,
Sorry for the length of time it has taken me to get back to you.
Initially I was just chewing over your feedback, then I got
sidetracked.....
I have a question about what you wrote, see inline below.
* Joel Brobecker <brobecker@adacore.com> [2020-11-15 18:07:59 +0400]:
> Hi Andrew,
>
> > So your third choice was the winner, the array has dynamic type and
> > includes a computed data location.
>
> Thanks for providing the additional details.
>
> > The problem in terms of implementation is that really everything is
> > either a real inline value, or a pointer. All the other words are
> > just language sugar on top of these two choices.
>
> Agreed.
>
> > When we look at C++ references (basically pointers + automatic
> > dereferencing),
>
> Thanks for sharing the experiment. This was actually a good refresher
> for me.
>
> > So we get the behaviour we might expect, the pointer value underlying
> > the reference is preserved, but the value pointed too is not.
>
> Yeah. What I took away from this experiment is that I think
> the syntax in printing the value gives some clues as to why
> some of the underlying value is not captured by the value history
> mechanism. Whether this is enough or not will likely depend on
> the level of the user, but I tend to think that a reasonably sharp
> user seeing @<some-address>: can infer that the value printed
> from it is the current value at the address shown. So good enough
> for me.
>
> > > It's possible that the compromise you suggest (treat dynamic components
> > > the same as pointers) might be the most reasonable way out, but I think
> > > it'll invite confusion on the users' side, and probably bug reports.
> > > At the very least, I think we should warn users when we do this, so
> > > as to be sure to set expectations right, on the spot.
> >
> > Adding a warning would be reasonably simple, we can start with (in
> > value.c:set_internalvar):
> >
> > if (is_dynamic_type (value_type (new_data.value)))
> > warning ("some warning text here...");
> >
> > There's two problems, the first is easy enough to solve: if the top
> > level value being captured is dynamic, then we do capture the _actual_
> > value, it's only when a sub-component is dynamic that we have
> > problems. The above check will trigger if only the top-level value is
> > dynamic, so it warns in too many places.
> >
> > As a concrete example, given this Fortran type:
> >
> > type :: some_type
> > integer, allocatable :: array_one (:,:)
> > integer :: a_field
> > integer, allocatable :: array_two (:,:)
> > end type some_type
> >
> > type(some_type) :: some_var
> >
> > Then in GDB:
> >
> > (gdb) set $foo = some_var
> >
> > We capture the contents of the some_type struct, including the
> > pointers to the dynamic objects array_one and array_two. But if
> > instead we do:
> >
> > (gdb) set $bar = some_var%array_one
> >
> > Now we capture the full contents of array_one, there's no further
> > dynamic type resolution required. Changing 'some_var%array_one' will
> > not change the value of $bar, but the change would be see in $foo.
> >
> > The harder problem is, what warning do we print?? I initially went
> > with:
> >
> > components of dynamically typed values are not currently captured within internal variables
> >
> > despite being a bit long, it's not immediately clear if a user will
> > know what 'dynamically typed values' means? Maybe we end up needing a
> > language specific warning, so for Fortran:
> >
> > the values of allocatable fields are not currently captured within internal variables
> >
> > thoughts or suggestions are welcome...
>
> I admit I'm having minor headaches trying to wrap my head around
> the various scenarios and how we would want to deal with the situation.
>
> For instance, do we auto-dereference [1] the same way for all languages?
> For Ada, for instance, I think we always auto-derefrence, regardless
> if the data we're printing is top-level or whether it's part of
> an aggregate. It sounds like the same is True for Fortran as well.
> But it does look from your experiments that C++ only dereferences
> in some of the cases (e.g. when printing a struct with a field being
> a reference, the field's value is the address, without the actual
> underlying value). This question would have an impact on the warning,
> since we would warn only when there is auto-dereferencing.
>
> For the timing of the warning, at least, it seems clear to me that
> it would be printed when printing the value. Do we want to print it
> when accessing the value too? It would seem logical, I think, but
> are there scenarios where this could be counter-productive?
>
> There's also the question of avoiding to get the warning printed
> in the middle of the value itself. This might be a bit minor, but
> it's something I've always found to be distracting and hard to read,
> when a warning is lost with some other data being printed.
>
> > > Have you looked at how we handle components which are references?
> > > I wonder how well we handle those...
> >
> > As above we treat them as pointers, but guard against possible
> > confusion by displaying them as pointers.
> >
> > I would not like to change Fortran from displaying dynamnic types as
> > their actual value (and instead just display a pointer) as that seems
> > like a really bad change just to work around a limitation with
> > internal variables.
>
> 100% agreed.
That's good, and not really surprising as from what you wrote I think
this is what Ada does, but then later on you write...
>
> > What I think is super interesting is how this all interacts with
> > pretty-printers. So, if I start with this test program:
>
> Interesting indeed.
>
> > I wonder if this problem should just be solved (at least in the
> > short/medium term) by improving the documentation for internal
> > variables?
>
> For me, if I had to summarize my current thinking thanks to our
> discussions so far, I would say:
>
> (a) The case of fields whose type is dynamic is really very
> similar to the case of references.
>
> (b) Since we handle references the same as pointers, I think
> it's fine as a first instance to fix our immediate issue,
> which is the internal error, by providing the exact same
> kind of behavior for those dynamic fields as we provide
> for references.
...this.
Aren't you here arguing that we should immediately fix this issue by
changing the way dynamic types are printed such that they are handled
more like C++ references (i.e. print the address rather than the
contents). Which seems to be the complete opposite of your opinion
above.
I'm curious, if Ada makes use of dynamic typing then what does GDB
print when an object of dynamic type is nested within some other
object?
What happens if the parent object is assigned to a user variable?
My Ada foo is weak, but I'll try to dig into this and understand what
happens, but maybe you know the answer already.
I guess given your concerns then the idea solution here would be one
that somehow allows GDB to capture all of the dynamic content at the
time the user variable is created. I'll try to figure out if there's
any good ways to achieve this.
Thanks,
Andrew
>
> (c) Finding a way to make this clearer for users would be a nice
> enhancement, but this would be a general enhancement, not
> something required in the context of your patch.
>
> So you and I are converging towards the same solution in the code,
> and I agree that a documentation update might be useful as well.
>
> Going back to your original patch, I would need more research
> than what I have time for to determine whether I'd fix it
> the same way you did at the location you did. After much staring
> with the entire function's implementation as a context, the patch
> does make sense to me, especially if I ignore a bit the asserts
> for a minute. Hopefully others with a more complete knowledge of
> the area of value-saving for the value history can chime in.
>
> I wish I could be more help! :-/
>
> --
> Joel
>
> [1] I know your situation has to do with dynamic types rather than
> references, but I think both cases are in the same boat and could
> be treated the same.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gdb: user variables with components of dynamic type
2020-12-03 11:04 ` Andrew Burgess
@ 2020-12-06 9:59 ` Joel Brobecker
0 siblings, 0 replies; 11+ messages in thread
From: Joel Brobecker @ 2020-12-06 9:59 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
Hi Andrew,
> Sorry for the length of time it has taken me to get back to you.
> Initially I was just chewing over your feedback, then I got
> sidetracked.....
>
> I have a question about what you wrote, see inline below.
> > > I would not like to change Fortran from displaying dynamnic types as
> > > their actual value (and instead just display a pointer) as that seems
> > > like a really bad change just to work around a limitation with
> > > internal variables.
> >
> > 100% agreed.
>
> That's good, and not really surprising as from what you wrote I think
> this is what Ada does, but then later on you write...
>
> >
> > > What I think is super interesting is how this all interacts with
> > > pretty-printers. So, if I start with this test program:
> >
> > Interesting indeed.
> >
> > > I wonder if this problem should just be solved (at least in the
> > > short/medium term) by improving the documentation for internal
> > > variables?
> >
> > For me, if I had to summarize my current thinking thanks to our
> > discussions so far, I would say:
> >
> > (a) The case of fields whose type is dynamic is really very
> > similar to the case of references.
> >
> > (b) Since we handle references the same as pointers, I think
> > it's fine as a first instance to fix our immediate issue,
> > which is the internal error, by providing the exact same
> > kind of behavior for those dynamic fields as we provide
> > for references.
>
> ...this.
>
> Aren't you here arguing that we should immediately fix this issue by
> changing the way dynamic types are printed such that they are handled
> more like C++ references (i.e. print the address rather than the
> contents). Which seems to be the complete opposite of your opinion
> above.
It does like a contradiction indeed. I must have had a brainfarting
moment when I wrote that -- very sorry!
> I'm curious, if Ada makes use of dynamic typing then what does GDB
> print when an object of dynamic type is nested within some other
> object?
Ada resolves the dynamic object, and presents the actual type.
Reference layers are also silently peeled (we may need to rediscuss
the "silently" part, but that would be a separate discussion).
I don't have a full map of the various possibilities, but it's not
unusual for the "variable" part of an object to be replaced by
a reference. It's not always replaced by a reference, though.
> What happens if the parent object is assigned to a user variable?
Tom was recently asked about something like that, when one of
our developers had a situation where the value of something stored
in $1 was changing over time, and was surprised by that.
Only the contents of the value that's "directly accessible" to
re-use Tom's words, are captured. The printing of an object in
Ada can involve some silent dereferencing and the de-referenced
data is not captured. So re-printing a captured value can yield
a different answer.
> I guess given your concerns then the idea solution here would be one
> that somehow allows GDB to capture all of the dynamic content at the
> time the user variable is created. I'll try to figure out if there's
> any good ways to achieve this.
That would definitely be ideal, absolutely. But it does sound like
a significant undertaking. Just thinking of the simple case of
a record with one field being a reference. How do go from capturing
a buffer with the contents of the record, to a case where we have
to have part of that buffer be treated specially so as to know that
the data targetted by the corresponding field is not obtained by
dereferencing the pointer, but by reading something elsewhere.
So, for me, what this discussion has helped me with is that
perhaps the best compromise is to do what's right when printing
the value of an expression, and then accept that the value
capturing method has a limitation that references are captured
as pointers, and that therefore that the captured data is not
entirely complete and may change while the inferior runs further.
Or said differently, if I remember what your patch is trying to do,
you've helped me come to the same conclusion as the one you reached
back when you proposed the patch.
> > (c) Finding a way to make this clearer for users would be a nice
> > enhancement, but this would be a general enhancement, not
> > something required in the context of your patch.
> >
> > So you and I are converging towards the same solution in the code,
> > and I agree that a documentation update might be useful as well.
> >
> > Going back to your original patch, I would need more research
> > than what I have time for to determine whether I'd fix it
> > the same way you did at the location you did. After much staring
> > with the entire function's implementation as a context, the patch
> > does make sense to me, especially if I ignore a bit the asserts
> > for a minute. Hopefully others with a more complete knowledge of
> > the area of value-saving for the value history can chime in.
> >
> > I wish I could be more help! :-/
> >
> > --
> > Joel
> >
> > [1] I know your situation has to do with dynamic types rather than
> > references, but I think both cases are in the same boat and could
> > be treated the same.
--
Joel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gdb: user variables with components of dynamic type
2020-11-15 14:24 ` Joel Brobecker
@ 2021-01-08 11:56 ` Andrew Burgess
2021-01-11 14:30 ` Luis Machado
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Burgess @ 2021-01-08 11:56 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
* Joel Brobecker <brobecker@adacore.com> [2020-11-15 18:24:58 +0400]:
> Hi Andrew,
>
> > - /* 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));
> > + }
>
> I have a suggestion, but I am not sure it might be right for
> everyone, as perhaps other people's brains might be thinking
> differently.
>
> In your patch you architected it with one large comment at beginning
> followed by a number of conditinal branches, with the comment explaining
> the various scenarios that we're about to handle. If your brain works
> like mine, I would find the following approach to make it easier for me
> to understand the code:
>
> type = value_type (component);
> if (NULL != TYPE_DATA_LOCATION (type)
> && TYPE_DATA_LOCATION_KIND (type) == PROP_CONST)
> {
> /* COMPONENT's type has a dynamic location, so we need
> to update our component's address to reflect the actual
> location after resolution. */
> if (VALUE_LVAL (component) == lval_internalvar_component)
> {
> /* This happens when [...].
> We have to change the lval to lval_memory because ... */
> gdb_assert (value_lazy (component));
> VALUE_LVAL (component) = lval_memory;
> }
>
> /* At this point, we assume that COMPONENT is now an lval_memory,
> and we can now set it address. */
> gdb_assert (VALUE_LVAL (component) == lval_memory);
> set_value_address (component, TYPE_DATA_LOCATION_ADDR (type));
> }
>
> As I mentioned, maybe you don't think the read code the same way
> as I do, and so it would be absolutely fine with me if you don't
> agree with the suggestion ;-).
After rereading the our other discussion of this patch I believe the
conclusion was that you don't object to this patch.
As nobody else has commented I went ahead and pushed the version
below.
The only changes are:
- Split the single big comment up as you suggested, and
- Tweaked some wording in the commit log.
Thanks,
Andrew
---
commit 3c8c6de21daaae6450860af6b0df3b464ccb19f6
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Thu Oct 22 11:34:52 2020 +0100
gdb: user variables with components of dynamic type
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, in
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 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 though 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 as 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 an 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.
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..16dc603cb47
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/intvar-dynamic-types.exp
@@ -0,0 +1,97 @@
+# Copyright 2020-2021 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 <http://www.gnu.org/licenses/>.
+
+# 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..ef51a32ec77
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/intvar-dynamic-types.f90
@@ -0,0 +1,32 @@
+! Copyright 2020-2021 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 <http://www.gnu.org/licenses/>.
+
+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 a20ae5af4f0..c84698d25e0 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1784,12 +1784,45 @@ 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 the WHOLE value has a dynamically resolved location property then
+ update the address of 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));
+
+ /* Similarly, if the COMPONENT value has a dynamically resolved location
+ property then update its address. */
+ type = value_type (component);
+ if (NULL != TYPE_DATA_LOCATION (type)
+ && TYPE_DATA_LOCATION_KIND (type) == PROP_CONST)
+ {
+ /* If the COMPONENT has a dynamic location, and is an
+ lval_internalvar_component, then we change it to a 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.
+
+ This solution isn't ideal, but a real fix would require values to
+ carry around both the parent value contents, and the contents of
+ any dynamic fields within the parent. This is a substantial
+ change to how values work in GDB. */
+ 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. */
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gdb: user variables with components of dynamic type
2021-01-08 11:56 ` Andrew Burgess
@ 2021-01-11 14:30 ` Luis Machado
2021-01-11 14:55 ` Luis Machado
0 siblings, 1 reply; 11+ messages in thread
From: Luis Machado @ 2021-01-11 14:30 UTC (permalink / raw)
To: Andrew Burgess, Joel Brobecker; +Cc: gdb-patches
Hi,
On 1/8/21 8:56 AM, Andrew Burgess wrote:
> * Joel Brobecker <brobecker@adacore.com> [2020-11-15 18:24:58 +0400]:
>
>> Hi Andrew,
>>
>>> - /* 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));
>>> + }
>>
>> I have a suggestion, but I am not sure it might be right for
>> everyone, as perhaps other people's brains might be thinking
>> differently.
>>
>> In your patch you architected it with one large comment at beginning
>> followed by a number of conditinal branches, with the comment explaining
>> the various scenarios that we're about to handle. If your brain works
>> like mine, I would find the following approach to make it easier for me
>> to understand the code:
>>
>> type = value_type (component);
>> if (NULL != TYPE_DATA_LOCATION (type)
>> && TYPE_DATA_LOCATION_KIND (type) == PROP_CONST)
>> {
>> /* COMPONENT's type has a dynamic location, so we need
>> to update our component's address to reflect the actual
>> location after resolution. */
>> if (VALUE_LVAL (component) == lval_internalvar_component)
>> {
>> /* This happens when [...].
>> We have to change the lval to lval_memory because ... */
>> gdb_assert (value_lazy (component));
>> VALUE_LVAL (component) = lval_memory;
>> }
>>
>> /* At this point, we assume that COMPONENT is now an lval_memory,
>> and we can now set it address. */
>> gdb_assert (VALUE_LVAL (component) == lval_memory);
>> set_value_address (component, TYPE_DATA_LOCATION_ADDR (type));
>> }
>>
>> As I mentioned, maybe you don't think the read code the same way
>> as I do, and so it would be absolutely fine with me if you don't
>> agree with the suggestion ;-).
>
> After rereading the our other discussion of this patch I believe the
> conclusion was that you don't object to this patch.
>
> As nobody else has commented I went ahead and pushed the version
> below.
>
> The only changes are:
>
> - Split the single big comment up as you suggested, and
> - Tweaked some wording in the commit log.
>
> Thanks,
> Andrew
This seems to be causing some internal errors on AArch64-Linux Ubuntu 18.04.
FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a (GDB internal error)
FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a: print array_one
field (GDB internal error)
FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a: print array_two
field (GDB internal error)
FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a: print full
contents (GDB internal error)
FAIL: gdb.fortran/intvar-dynamic-types.exp: print $b after a change
FAIL: gdb.fortran/intvar-dynamic-types.exp: print $c after a change
FAIL: gdb.fortran/intvar-dynamic-types.exp: print some_var
FAIL: gdb.fortran/intvar-dynamic-types.exp: set $a%array_one(2,2) = 3
FAIL: gdb.fortran/intvar-dynamic-types.exp: set $a%array_two(3,1) = 4
FAIL: gdb.fortran/intvar-dynamic-types.exp: set $b(2,2) = 3
FAIL: gdb.fortran/intvar-dynamic-types.exp: set $c(3,1) = 4
The internal error is...
print $a
$1 = ( array_one = ../../../repos/binutils-gdb/gdb/value.c:3983:
internal-error: Unexpected lazy value type.
Any ideas? I can provide the full log as well, if you think that is useful.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gdb: user variables with components of dynamic type
2021-01-11 14:30 ` Luis Machado
@ 2021-01-11 14:55 ` Luis Machado
0 siblings, 0 replies; 11+ messages in thread
From: Luis Machado @ 2021-01-11 14:55 UTC (permalink / raw)
To: Andrew Burgess, Joel Brobecker; +Cc: gdb-patches
On 1/11/21 11:30 AM, Luis Machado wrote:
> Hi,
>
>
> On 1/8/21 8:56 AM, Andrew Burgess wrote:
>> * Joel Brobecker <brobecker@adacore.com> [2020-11-15 18:24:58 +0400]:
>>
>>> Hi Andrew,
>>>
>>>> - /* 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));
>>>> + }
>>>
>>> I have a suggestion, but I am not sure it might be right for
>>> everyone, as perhaps other people's brains might be thinking
>>> differently.
>>>
>>> In your patch you architected it with one large comment at beginning
>>> followed by a number of conditinal branches, with the comment explaining
>>> the various scenarios that we're about to handle. If your brain works
>>> like mine, I would find the following approach to make it easier for me
>>> to understand the code:
>>>
>>> type = value_type (component);
>>> if (NULL != TYPE_DATA_LOCATION (type)
>>> && TYPE_DATA_LOCATION_KIND (type) == PROP_CONST)
>>> {
>>> /* COMPONENT's type has a dynamic location, so we need
>>> to update our component's address to reflect the actual
>>> location after resolution. */
>>> if (VALUE_LVAL (component) == lval_internalvar_component)
>>> {
>>> /* This happens when [...].
>>> We have to change the lval to lval_memory because ... */
>>> gdb_assert (value_lazy (component));
>>> VALUE_LVAL (component) = lval_memory;
>>> }
>>>
>>> /* At this point, we assume that COMPONENT is now an
>>> lval_memory,
>>> and we can now set it address. */
>>> gdb_assert (VALUE_LVAL (component) == lval_memory);
>>> set_value_address (component, TYPE_DATA_LOCATION_ADDR (type));
>>> }
>>>
>>> As I mentioned, maybe you don't think the read code the same way
>>> as I do, and so it would be absolutely fine with me if you don't
>>> agree with the suggestion ;-).
>>
>> After rereading the our other discussion of this patch I believe the
>> conclusion was that you don't object to this patch.
>>
>> As nobody else has commented I went ahead and pushed the version
>> below.
>>
>> The only changes are:
>>
>> - Split the single big comment up as you suggested, and
>> - Tweaked some wording in the commit log.
>>
>> Thanks,
>> Andrew
>
>
> This seems to be causing some internal errors on AArch64-Linux Ubuntu
> 18.04.
>
> FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a (GDB internal error)
> FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a: print array_one
> field (GDB internal error)
> FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a: print array_two
> field (GDB internal error)
> FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a: print full
> contents (GDB internal error)
> FAIL: gdb.fortran/intvar-dynamic-types.exp: print $b after a change
> FAIL: gdb.fortran/intvar-dynamic-types.exp: print $c after a change
> FAIL: gdb.fortran/intvar-dynamic-types.exp: print some_var
> FAIL: gdb.fortran/intvar-dynamic-types.exp: set $a%array_one(2,2) = 3
> FAIL: gdb.fortran/intvar-dynamic-types.exp: set $a%array_two(3,1) = 4
> FAIL: gdb.fortran/intvar-dynamic-types.exp: set $b(2,2) = 3
> FAIL: gdb.fortran/intvar-dynamic-types.exp: set $c(3,1) = 4
>
> The internal error is...
>
> print $a
> $1 = ( array_one = ../../../repos/binutils-gdb/gdb/value.c:3983:
> internal-error: Unexpected lazy value type.
>
> Any ideas? I can provide the full log as well, if you think that is useful.
I think this has been addressed already in a later change. I just
noticed some libiberty change broke the HEAD build and I was sitting at
a GDB from Jan 4th. This is a non-issue then.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-01-11 14:55 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-22 15:32 [PATCH] gdb: user variables with components of dynamic type Andrew Burgess
2020-11-06 23:04 ` Andrew Burgess
2020-11-08 10:50 ` Joel Brobecker
2020-11-12 16:00 ` Andrew Burgess
2020-11-15 14:07 ` Joel Brobecker
2020-12-03 11:04 ` Andrew Burgess
2020-12-06 9:59 ` Joel Brobecker
2020-11-15 14:24 ` Joel Brobecker
2021-01-08 11:56 ` Andrew Burgess
2021-01-11 14:30 ` Luis Machado
2021-01-11 14:55 ` Luis Machado
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).