public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFC] gdb: introduce limited array lengths while printing values
@ 2021-10-06 17:29 Andrew Burgess
  2021-10-06 17:36 ` Eli Zaretskii
                   ` (10 more replies)
  0 siblings, 11 replies; 13+ messages in thread
From: Andrew Burgess @ 2021-10-06 17:29 UTC (permalink / raw)
  To: gdb-patches

This commit introduces the idea of loading only part of an array in
order to print it, what I call "limited length" arrays.

The motivation behind this work is to make it possible to print slices
of very large arrays, where very large means bigger than
max-value-size.

Consider this GDB session with the current GDB:

  (gdb) set max-value-size 100
  (gdb) p large_1d_array
  value requires 400 bytes, which is more than max-value-size
  (gdb) p -elements 10 -- large_1d_array
  value requires 400 bytes, which is more than max-value-size

notice that the request to print 10 elements still fails, even though
10 elements should be less than the max-value-size. With a patched
version of GDB:

  (gdb) p -elements 10 -- large_1d_array
  $1 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9...}

So now the print has succeeded.  The only remaining problem is that
the '$1 = ' implies that the value of the array has been placed into
the value history, so what happens if the tries to reprint the full
array without the print elements?  My solution for now is:

  (gdb) p $1
  $2 = <unavailable>

This patch is currently RFC, I would like to hear what people think
about both the idea in general, and the approach taken.

One question I have is whether the value history problem would be
better addressed in a different way, for example, we could just drop
the '$1 = ' for values that are not being added into the history, so
things would look like:

  (gdb) p -elements 10 -- large_1d_array
  {0, 1, 2, 3, 4, 5, 6, 7, 8, 9...}

which might be a better solution.  Another possibility would be to tag
the "unavailable" value with a reason field so we could do something
like:

  (gdb) p $1
  $2 = <unavailable: original value was too large>

which is slightly more informative, but clearly is a more invasive
change to the value structure.

The current solution relies on loading part of the array value into
GDB's memory, this is done because very early in the value printing
code (in value_check_printable) we call value_entirely_optimized_out,
which loads the values complete contents into memory.

But I think Tom was looking into having the value optimized out checks
not actually load the value in at one point, so maybe, if that change
landed, then we could investigate the possibility of having the array
printing code only load the elements from the target one at a
time (with the dcache providing some optimisation), which might avoid
the need to perform the current partial load?

Anyway, I'd be interested to hear people's thoughts; is this a
valuable change?  Which approach seems like the right way to go?

Thanks,
Andrew


---
 gdb/NEWS                                     |   6 +
 gdb/doc/gdb.texinfo                          |   9 +
 gdb/printcmd.c                               |   5 +
 gdb/testsuite/gdb.ada/limited-length.exp     |  51 +++++
 gdb/testsuite/gdb.ada/limited-length/foo.adb |  32 ++++
 gdb/testsuite/gdb.ada/limited-length/pck.adb |  23 +++
 gdb/testsuite/gdb.ada/limited-length/pck.ads |  19 ++
 gdb/testsuite/gdb.base/limited-length.c      |  31 ++++
 gdb/testsuite/gdb.base/limited-length.exp    |  72 ++++++++
 gdb/testsuite/gdb.fortran/limited-length.exp |  66 +++++++
 gdb/testsuite/gdb.fortran/limited-length.f90 |  37 ++++
 gdb/value.c                                  | 184 +++++++++++++++++--
 gdb/value.h                                  |  17 ++
 13 files changed, 535 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/limited-length.exp
 create mode 100644 gdb/testsuite/gdb.ada/limited-length/foo.adb
 create mode 100644 gdb/testsuite/gdb.ada/limited-length/pck.adb
 create mode 100644 gdb/testsuite/gdb.ada/limited-length/pck.ads
 create mode 100644 gdb/testsuite/gdb.base/limited-length.c
 create mode 100644 gdb/testsuite/gdb.base/limited-length.exp
 create mode 100644 gdb/testsuite/gdb.fortran/limited-length.exp
 create mode 100644 gdb/testsuite/gdb.fortran/limited-length.f90

diff --git a/gdb/NEWS b/gdb/NEWS
index d7c29c82edb..cf74bb0260c 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,12 @@
 
 *** Changes since GDB 11
 
+* The 'set print elements' setting now helps when printing large
+  arrays.  If an array would otherwise exceed max-value-size, but
+  'print elements' is set such that the number of elements to print is
+  less than max-value-size, GDB will now still print the array,
+  however, the array will not be added into the value history.
+
 * New commands
 
 maint set backtrace-on-fatal-signal on|off
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index fcfdc26ac1a..27f7348f769 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11374,6 +11374,14 @@
 Setting @var{number-of-elements} to @code{unlimited} or zero means
 that the number of elements to print is unlimited.
 
+When printing very large arrays, whose size is greater than
+@code{max-value-size} (@pxref{set max-value-size,,max-value-size}), if
+the @code{print elements} is set such that the size of the elements
+being printed is less than @code{max-value-size}, then @value{GDBN}
+will print the array (up to the @code{print elements} limit), but the
+value will not be added to the value history (@pxref{Value History,
+,Value History}).
+
 @item show print elements
 Display the number of elements of a large array that @value{GDBN} will print.
 If the number is 0, then the printing is unlimited.
@@ -13779,6 +13787,7 @@
 @value{GDBN} to try and allocate an overly large amount of memory.
 
 @table @code
+@anchor{set max-value-size}
 @kindex set max-value-size
 @item set max-value-size @var{bytes}
 @itemx set max-value-size unlimited
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 2fe3f4b0cc5..022cf35da51 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1247,6 +1247,11 @@ print_command_parse_format (const char **expp, const char *cmdname,
 void
 print_value (value *val, const value_print_options &opts)
 {
+  /* This setting allows large arrays to be printed by limiting the number
+     of elements that are loaded into GDB's memory; we only need to load as
+     many array elements as we plan to print.  */
+  scoped_array_length_limiting limit_large_arrays (opts.print_max);
+
   int histindex = record_latest_value (val);
 
   annotate_value_history_begin (histindex, value_type (val));
diff --git a/gdb/testsuite/gdb.ada/limited-length.exp b/gdb/testsuite/gdb.ada/limited-length.exp
new file mode 100644
index 00000000000..e5c3e3f3949
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/limited-length.exp
@@ -0,0 +1,51 @@
+# Copyright 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/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+  return -1
+}
+
+clean_restart ${testfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
+if ![runto "foo.adb:$bp_location" ] then {
+  perror "Couldn't run ${testfile}"
+  return
+}
+
+gdb_test "print Large_Array" \
+    "\\(\\(\\(1, 2, 3, 4\\), \\(5, 6, 7, 8\\), \\(9, 10, 11, 12\\), .* \\(53, 54, 55, 56\\), \\(57, 58, 59, 60\\), \\(61, 62, 63, 64\\)\\)\\)"
+
+set elem_size [get_valueof "/d" "(Large_Array(1,1,1)'Size + 7) / 8" "*unknown"]
+set max_value_size [expr $elem_size * 32]
+gdb_test_no_output "set max-value-size $max_value_size"
+
+gdb_test "print Large_Array" \
+    "value of type .* requires $decimal bytes, which is more than max-value-size" \
+    "print Large_Array with reduced max-value-size"
+
+gdb_test "print -elements 2 -- Large_Array" \
+    "\\(\\(\\(1, 2\\.\\.\\.\\), \\(5, 6\\.\\.\\.\\)\\.\\.\\.\\), \\(\\(17, 18\\.\\.\\.\\), \\(21, 22\\.\\.\\.\\)\\.\\.\\.\\)\\.\\.\\.\\)"
+
+gdb_test_no_output "set print elements 2"
+gdb_test "print Large_Array" \
+    "\\(\\(\\(1, 2\\.\\.\\.\\), \\(5, 6\\.\\.\\.\\)\\.\\.\\.\\), \\(\\(17, 18\\.\\.\\.\\), \\(21, 22\\.\\.\\.\\)\\.\\.\\.\\)\\.\\.\\.\\)" \
+    "print Large_Array with 'set print elements 2' in effect"
diff --git a/gdb/testsuite/gdb.ada/limited-length/foo.adb b/gdb/testsuite/gdb.ada/limited-length/foo.adb
new file mode 100644
index 00000000000..85b421b0498
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/limited-length/foo.adb
@@ -0,0 +1,32 @@
+--  Copyright 2012-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/>.
+
+with Pck; use Pck;
+
+procedure Foo is
+   Large_Array :  array (1..4,1..4,1..4) of Integer;
+   Count : Integer := 1;
+begin
+   for i in 1 .. 4 loop
+     for j in 1 .. 4 loop
+        for k in 1 .. 4 loop
+           Large_Array (i,J,k) := Count;
+           Count := Count + 1;
+        end loop;
+     end loop;
+   end loop;
+   Do_Nothing (Large_Array'Address); -- STOP
+end Foo;
+
diff --git a/gdb/testsuite/gdb.ada/limited-length/pck.adb b/gdb/testsuite/gdb.ada/limited-length/pck.adb
new file mode 100644
index 00000000000..b3268565ec4
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/limited-length/pck.adb
@@ -0,0 +1,23 @@
+--  Copyright 2011-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/>.
+
+package body Pck is
+
+   procedure Do_Nothing (A : System.Address) is
+   begin
+      null;
+   end Do_Nothing;
+
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/limited-length/pck.ads b/gdb/testsuite/gdb.ada/limited-length/pck.ads
new file mode 100644
index 00000000000..419d45ffb09
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/limited-length/pck.ads
@@ -0,0 +1,19 @@
+--  Copyright 2011-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/>.
+
+with System;
+package Pck is
+   procedure Do_Nothing (A : System.Address);
+end Pck;
diff --git a/gdb/testsuite/gdb.base/limited-length.c b/gdb/testsuite/gdb.base/limited-length.c
new file mode 100644
index 00000000000..a51dab1dbe3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/limited-length.c
@@ -0,0 +1,31 @@
+int large_1d_array[] = {
+  0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
+  10, 11, 12, 13, 14, 15, 16, 17, 18, 19,
+  20, 21, 22, 23, 24, 25, 26, 27, 28, 29,
+  30, 31, 32, 33, 34, 35, 36, 37, 38, 39,
+  40, 41, 42, 43, 44, 45, 46, 47, 48, 49,
+  50, 51, 52, 53, 54, 55, 56, 57, 58, 59,
+  60, 61, 62, 63, 64, 65, 66, 67, 68, 69,
+  70, 71, 72, 73, 74, 75, 76, 77, 78, 79,
+  80, 81, 82, 83, 84, 85, 86, 87, 88, 89,
+  90, 91, 92, 93, 94, 95, 96, 97, 98, 99
+};
+
+int large_2d_array[][10] = {
+  {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
+  {10, 11, 12, 13, 14, 15, 16, 17, 18, 19},
+  {20, 21, 22, 23, 24, 25, 26, 27, 28, 29},
+  {30, 31, 32, 33, 34, 35, 36, 37, 38, 39},
+  {40, 41, 42, 43, 44, 45, 46, 47, 48, 49},
+  {50, 51, 52, 53, 54, 55, 56, 57, 58, 59},
+  {60, 61, 62, 63, 64, 65, 66, 67, 68, 69},
+  {70, 71, 72, 73, 74, 75, 76, 77, 78, 79},
+  {80, 81, 82, 83, 84, 85, 86, 87, 88, 89},
+  {90, 91, 92, 93, 94, 95, 96, 97, 98, 99}
+};
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/limited-length.exp b/gdb/testsuite/gdb.base/limited-length.exp
new file mode 100644
index 00000000000..8cb41fa7c1e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/limited-length.exp
@@ -0,0 +1,72 @@
+# Copyright 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/>.
+
+# Test GDB's limited array printing.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+with_test_prefix "with standard max-value size" {
+    gdb_test "print large_1d_array" \
+	" = \\{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, .*\\}"
+    gdb_test "print large_2d_array" \
+	" = \\{\\{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, \\{10, 11, 12, .*\\}\\}"
+}
+
+# Set the max-value-size so we can only print 51 elements.
+set int_size [get_valueof "/d" "sizeof(int)" "*unknown*"]
+gdb_test_no_output "set max-value-size [expr $int_size * 51]"
+
+with_test_prefix "with reduced max-value size" {
+    gdb_test "print large_1d_array" \
+	"\r\nvalue requires $decimal bytes, which is more than max-value-size"
+    gdb_test "print large_2d_array" \
+	"\r\nvalue requires $decimal bytes, which is more than max-value-size"
+}
+
+gdb_test_no_output "set print elements 3"
+
+with_test_prefix "with reduced print elements" {
+    # TODO: I find it weid that the syntax here is '0, 1, 2...' rather
+    # than '0, 1, 2, ...' which I think better represents that
+    # separate elements have been removed.
+    gdb_test "print large_1d_array" \
+	" = \\{0, 1, 2\\.\\.\\.\\}"
+
+    # TODO: Should we print something better than '<unavailable>'
+    # here?  This would require that values understand more about why
+    # they are not available.
+    gdb_test "print \$" " = <unavailable>" \
+	"print large_1d_array from history"
+
+    # TODO: I find this syntax really weird.  I guess I see that C/C++
+    # don't have true multi-dimensional arrays, but printing the first
+    # few elements from each sub-array was really not what I was
+    # expecting.
+    gdb_test "print large_2d_array" \
+	" = \\{\\{0, 1, 2\\.\\.\\.\\}, \\{10, 11, 12\\.\\.\\.\\}, \\{20, 21, 22\\.\\.\\.\\}\\.\\.\\.\\}"
+
+    # TODO: See previous '<unavailable>' test above.
+    gdb_test "print \$" " = <unavailable>" \
+	"print large_2d_array from history"
+}
diff --git a/gdb/testsuite/gdb.fortran/limited-length.exp b/gdb/testsuite/gdb.fortran/limited-length.exp
new file mode 100644
index 00000000000..3d328b54ca5
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/limited-length.exp
@@ -0,0 +1,66 @@
+# Copyright 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/>.
+
+# This file tests GDB's limited length array printing.
+
+load_lib "fortran.exp"
+
+if { [skip_fortran_tests] } { continue }
+
+standard_testfile .f90
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile {debug f90}] } {
+    return -1
+}
+
+if { ![fortran_runto_main] } {
+    perror "Could not run to main."
+    continue
+}
+
+gdb_breakpoint [gdb_get_line_number "Break Here"]
+gdb_continue_to_breakpoint "stop-here" ".*Break Here.*"
+
+with_test_prefix "with standard max-value size" {
+    gdb_test "print large_4d_array" \
+	" = \\(\\(\\(\\(1, 2, 3\\) \\(4, 5, 6\\) .* \\(76, 77, 78\\) \\(79, 80, 81\\)\\)\\)\\)"
+    gdb_test "print large_1d_array" \
+	" = \\(1, 2, 3, 4, 5, 6, 7, .* 76, 77, 78, 79, 80, 81\\)"
+}
+
+gdb_test_no_output "set max-value-size 400"
+
+with_test_prefix "with reduced max-value size" {
+    gdb_test "print large_4d_array" \
+	"value requires $decimal bytes, which is more than max-value-size"
+    gdb_test "print large_1d_array" \
+	"value requires $decimal bytes, which is more than max-value-size"
+}
+
+with_test_prefix "with reduced print -elements flag" {
+    gdb_test "print -elements 5 -- large_4d_array" \
+	" = \\(\\(\\(\\(1, 2, 3\\) \\(4, 5, \\.\\.\\.\\) \\.\\.\\.\\) \\.\\.\\.\\) \\.\\.\\.\\)"
+    gdb_test "print -elements 5 -- large_1d_array" \
+	" = \\(1, 2, 3, 4, 5, \\.\\.\\.\\)"
+}
+
+gdb_test_no_output "set print elements 5"
+
+with_test_prefix "with reduced print elements" {
+    gdb_test "print large_4d_array" \
+	" = \\(\\(\\(\\(1, 2, 3\\) \\(4, 5, \\.\\.\\.\\) \\.\\.\\.\\) \\.\\.\\.\\) \\.\\.\\.\\)"
+    gdb_test "print large_1d_array" \
+	" = \\(1, 2, 3, 4, 5, \\.\\.\\.\\)"
+}
diff --git a/gdb/testsuite/gdb.fortran/limited-length.f90 b/gdb/testsuite/gdb.fortran/limited-length.f90
new file mode 100644
index 00000000000..b784884c5db
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/limited-length.f90
@@ -0,0 +1,37 @@
+! Copyright 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 2 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 main
+  integer(kind=8), dimension (3, 3, 3, 3) :: large_4d_array = reshape ((/ &
+       1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, &
+       18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, &
+       33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, &
+       48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, &
+       63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, &
+       78, 79, 80, 81/), (/3, 3, 3, 3/))
+
+  integer(kind=8), dimension (81) :: large_1d_array = reshape ((/ &
+       1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, &
+       18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, &
+       33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, &
+       48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, &
+       63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, &
+       78, 79, 80, 81/), (/81/))
+
+  print *, ""
+  print *, ""	! Break Here
+  print *, large_4d_array
+  print *, large_1d_array
+end program main
diff --git a/gdb/value.c b/gdb/value.c
index 3a2bc13985e..43d9867fa7c 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -358,6 +358,14 @@ struct value
      treated pretty much the same, except not-saved registers have a
      different string representation and related error strings.  */
   std::vector<range> optimized_out;
+
+  /* This is only non-zero for values of TYPE_CODE_ARRAY and if the size of
+     the array in inferior memory is greater than max_value_size.  If these
+     conditions are met then, when the value is loaded from the inferior
+     GDB will only load a portion of the array into memory, and
+     limited_length will be set to indicate the length in octets that were
+     loaded from the inferior.  */
+  int limited_length = 0;
 };
 
 /* See value.h.  */
@@ -1012,6 +1020,70 @@ check_type_length_before_alloc (const struct type *type)
     }
 }
 
+/* When this has a value, it is used to limit the number of array elements
+   of an array that are loaded into memory when an array value is make
+   non-lazy.  */
+static gdb::optional<int> array_length_limiting_element_count;
+
+/* See value.h.  */
+scoped_array_length_limiting::scoped_array_length_limiting (int elements)
+{
+  m_old_value = array_length_limiting_element_count;
+  array_length_limiting_element_count.emplace (elements);
+}
+
+/* See value.h.  */
+scoped_array_length_limiting::~scoped_array_length_limiting ()
+{
+  array_length_limiting_element_count = m_old_value;
+}
+
+/* Find the inner element type for ARRAY_TYPE.  */
+
+static struct type *
+find_array_element_type (struct type *array_type)
+{
+  array_type = check_typedef (array_type);
+  gdb_assert (array_type->code () == TYPE_CODE_ARRAY);
+
+  if (current_language->la_language == language_fortran)
+    while (array_type->code () == TYPE_CODE_ARRAY)
+      {
+	array_type = TYPE_TARGET_TYPE (array_type);
+	array_type = check_typedef (array_type);
+      }
+  else
+    {
+      array_type = TYPE_TARGET_TYPE (array_type);
+      array_type = check_typedef (array_type);
+    }
+
+  return array_type;
+}
+
+/* Return the limited length of ARRAY_TYPE, which must be of
+   TYPE_CODE_ARRAY.  This function can only be called when the global
+   ARRAY_LENGTH_LIMITING_ELEMENT_COUNT has a value.
+
+   The limited length of an array is the smallest of either (1) the total
+   size of the array type, or (2) the array target type multiplies by the
+   array_length_limiting_element_count.  */
+static ULONGEST
+calculate_limited_array_length (struct type *array_type)
+{
+  gdb_assert (array_length_limiting_element_count.has_value ());
+
+  array_type = check_typedef (array_type);
+  gdb_assert (array_type->code () == TYPE_CODE_ARRAY);
+
+  struct type *elm_type = find_array_element_type (array_type);
+  ULONGEST len = (TYPE_LENGTH (elm_type)
+		  * (*array_length_limiting_element_count));
+  len = std::min (len, TYPE_LENGTH (array_type));
+
+  return len;
+}
+
 /* Allocate the contents of VAL if it has not been allocated yet.  */
 
 static void
@@ -1019,9 +1091,34 @@ allocate_value_contents (struct value *val)
 {
   if (!val->contents)
     {
-      check_type_length_before_alloc (val->enclosing_type);
-      val->contents.reset
-	((gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type)));
+      /* If we are allocating the contents of an array, which is greater in
+	 size than max_value_size, and there is an element limit in effect,
+	 then we can possibly try to load only a sub-set of the array
+	 contents into GDB's memory.  */
+      if (val->enclosing_type == val->type
+	  && val->type->code () == TYPE_CODE_ARRAY
+	  && TYPE_LENGTH (val->type) > max_value_size
+	  && array_length_limiting_element_count.has_value ())
+	{
+	  /* Try to limit ourselves to only fetching the limited number of
+	     elements.  However, if this limited number of elements still
+	     puts us over max_value_size, then we still throw an error.  */
+	  ULONGEST len = calculate_limited_array_length (val->type);
+	  if (len > max_value_size)
+	    check_type_length_before_alloc (val->type);
+
+	  /* Set the limited_length field of this value to indicate that
+	     the value contents buffer is smaller than the actual full type
+	     length of this value.  */
+	  val->limited_length = len;
+	  val->contents.reset ((gdb_byte *) xzalloc (val->limited_length));
+	}
+      else
+	{
+	  check_type_length_before_alloc (val->enclosing_type);
+	  val->contents.reset
+	    ((gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type)));
+	}
     }
 }
 
@@ -1678,10 +1775,7 @@ value_copy (struct value *arg)
   struct type *encl_type = value_enclosing_type (arg);
   struct value *val;
 
-  if (value_lazy (arg))
-    val = allocate_value_lazy (encl_type);
-  else
-    val = allocate_value (encl_type);
+  val = allocate_value_lazy (encl_type);
   val->type = arg->type;
   VALUE_LVAL (val) = VALUE_LVAL (arg);
   val->location = arg->location;
@@ -1694,15 +1788,37 @@ value_copy (struct value *arg)
   val->modifiable = arg->modifiable;
   val->stack = arg->stack;
   val->initialized = arg->initialized;
+  val->unavailable = arg->unavailable;
+  val->optimized_out = arg->optimized_out;
+  val->parent = arg->parent;
+  val->limited_length = arg->limited_length;
   if (!value_lazy (val))
     {
-      memcpy (value_contents_all_raw (val), value_contents_all_raw (arg),
-	      TYPE_LENGTH (value_enclosing_type (arg)));
+      if (!(value_entirely_optimized_out (val)
+	    || value_entirely_unavailable (val)))
+	{
+	  allocate_value_contents (val);
+	  /* We need to handle the case where the new VAL has its content
+	     buffer limited, but to a different length, than the original
+	     ARG value.  This all gets rather messy.  */
+	  int src_len, dst_len;
+	  if (val->limited_length > 0)
+	    dst_len = val->limited_length;
+	  else
+	    dst_len = TYPE_LENGTH (value_enclosing_type (val));
 
+	  if (arg->limited_length > 0)
+	    src_len = val->limited_length;
+	  else
+	    src_len = TYPE_LENGTH (value_enclosing_type (arg));
+
+	  int len = std::min (src_len, dst_len);
+	  memcpy (value_contents_all_raw (val), value_contents_all_raw (arg),
+		  len);
+	  if (dst_len < src_len)
+	    mark_value_bytes_optimized_out (val, dst_len, (src_len - dst_len));
+	}
     }
-  val->unavailable = arg->unavailable;
-  val->optimized_out = arg->optimized_out;
-  val->parent = arg->parent;
   if (VALUE_LVAL (val) == lval_computed)
     {
       const struct lval_funcs *funcs = val->location.computed.funcs;
@@ -1842,7 +1958,28 @@ record_latest_value (struct value *val)
      the value was taken, and fast watchpoints should be able to assume that
      a value on the value history never changes.  */
   if (value_lazy (val))
-    value_fetch_lazy (val);
+    {
+      if (value_type (val)->code () == TYPE_CODE_ARRAY
+	  && TYPE_LENGTH (value_enclosing_type (val)) > max_value_size
+	  && array_length_limiting_element_count.has_value ()
+	  && value_enclosing_type (val) == value_type (val)
+	  && (calculate_limited_array_length (value_type (val))
+	      <= max_value_size))
+	{
+	  /* We know that this is a _huge_ array, any attempt to fetch this
+	     is going to cause GDB to throw an error.  However, to allow
+	     the array to still be displayed we place an "unavailable"
+	     value into the history and skip fetching the value at this
+	     point.  */
+	  struct type *type = value_type (val);
+	  val = allocate_value_lazy (type);
+	  mark_value_bytes_unavailable (val, 0, TYPE_LENGTH (type));
+	  val->lazy = 0;
+	}
+      else
+	value_fetch_lazy (val);
+      }
+
   /* We preserve VALUE_LVAL so that the user can find out where it was fetched
      from.  This is a bit dubious, because then *&$1 does not just return $1
      but the current contents of that location.  c'est la vie...  */
@@ -3872,10 +4009,23 @@ value_fetch_lazy_memory (struct value *val)
   CORE_ADDR addr = value_address (val);
   struct type *type = check_typedef (value_enclosing_type (val));
 
-  if (TYPE_LENGTH (type))
-      read_value_memory (val, 0, value_stack (val),
-			 addr, value_contents_all_raw (val),
-			 type_length_units (type));
+  /* Figure out how much we should copy from memory.  Usually, this is just
+     the size of the type, but, for arrays, we might only be loading a
+     small part of the array (this is only done for very large arrays).  */
+  int len = 0;
+  if (val->limited_length > 0)
+    {
+      gdb_assert (val->type->code () == TYPE_CODE_ARRAY);
+      len = val->limited_length;
+    }
+  else if (TYPE_LENGTH (type) > 0)
+    len = type_length_units (type);
+
+  gdb_assert (len >= 0);
+
+  if (len > 0)
+    read_value_memory (val, 0, value_stack (val), addr,
+		       value_contents_all_raw (val), len);
 }
 
 /* Helper for value_fetch_lazy when the value is in a register.  */
diff --git a/gdb/value.h b/gdb/value.h
index 3f00444e7e3..81525c8d06f 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1214,4 +1214,21 @@ extern void finalize_values ();
    of floating-point, fixed-point, or integer type.  */
 extern gdb_mpq value_to_gdb_mpq (struct value *value);
 
+/* While an instance of this class is live, and array values that are
+   created, that are larger than max_value_size, will be restricted in size
+   to a particular number of elements.  */
+
+struct scoped_array_length_limiting
+{
+  /* Limit any large array values to only contain ELEMENTS elements.  */
+  scoped_array_length_limiting (int elements);
+
+  /* Restore the previous array value limit.  */
+  ~scoped_array_length_limiting ();
+
+private:
+  /* Used to hold the previous array value element limit.  */
+  gdb::optional<int> m_old_value;
+};
+
 #endif /* !defined (VALUE_H) */
-- 
2.25.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] gdb: introduce limited array lengths while printing values
  2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
@ 2021-10-06 17:36 ` Eli Zaretskii
  2021-11-02 10:03 ` [PING][RFC] " Maciej W. Rozycki
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2021-10-06 17:36 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Wed,  6 Oct 2021 18:29:02 +0100
> 
>  gdb/NEWS                                     |   6 +
>  gdb/doc/gdb.texinfo                          |   9 +
>  gdb/printcmd.c                               |   5 +
>  gdb/testsuite/gdb.ada/limited-length.exp     |  51 +++++
>  gdb/testsuite/gdb.ada/limited-length/foo.adb |  32 ++++
>  gdb/testsuite/gdb.ada/limited-length/pck.adb |  23 +++
>  gdb/testsuite/gdb.ada/limited-length/pck.ads |  19 ++
>  gdb/testsuite/gdb.base/limited-length.c      |  31 ++++
>  gdb/testsuite/gdb.base/limited-length.exp    |  72 ++++++++
>  gdb/testsuite/gdb.fortran/limited-length.exp |  66 +++++++
>  gdb/testsuite/gdb.fortran/limited-length.f90 |  37 ++++
>  gdb/value.c                                  | 184 +++++++++++++++++--
>  gdb/value.h                                  |  17 ++
>  13 files changed, 535 insertions(+), 17 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.ada/limited-length.exp
>  create mode 100644 gdb/testsuite/gdb.ada/limited-length/foo.adb
>  create mode 100644 gdb/testsuite/gdb.ada/limited-length/pck.adb
>  create mode 100644 gdb/testsuite/gdb.ada/limited-length/pck.ads
>  create mode 100644 gdb/testsuite/gdb.base/limited-length.c
>  create mode 100644 gdb/testsuite/gdb.base/limited-length.exp
>  create mode 100644 gdb/testsuite/gdb.fortran/limited-length.exp
>  create mode 100644 gdb/testsuite/gdb.fortran/limited-length.f90

OK for the documentation parts.

Thanks.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PING][RFC] gdb: introduce limited array lengths while printing values
  2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
  2021-10-06 17:36 ` Eli Zaretskii
@ 2021-11-02 10:03 ` Maciej W. Rozycki
  2021-11-09 16:44 ` [PING^2][RFC] " Maciej W. Rozycki
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2021-11-02 10:03 UTC (permalink / raw)
  To: gdb-patches

On Wed, 6 Oct 2021, Andrew Burgess wrote:

> This commit introduces the idea of loading only part of an array in
> order to print it, what I call "limited length" arrays.

 Ping for: 
<https://sourceware.org/pipermail/gdb-patches/2021-October/182435.html>.

 NB since Andrew has left it's now me who'll be handling this submission.

  Maciej

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PING^2][RFC] gdb: introduce limited array lengths while printing values
  2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
  2021-10-06 17:36 ` Eli Zaretskii
  2021-11-02 10:03 ` [PING][RFC] " Maciej W. Rozycki
@ 2021-11-09 16:44 ` Maciej W. Rozycki
  2021-11-16 12:40 ` [PING^3][RFC] " Maciej W. Rozycki
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2021-11-09 16:44 UTC (permalink / raw)
  To: gdb-patches

On Wed, 6 Oct 2021, Andrew Burgess wrote:

> This commit introduces the idea of loading only part of an array in
> order to print it, what I call "limited length" arrays.

 Ping for:
<https://sourceware.org/pipermail/gdb-patches/2021-October/182435.html>.

  Maciej

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PING^3][RFC] gdb: introduce limited array lengths while printing values
  2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
                   ` (2 preceding siblings ...)
  2021-11-09 16:44 ` [PING^2][RFC] " Maciej W. Rozycki
@ 2021-11-16 12:40 ` Maciej W. Rozycki
  2021-11-23 21:37 ` [PING^4][RFC] " Maciej W. Rozycki
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2021-11-16 12:40 UTC (permalink / raw)
  To: gdb-patches

On Wed, 6 Oct 2021, Andrew Burgess wrote:

> This commit introduces the idea of loading only part of an array in
> order to print it, what I call "limited length" arrays.

 Ping for:
<https://sourceware.org/pipermail/gdb-patches/2021-October/182435.html>.

  Maciej

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PING^4][RFC] gdb: introduce limited array lengths while printing values
  2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
                   ` (3 preceding siblings ...)
  2021-11-16 12:40 ` [PING^3][RFC] " Maciej W. Rozycki
@ 2021-11-23 21:37 ` Maciej W. Rozycki
  2021-11-30 13:14 ` [PING^5][RFC] " Maciej W. Rozycki
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2021-11-23 21:37 UTC (permalink / raw)
  To: gdb-patches

On Wed, 6 Oct 2021, Andrew Burgess wrote:

> This commit introduces the idea of loading only part of an array in
> order to print it, what I call "limited length" arrays.

 Ping for:
<https://sourceware.org/pipermail/gdb-patches/2021-October/182435.html>.

  Maciej

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PING^5][RFC] gdb: introduce limited array lengths while printing values
  2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
                   ` (4 preceding siblings ...)
  2021-11-23 21:37 ` [PING^4][RFC] " Maciej W. Rozycki
@ 2021-11-30 13:14 ` Maciej W. Rozycki
  2021-12-08 23:19 ` [PING^6][RFC] " Maciej W. Rozycki
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2021-11-30 13:14 UTC (permalink / raw)
  To: gdb-patches

On Wed, 6 Oct 2021, Andrew Burgess wrote:

> This commit introduces the idea of loading only part of an array in
> order to print it, what I call "limited length" arrays.

 Ping for:
<https://sourceware.org/pipermail/gdb-patches/2021-October/182435.html>.

  Maciej

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PING^6][RFC] gdb: introduce limited array lengths while printing values
  2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
                   ` (5 preceding siblings ...)
  2021-11-30 13:14 ` [PING^5][RFC] " Maciej W. Rozycki
@ 2021-12-08 23:19 ` Maciej W. Rozycki
  2021-12-15 17:14 ` [PING^7][RFC] " Maciej W. Rozycki
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2021-12-08 23:19 UTC (permalink / raw)
  To: gdb-patches

On Wed, 6 Oct 2021, Andrew Burgess wrote:

> This commit introduces the idea of loading only part of an array in
> order to print it, what I call "limited length" arrays.

 Ping for:
<https://sourceware.org/pipermail/gdb-patches/2021-October/182435.html>.

  Maciej

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PING^7][RFC] gdb: introduce limited array lengths while printing values
  2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
                   ` (6 preceding siblings ...)
  2021-12-08 23:19 ` [PING^6][RFC] " Maciej W. Rozycki
@ 2021-12-15 17:14 ` Maciej W. Rozycki
  2022-01-04 17:50 ` [PING^8][RFC] " Maciej W. Rozycki
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2021-12-15 17:14 UTC (permalink / raw)
  To: gdb-patches

On Wed, 6 Oct 2021, Andrew Burgess wrote:

> This commit introduces the idea of loading only part of an array in
> order to print it, what I call "limited length" arrays.

 Ping for:
<https://sourceware.org/pipermail/gdb-patches/2021-October/182435.html>.

  Maciej

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PING^8][RFC] gdb: introduce limited array lengths while printing values
  2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
                   ` (7 preceding siblings ...)
  2021-12-15 17:14 ` [PING^7][RFC] " Maciej W. Rozycki
@ 2022-01-04 17:50 ` Maciej W. Rozycki
  2022-01-12 18:17 ` [PING^9][RFC] " Maciej W. Rozycki
  2022-01-14 17:45 ` [RFC] " Tom Tromey
  10 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2022-01-04 17:50 UTC (permalink / raw)
  To: gdb-patches

On Wed, 6 Oct 2021, Andrew Burgess wrote:

> This commit introduces the idea of loading only part of an array in
> order to print it, what I call "limited length" arrays.

 Ping for:
<https://sourceware.org/pipermail/gdb-patches/2021-October/182435.html>.

  Maciej

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PING^9][RFC] gdb: introduce limited array lengths while printing values
  2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
                   ` (8 preceding siblings ...)
  2022-01-04 17:50 ` [PING^8][RFC] " Maciej W. Rozycki
@ 2022-01-12 18:17 ` Maciej W. Rozycki
  2022-01-14 17:45 ` [RFC] " Tom Tromey
  10 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2022-01-12 18:17 UTC (permalink / raw)
  To: gdb-patches

On Wed, 6 Oct 2021, Andrew Burgess wrote:

> This commit introduces the idea of loading only part of an array in
> order to print it, what I call "limited length" arrays.

 Ping for:
<https://sourceware.org/pipermail/gdb-patches/2021-October/182435.html>.

  Maciej

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] gdb: introduce limited array lengths while printing values
  2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
                   ` (9 preceding siblings ...)
  2022-01-12 18:17 ` [PING^9][RFC] " Maciej W. Rozycki
@ 2022-01-14 17:45 ` Tom Tromey
  2023-01-12  9:00   ` Maciej W. Rozycki
  10 siblings, 1 reply; 13+ messages in thread
From: Tom Tromey @ 2022-01-14 17:45 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> This commit introduces the idea of loading only part of an array in
Andrew> order to print it, what I call "limited length" arrays.

Andrew> The motivation behind this work is to make it possible to print slices
Andrew> of very large arrays, where very large means bigger than
Andrew> max-value-size.

Seems reasonable.  My first thought was why doesn't the user just bump
up max-value-size, but I suppose one can always find an even bigger
array.

Andrew>   (gdb) p $1
Andrew>   $2 = <unavailable>

Andrew> This patch is currently RFC, I would like to hear what people think
Andrew> about both the idea in general, and the approach taken.

I think this detail is the crucial point.

Andrew> One question I have is whether the value history problem would be
Andrew> better addressed in a different way, for example, we could just drop
Andrew> the '$1 = ' for values that are not being added into the history, so
Andrew> things would look like:

Andrew>   (gdb) p -elements 10 -- large_1d_array
Andrew>   {0, 1, 2, 3, 4, 5, 6, 7, 8, 9...}

Andrew> which might be a better solution.

I'm not a fan of this one.  It seems overly subtle to me.

Andrew> Another possibility would be to tag
Andrew> the "unavailable" value with a reason field so we could do something
Andrew> like:

Andrew>   (gdb) p $1
Andrew>   $2 = <unavailable: original value was too large>

Andrew> which is slightly more informative, but clearly is a more invasive
Andrew> change to the value structure.

How much more invasive?

Also, it seems to me that if the new print request would show elements
that do exist, then the <unavailable> could be avoided.  That is, if the
number requested from history is less than or equal to what was done
before, just satisfy the request.

Andrew> But I think Tom was looking into having the value optimized out checks
Andrew> not actually load the value in at one point, so maybe, if that change
Andrew> landed, then we could investigate the possibility of having the array
Andrew> printing code only load the elements from the target one at a
Andrew> time (with the dcache providing some optimisation), which might avoid
Andrew> the need to perform the current partial load?

This did land:

    commit a519e8ffe2b0f008deaef1517562090d9eaadccc
    Author: Tom Tromey <tromey@adacore.com>
    Date:   Fri Sep 10 12:40:54 2021 -0600

        Add lval_funcs::is_optimized_out

Andrew> Anyway, I'd be interested to hear people's thoughts; is this a
Andrew> valuable change?  Which approach seems like the right way to go?

I think it makes sense to allow something here.

Another possibility is that when printing a length-limited array, just
make a new array type with the requested bounds.  Then it will fit
automatically and work "properly" in history.  The downside is this may
be confusing to users.  On the whole I think I'd prefer some kind of
unavailability message when a request can't be satisfied.  Though
perhaps one way to do this would be to make an array type, but also mark
the value as "this has a synthetic type", so that the value code can
know when to emit the message.

Tom

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] gdb: introduce limited array lengths while printing values
  2022-01-14 17:45 ` [RFC] " Tom Tromey
@ 2023-01-12  9:00   ` Maciej W. Rozycki
  0 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2023-01-12  9:00 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Andrew Burgess, gdb-patches

Hi Tom,

 It looks like it comes at almost an anniversary.

On Fri, 14 Jan 2022, Tom Tromey wrote:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> This commit introduces the idea of loading only part of an array in
> Andrew> order to print it, what I call "limited length" arrays.
> 
> Andrew> The motivation behind this work is to make it possible to print slices
> Andrew> of very large arrays, where very large means bigger than
> Andrew> max-value-size.
> 
> Seems reasonable.  My first thought was why doesn't the user just bump
> up max-value-size, but I suppose one can always find an even bigger
> array.

 My understanding of the motivation for this feature has been this is for 
data objects so large as to put a serious strain on the resources of the 
debugger if retrieved whole, so just bumping up `max-value-size' would be 
defeating the purpose the setting is there in the first place.

> Andrew>   (gdb) p $1
> Andrew>   $2 = <unavailable>
> 
> Andrew> This patch is currently RFC, I would like to hear what people think
> Andrew> about both the idea in general, and the approach taken.
> 
> I think this detail is the crucial point.
> 
> Andrew> One question I have is whether the value history problem would be
> Andrew> better addressed in a different way, for example, we could just drop
> Andrew> the '$1 = ' for values that are not being added into the history, so
> Andrew> things would look like:
> 
> Andrew>   (gdb) p -elements 10 -- large_1d_array
> Andrew>   {0, 1, 2, 3, 4, 5, 6, 7, 8, 9...}
> 
> Andrew> which might be a better solution.
> 
> I'm not a fan of this one.  It seems overly subtle to me.
> 
> Andrew> Another possibility would be to tag
> Andrew> the "unavailable" value with a reason field so we could do something
> Andrew> like:
> 
> Andrew>   (gdb) p $1
> Andrew>   $2 = <unavailable: original value was too large>
> 
> Andrew> which is slightly more informative, but clearly is a more invasive
> Andrew> change to the value structure.
> 
> How much more invasive?
> 
> Also, it seems to me that if the new print request would show elements
> that do exist, then the <unavailable> could be avoided.  That is, if the
> number requested from history is less than or equal to what was done
> before, just satisfy the request.
> 
> Andrew> But I think Tom was looking into having the value optimized out checks
> Andrew> not actually load the value in at one point, so maybe, if that change
> Andrew> landed, then we could investigate the possibility of having the array
> Andrew> printing code only load the elements from the target one at a
> Andrew> time (with the dcache providing some optimisation), which might avoid
> Andrew> the need to perform the current partial load?
> 
> This did land:
> 
>     commit a519e8ffe2b0f008deaef1517562090d9eaadccc
>     Author: Tom Tromey <tromey@adacore.com>
>     Date:   Fri Sep 10 12:40:54 2021 -0600
> 
>         Add lval_funcs::is_optimized_out
> 
> Andrew> Anyway, I'd be interested to hear people's thoughts; is this a
> Andrew> valuable change?  Which approach seems like the right way to go?
> 
> I think it makes sense to allow something here.
> 
> Another possibility is that when printing a length-limited array, just
> make a new array type with the requested bounds.  Then it will fit
> automatically and work "properly" in history.  The downside is this may
> be confusing to users.  On the whole I think I'd prefer some kind of
> unavailability message when a request can't be satisfied.  Though
> perhaps one way to do this would be to make an array type, but also mark
> the value as "this has a synthetic type", so that the value code can
> know when to emit the message.

 I have come up with yet another approach, which will hopefully make 
everyone happy.  For limited-length requests data up to `max-value-size' 
is retrieved and stored in the history regardless of how much less has 
been requested with the relevant `print' command.

 If history is later accessed, then elements beyond the `max-value-size' 
limit at the time the entry was created are shown as <unavailable>, while 
these within are returned normally whether for printing or for expression 
evaluation.  I think this is consistent and what one might expect.

 This required some preparatory changes to how history is handled to avoid 
surprising effects, so I have turned Andrew's original change into a small 
patch series.  Submitted shortly as v2 and not an RFC anymore.

 Thank you for your review.

  Maciej

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-01-12  9:00 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-06 17:29 [RFC] gdb: introduce limited array lengths while printing values Andrew Burgess
2021-10-06 17:36 ` Eli Zaretskii
2021-11-02 10:03 ` [PING][RFC] " Maciej W. Rozycki
2021-11-09 16:44 ` [PING^2][RFC] " Maciej W. Rozycki
2021-11-16 12:40 ` [PING^3][RFC] " Maciej W. Rozycki
2021-11-23 21:37 ` [PING^4][RFC] " Maciej W. Rozycki
2021-11-30 13:14 ` [PING^5][RFC] " Maciej W. Rozycki
2021-12-08 23:19 ` [PING^6][RFC] " Maciej W. Rozycki
2021-12-15 17:14 ` [PING^7][RFC] " Maciej W. Rozycki
2022-01-04 17:50 ` [PING^8][RFC] " Maciej W. Rozycki
2022-01-12 18:17 ` [PING^9][RFC] " Maciej W. Rozycki
2022-01-14 17:45 ` [RFC] " Tom Tromey
2023-01-12  9:00   ` Maciej W. Rozycki

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).