[PATCH 3/3] gdb: Guard against undefined behaviour in mi-vla-fortran.exp

[PATCH 3/3] gdb: Guard against undefined behaviour in mi-vla-fortran.exp

[Andrew Burgess]

The test gdb.mi/mi-vla-fortran.exp reveals an issue with the DWARF
generated by gfortran.

In the test a pointer variable 'pvla2' is created:
    real, pointer :: pvla2 (:, :)

Initially this variable will be unassociated, so something like this:
    l = associated(pvla2)

should return false.

In the test gdb stops at a point _before_ pvla2 is associated with
anything, and we then try to print pvla2, the expectation is that gdb
should reply <not associated>.

The problem is that the data the DWARF directs gdb to read (to identify
if the variable is associated or not) is not initialised until the first
time pvla2 is accessed.

As a result gdb ends up reading uninitialised memory, sometimes this
uninitialised memory indicates the variable is associated (when it's
not).  This first mistake can lead to a cascade of errors, reading
uninitialised memory, with the result that gdb builds an invalid type to
associate with the variable pvla2.

In some cases, this invalid type can be very large, which when we try to
print pvla2 causes gdb to allocate a large amount of memory.

A recent commit has added 'set max-value-size' to the gdb testsuite
start up code, this saves us in some regard, directly trying to print
pvla2 will now now error rather than allocate a large amount of memory.

However, some of the later tests create a varobj for pvla2, and then
ask for the children of that varobj to be displayed.  In the case where
an invalid type has been computed for pvla2 then the number of children
can be wrong, and very big, in which case trying to display all of these
children can cause gdb to consume an excessive amount of memory.

This commit first detects if printing pvla2 triggers the max-value-size
error, if it does then we avoid all the follow on tests relating to the
unassociated pvla2, which avoids the second error printing the varobj
children.

gdb/testsuite/ChangeLog:

	* gdb.mi/mi-vla-fortran.exp: Add XFAIL for accessing unassociated
	pointer.  Don't perform further tests on the unassociated pointer
	if the first test fails.
---
 gdb/testsuite/ChangeLog                 |  6 +++++
 gdb/testsuite/gdb.mi/mi-vla-fortran.exp | 48 ++++++++++++++++++++++-----------
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0673d01..429e98d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,11 @@
 2015-12-11  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb.mi/mi-vla-fortran.exp: Add XFAIL for accessing unassociated
+	pointer.  Don't perform further tests on the unassociated pointer
+	if the first test fails.
+
+2015-12-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* lib/gdb.exp (default_gdb_start): Set max-value-size.
 	* lib/mi-support.exp (default_mi_gdb_start): Likewise.
 	* gdb.base/max-value-size.exp: Don't check the initial value.
diff --git a/gdb/testsuite/gdb.mi/mi-vla-fortran.exp b/gdb/testsuite/gdb.mi/mi-vla-fortran.exp
index 8902ecb..ab697d8 100644
--- a/gdb/testsuite/gdb.mi/mi-vla-fortran.exp
+++ b/gdb/testsuite/gdb.mi/mi-vla-fortran.exp
@@ -128,24 +128,40 @@ mi_create_breakpoint "-t vla.f90:$bp_lineno" 6 "del" "vla" ".*vla.f90" \
 mi_run_cmd
 mi_expect_stop "breakpoint-hit" "vla" "" ".*vla.f90" "$bp_lineno" \
   { "" "disp=\"del\"" } "run to breakpoint at line $bp_lineno"
-mi_gdb_test "580-data-evaluate-expression pvla2" \
-  "580\\^done,value=\"<not associated>\"" "evaluate not associated vla"
-
-mi_create_varobj_checked pvla2_not_associated pvla2 "<not associated>" \
-  "create local variable pvla2_not_associated"
-mi_gdb_test "581-var-info-type pvla2_not_associated" \
-  "581\\^done,type=\"<not associated>\"" \
-  "info type variable pvla2_not_associated"
-mi_gdb_test "582-var-show-format pvla2_not_associated" \
-  "582\\^done,format=\"natural\"" \
-  "show format variable pvla2_not_associated"
-mi_gdb_test "583-var-evaluate-expression pvla2_not_associated" \
-  "583\\^done,value=\"\\\[0\\\]\"" \
-  "eval variable pvla2_not_associated"
-mi_list_array_varobj_children_with_index "pvla2_not_associated" "0" "1" \
-    "real\\\(kind=4\\\)" "get children of pvla2_not_associated"
 
 
+set test "evaluate not associated vla"
+send_gdb "580-data-evaluate-expression pvla2\n"
+gdb_expect {
+    -re "580\\^done,value=\"<not associated>\".*${mi_gdb_prompt}$" {
+	pass $test
+
+	mi_create_varobj_checked pvla2_not_associated pvla2 "<not associated>" \
+	    "create local variable pvla2_not_associated"
+	mi_gdb_test "581-var-info-type pvla2_not_associated" \
+	    "581\\^done,type=\"<not associated>\"" \
+	    "info type variable pvla2_not_associated"
+	mi_gdb_test "582-var-show-format pvla2_not_associated" \
+	    "582\\^done,format=\"natural\"" \
+	    "show format variable pvla2_not_associated"
+	mi_gdb_test "583-var-evaluate-expression pvla2_not_associated" \
+	    "583\\^done,value=\"\\\[0\\\]\"" \
+	    "eval variable pvla2_not_associated"
+	mi_list_array_varobj_children_with_index "pvla2_not_associated" "0" "1" \
+	    "real\\\(kind=4\\\)" "get children of pvla2_not_associated"
+    }
+    -re "580\\^error,msg=\"value contents too large \\(\[0-9\]+ bytes\\).*${mi_gdb_prompt}$" {
+	# Undefined behaviour in gfortran.
+	xfail $test
+    }
+    -re "${mi_gdb_prompt}$" {
+	fail $test
+    }
+    timeout {
+	fail "$test (timeout)"
+    }
+}
+
 set bp_lineno [gdb_get_line_number "pvla2-associated"]
 mi_create_breakpoint "-t vla.f90:$bp_lineno" 7 "del" "vla" ".*vla.f90" \
   $bp_lineno $hex "insert breakpoint at line $bp_lineno"
-- 
2.5.1

[PATCH 0/2] Problems with gdb.mi/mi-vla-fortran.exp

[Andrew Burgess]

I recently ran into an issue while running the gdb regression tests,
the symptom was that one of the tests was causing gdb to consume a
large amount of memory, 3G+.

I tracked this down to the test gdb.mi/mi-vla-fortran.exp.  The
specific causes of the issue appears to be a bug in gfortran.

In the test we have gdb stop at vla.f90:35, which looks like this:

    pvla2 => vla2               ! pvla2-not-associated

Before this point, the pvla2 pointer variable is not associated, and
so, something like:

    l = associated(pvla2)

Would return false.  In order to determine if pvla2 is associated gdb
makes use of the DWARF DW_AT_associated property which is an attribute
of the DWARF type corresponding to pvla2.

The DW_AT_associated property for the type of pvla2 is this (newlines
added by me for clarity):

    DW_AT_associated  : 4 byte block: 97 6 30 2e        \
                                (DW_OP_push_object_address; \
                                 DW_OP_deref; DW_OP_lit0; \
                                 DW_OP_ne)

So, take the address of the object (in this case the address is on the
stack), and use that to load an address sized block from memory, and
if that loaded block is not zero, then the the variable (pvla2) is
associated.

The problem is that gfortran does not initialise the block of memory
that the object address points to until the first time that pvla2 is
accessed.  Before that time the contents of the memory are undefined,
and if gdb accesses them we will see undefined behaviour.

I've confirmed that above behaviour by using the '-fdump-tree-all'
option to gfortran, then examining the pseudo-C code that is dumped.

From a compiler / code correctness point of view this behaviour is
perfectly understandable, and for performance, this is probably a win,
the problem (as I see it) is that a promise has been made in the
DWARF, which is not being honoured in the code.  I think gfortran
either needs more complex DWARF, or to initialise the memory block for
associatable variables earlier on.

Given the above mistake, what follows is just undefined behaviour
kicking in; gdb reads the uninitialised memory, which happens to be
non-zero, so it believes pvla2 is associated, it then tries to figure
out what it's associated with, and reads yet more uninitialised memory.

In this end it figures that pvla2 is pointing at a huge 3G+ array,
which in the gdb.mi/mi-vla-fortran.exp test we gdb to print.  This
causes gdb to allocate the 3G+ of memory to hold the contents of the
value.

My question is what to do next from the gdb side of things?

We could leave the test unchanged, arguing that gdb is asking sensible
questions, and it's gfortran that's getting things wrong.

Or we could remove, or comment out, the offending test.

Leaving the test in place unchanged would be a bit of a shame, I know
that 3G of memory isn't much on a lot of machines, but it's still
making my experience running the testsuite pretty painful; I can't be
the only one.

I think that this issue has highlighted a bigger issue that effects
gdb, when dealing with languages that support dynamic types, like
fortran.  An incorrect program can result in gdb having invalid type
information, it would be nice if this invalid information did not
cause gdb to do something so obviously wrong that it brings the
maching running gdb down.

The following patches are my attempt to address this issue.  The first
adds a mechansim to stop gdb allocating overly large arrays for value
contents, the second enables this mechanism withn the test suite,
while the third makes additional changes to the mi-vla-fortran.exp
test to prevent additional errors that are a consequence of the above
issue.

Thoughts and comments welcome.

Thanks,
Andrew


---

Andrew Burgess (3):
  gdb: New set/show max-value-size command.
  gdb: Set max-value-size before running tests.
  gdb: Guard against undefined behaviour in mi-vla-fortran.exp

 gdb/ChangeLog                             | 11 +++++
 gdb/NEWS                                  |  6 +++
 gdb/doc/ChangeLog                         |  4 ++
 gdb/doc/gdb.texinfo                       | 35 +++++++++++++++
 gdb/testsuite/ChangeLog                   | 17 ++++++++
 gdb/testsuite/gdb.base/max-value-size.c   | 26 +++++++++++
 gdb/testsuite/gdb.base/max-value-size.exp | 66 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-vla-fortran.exp   | 48 ++++++++++++++-------
 gdb/testsuite/lib/gdb.exp                 | 10 +++++
 gdb/testsuite/lib/mi-support.exp          | 10 +++++
 gdb/value.c                               | 71 +++++++++++++++++++++++++++++--
 11 files changed, 284 insertions(+), 20 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/max-value-size.c
 create mode 100644 gdb/testsuite/gdb.base/max-value-size.exp

-- 
2.5.1

[PATCH 2/3] gdb: Set max-value-size before running tests.

[Andrew Burgess]

Setting the max-value-size before running any tests should prevent any
issues where a failing test causes gdb to bring down the testing
machine.

The maximum size is set to 1G, this is large enough to handle all the
well behaving tests, and should make no difference to the test results.

There is one test, gdb.mi/mi-vla-fortran.exp that contains undefined
behaviour, one some machines this test is known to trigger the
max-value-size error.  However, on those machines the test would have
failed anyway, so this commit does not change the PASS / FAIL nature of
the test.  A later commit should modify the mi-vla-fortran.exp test to
expect the failure case.

gdb/testsuite/ChangeLog:

	* lib/gdb.exp (default_gdb_start): Set max-value-size.
	* lib/mi-support.exp (default_mi_gdb_start): Likewise.
	* gdb.base/max-value-size.exp: Don't check the initial value.
---
 gdb/testsuite/ChangeLog                   |  6 ++++++
 gdb/testsuite/gdb.base/max-value-size.exp |  8 +++-----
 gdb/testsuite/lib/gdb.exp                 | 10 ++++++++++
 gdb/testsuite/lib/mi-support.exp          | 10 ++++++++++
 4 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 93bb3bb..0673d01 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,11 @@
 2015-12-11  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* lib/gdb.exp (default_gdb_start): Set max-value-size.
+	* lib/mi-support.exp (default_mi_gdb_start): Likewise.
+	* gdb.base/max-value-size.exp: Don't check the initial value.
+
+2015-12-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* gdb.base/max-value-size.c: New file.
 	* gdb.base/max-value-size.exp: New file.
 
diff --git a/gdb/testsuite/gdb.base/max-value-size.exp b/gdb/testsuite/gdb.base/max-value-size.exp
index cb09ad8..21f4552 100644
--- a/gdb/testsuite/gdb.base/max-value-size.exp
+++ b/gdb/testsuite/gdb.base/max-value-size.exp
@@ -25,11 +25,9 @@ if ![runto_main] then {
     return 0
 }
 
-gdb_test "show max-value-size" \
-    "Maximum value size is unlimited." \
-    "the initial value of max-value-size is unlimited"
-
-with_test_prefix "max-value-size is 'unlimited'" {
+# The testing infrastructure does set the max-value-size, however, it
+# should always be large enough for these values.
+with_test_prefix "using initial max-value-size" {
     gdb_test "p/d one" " = 0"
     gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
 }
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index beb97ea..215feba 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1631,6 +1631,16 @@ proc default_gdb_start { } {
 	    warning "Couldn't set the width to 0."
 	}
     }
+    # set maximum value size to 1G, no tests currently require more.
+    send_gdb "set max-value-size 1024 * 1024 * 1024\n"
+    gdb_expect 10 {
+	-re "$gdb_prompt $" {
+	    verbose "Setting max-value-size to 1G" 2
+	}
+	timeout {
+	    warning "Couldn't set the max-value-size to 1G."
+	}
+    }
     return 0
 }
 
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 9619fb3..ab64366 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -204,6 +204,16 @@ proc default_mi_gdb_start { args } {
 	    warning "Couldn't set the width to 0."
 	}
     }
+    # set maximum value size to 1G, no tests currently require more.
+    send_gdb "102-gdb-set max-value-size 1024 * 1024 * 1024\n"
+    gdb_expect 10 {
+	-re ".*102-gdb-set max-value-size \[^\r\n\]+\r\n102\\\^done\r\n$mi_gdb_prompt$" {
+	    verbose "Setting max-value-size to 1G" 2
+	}
+	timeout {
+	    warning "Couldn't set the max-value-size to 1G."
+	}
+    }
 
     # Create the new PTY for the inferior process.
     if { $separate_inferior_pty } {
-- 
2.5.1

[PATCH 1/3] gdb: New set/show max-value-size command.

[Andrew Burgess]

For languages with dynamic types, an incorrect program could result in
an incorrect, overly large type being associated with a value.
Currently, attempting to print such a variable will result in gdb
attempting to allocate an overly large buffer.

This could result in the allocation failing, and gdb terminating, or the
buffer might be allocated, but the machine on which gdb is being run
might struggle due to the high memory requirement of gdb.

A new user visible variable in gdb helps guard against such problems,
two new commands are available:

   set max-value-size
   show max-value-size

The 'max-value-size' is the maximum size in bytes that the contents of a
value may allocate.  Any attempt to allocate a value with size greater
than this will result in an error.  The default for this variable is
currently unlimited.

Setting the default to unlimited reduces the use of this variable a
little bit, if a user hits one of these rogue types without realising
it, then gdb will still allocate the large buffer, with all the problems
that this entails.  The user will then be forced to probably restart
their gdb session, and then set the 'max-value-size' variable to guard
against future crashes.

However, it's not clear what a good default would actually be, given the
huge range of resources that are available on different machines.  One
solution might be to introduce platform specific code that attempts to
figure out what memory resources are available on the machine running
gdb, we could then set the max-value-size to some percentage of the
available memory, while still defaulting to unlimited for those machines
where we can't figure out a good alternative.  Such a feature is not
implemented in this commit, but could be added later.

gdb/ChangeLog:

	* value.c (max_value_size): New variable.
	(show_max_value_size): New function.
	(check_type_length_before_alloc): New function.
	(allocate_value_contents): Call check_type_length_before_alloc.
	(set_value_enclosing_type): Likewise.
	(_initialize_values): Add set/show handler for max-value-size.
	* NEWS: Mention new set/show command.

gdb/doc/ChangeLog:

	* gdb.texinfo (Value Sizes): New section.

gdb/testsuite/ChangeLog:

	* gdb.base/max-value-size.c: New file.
	* gdb.base/max-value-size.exp: New file.
---
 gdb/ChangeLog                             | 11 +++++
 gdb/NEWS                                  |  6 +++
 gdb/doc/ChangeLog                         |  4 ++
 gdb/doc/gdb.texinfo                       | 35 +++++++++++++++
 gdb/testsuite/ChangeLog                   |  5 +++
 gdb/testsuite/gdb.base/max-value-size.c   | 26 +++++++++++
 gdb/testsuite/gdb.base/max-value-size.exp | 68 +++++++++++++++++++++++++++++
 gdb/value.c                               | 71 +++++++++++++++++++++++++++++--
 8 files changed, 222 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/max-value-size.c
 create mode 100644 gdb/testsuite/gdb.base/max-value-size.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 03ae010..8e36907 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2015-12-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* value.c (max_value_size): New variable.
+	(set_max_value_size): New function.
+	(show_max_value_size): New function.
+	(check_type_length_before_alloc): New function.
+	(allocate_value_contents): Call check_type_length_before_alloc.
+	(set_value_enclosing_type): Likewise.
+	(_initialize_values): Add set/show handler for max-value-size.
+	* NEWS: Mention new set/show command.
+
 2015-12-10  Antoine Tremblay  <antoine.tremblay@ericsson.com>
 
 	* linux-thread-db.c (find_new_threads_callback): Use record_thread.
diff --git a/gdb/NEWS b/gdb/NEWS
index 9ca7f49..2fb264a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -60,6 +60,12 @@ show ada print-signatures"
   Control whether parameter types and return types are displayed in overloads
   selection menus.  It is activaled (@code{on}) by default.
 
+set max-value-size
+show max-value-size
+  Control the maximum size, in bytes, that GDB will allocate for value
+  contents.  Prevent incorrect programs from causing GDB to allocate
+  overly large buffers.  Default is unlimited.
+
 * The "disassemble" command accepts a new modifier: /s.
   It prints mixed source+disassembly like /m with two differences:
   - disassembled instructions are now printed in program order, and
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index fc81d09..796e2cd 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2015-12-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.texinfo (Value Sizes): New section.
+
 2015-12-10  Pedro Alves  <palves@redhat.com>
 
 	* gdb.texinfo (Threads): Replace warning with explanation
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bb68e21..5d8b579 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11592,6 +11592,41 @@ $1 = 1
 $2 = (void *) 0x8049560
 @end smallexample
 
+@node Value Sizes
+@section Value Sizes
+
+Whenever @value{GDBN} prints a value memory will be allocated within
+@value{GDBN} to hold the contents of the value.  It is possible in
+some languages with dynamic typing systems, that an invalid program
+may indicate a value that is incorrectly large, this in turn may cause
+@value{GDBN} to try and allocate an overly large ammount of memory.
+
+@table @code
+@kindex set max-value-size
+@itemx set max-value-size @var{bytes}
+@itemx set max-value-size unlimited
+Set the maximum size, in bytes, that @value{GDBN} will allocate for
+the contents of a value to @var{bytes}.  Any value whose contents
+require more than this number of bytes can't be displayed by
+@value{GDBN}, and trying to display the value will result in an error.
+
+Setting this variable does not effect values that have already been
+allocated within gdb, only future allocations.
+
+By default this variable is set to @var{unlimited}, meaning
+@value{GDBN} will always attempt to allocate space for the values
+contents.
+
+There's a minimum size that @code{max-value-size} can be set too in
+order that @value{GDBN} can still operate correctly.  This varies by
+target, but will generally be around 8 bytes.
+
+@kindex show max-value-size
+@item show max-value-size
+Show the maximum size, in bytes, that @value{GDBN} will allocate for
+the contents of a value.
+@end table
+
 @node Optimized Code
 @chapter Debugging Optimized Code
 @cindex optimized code, debugging
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index ab7a324..93bb3bb 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2015-12-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.base/max-value-size.c: New file.
+	* gdb.base/max-value-size.exp: New file.
+
 2015-12-10  Pedro Alves  <palves@redhat.com>
 
 	* gdb.multi/base.exp: Remove stale "spaces" references.
diff --git a/gdb/testsuite/gdb.base/max-value-size.c b/gdb/testsuite/gdb.base/max-value-size.c
new file mode 100644
index 0000000..4d47280
--- /dev/null
+++ b/gdb/testsuite/gdb.base/max-value-size.c
@@ -0,0 +1,26 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 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/>.  */
+
+char one;
+char ten [10];
+char one_hundred [100];
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/max-value-size.exp b/gdb/testsuite/gdb.base/max-value-size.exp
new file mode 100644
index 0000000..cb09ad8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/max-value-size.exp
@@ -0,0 +1,68 @@
+# Copyright 2015 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/>.
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+    untested $testfile.exp
+    return -1
+}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 0
+}
+
+gdb_test "show max-value-size" \
+    "Maximum value size is unlimited." \
+    "the initial value of max-value-size is unlimited"
+
+with_test_prefix "max-value-size is 'unlimited'" {
+    gdb_test "p/d one" " = 0"
+    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
+}
+
+# Check that setting it low does prevent values being allocated.
+gdb_test_no_output "set max-value-size 25"
+with_test_prefix "max-value-size is '25'" {
+    gdb_test "p/d one" " = 0"
+    gdb_test "p/d one_hundred" "value contents too large \\(100 bytes\\)"
+}
+
+# Check that we can't set the maximum size stupidly low.
+gdb_test "set max-value-size 1" \
+    "max-value-size set too low, increasing to \[0-9\]+ bytes"
+gdb_test "set max-value-size 0" \
+    "max-value-size set too low, increasing to \[0-9\]+ bytes"
+
+# Check we can set it to something "large", and then view our values.
+gdb_test_no_output "set max-value-size 200"
+gdb_test "show max-value-size" \
+    "Maximum value size is 200 bytes." \
+    "check that the value shows as 200 bytes"
+with_test_prefix "max-value-size is '200'" {
+    gdb_test "p/d one" " = 0"
+    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
+}
+
+# Check we can restore it too unlimited.
+gdb_test_no_output "set max-value-size unlimited"
+gdb_test "show max-value-size" \
+    "Maximum value size is unlimited." \
+    "check that the unlimited maximum restored correctly"
+with_test_prefix "max-value-size is 'unlimited' again" {
+    gdb_test "p/d one" " = 0"
+    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
+}
diff --git a/gdb/value.c b/gdb/value.c
index 91bf49e..9554333 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -955,13 +955,60 @@ allocate_value_lazy (struct type *type)
   return val;
 }
 
+/* The maximum size, in bytes, that the contents of a value might have.
+   Setting this to -1 indicates unlimited.  Adjust this variable does not
+   invalidate already allocated values, only prevents future large values
+   being allocated.  */
+
+static int max_value_size = -1;
+static void
+set_max_value_size (char *args, int from_tty,
+		    struct cmd_list_element *c)
+{
+  if (max_value_size > -1 && max_value_size < sizeof (LONGEST))
+    {
+      max_value_size = sizeof (LONGEST);
+      error (_("max-value-size set too low, increasing to %u bytes"),
+	     ((unsigned int) max_value_size));
+    }
+}
+static void
+show_max_value_size (struct ui_file *file, int from_tty,
+		     struct cmd_list_element *c, const char *value)
+{
+  if (max_value_size == -1)
+    fprintf_filtered (file, _("Maximum value size is unlimited.\n"));
+  else
+    fprintf_filtered (file, _("Maximum value size is %d bytes.\n"),
+		      max_value_size);
+}
+
+/* Called before we attempt to allocate or reallocate a buffer for the
+   contents of a value.  TYPE is the type of the value for which we are
+   allocating the buffer.  If the buffer is too large (based on the user
+   controllable setting) then throw an error.  If this function returns
+   then we should attempt to allocate the buffer.  */
+
+static void
+check_type_length_before_alloc (const struct type *type)
+{
+  int length = TYPE_LENGTH (type);
+  if (max_value_size > -1 && (length < 0 || length > max_value_size))
+    error (_("value contents too large (%u bytes)"),
+	   ((unsigned int) length));
+}
+
 /* Allocate the contents of VAL if it has not been allocated yet.  */
 
 static void
 allocate_value_contents (struct value *val)
 {
   if (!val->contents)
-    val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
+    {
+      check_type_length_before_alloc (val->enclosing_type);
+      val->contents =
+	(gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
+    }
 }
 
 /* Allocate a  value  and its contents for type TYPE.  */
@@ -2986,9 +3033,12 @@ value_static_field (struct type *type, int fieldno)
 void
 set_value_enclosing_type (struct value *val, struct type *new_encl_type)
 {
-  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val))) 
-    val->contents =
-      (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
+  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val)))
+    {
+      check_type_length_before_alloc (new_encl_type);
+      val->contents =
+	(gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
+    }
 
   val->enclosing_type = new_encl_type;
 }
@@ -4013,4 +4063,17 @@ Check whether an expression is void.\n\
 Usage: $_isvoid (expression)\n\
 Return 1 if the expression is void, zero otherwise."),
 			 isvoid_internal_fn, NULL);
+
+  add_setshow_zuinteger_unlimited_cmd ("max-value-size",
+				       class_support, &max_value_size, _("\
+Set maximum sized value gdb will load from the inferior."), _("\
+Show maximum sized value gdb will load from the inferior."), _("\
+Use this to control the maximum size, in bytes, of a value that gdb\n\
+will load from the inferior.  Setting this value to 'unlimited'\n\
+disables checking.\n\
+Setting this does not invalidate already allocated values, it only\n\
+prevents future values, larger than this size, from being allocated."),
+			    set_max_value_size,
+			    show_max_value_size,
+			    &setlist, &showlist);
 }
-- 
2.5.1

Re: [PATCH 0/2] Problems with gdb.mi/mi-vla-fortran.exp

[Joel Brobecker]

Hello Andrew,

> Given the above mistake, what follows is just undefined behaviour
> kicking in; gdb reads the uninitialised memory, which happens to be
> non-zero, so it believes pvla2 is associated, it then tries to figure
> out what it's associated with, and reads yet more uninitialised memory.
> 
> In this end it figures that pvla2 is pointing at a huge 3G+ array,
> which in the gdb.mi/mi-vla-fortran.exp test we gdb to print.  This
> causes gdb to allocate the 3G+ of memory to hold the contents of the
> value.
> 
> My question is what to do next from the gdb side of things?
> 
> We could leave the test unchanged, arguing that gdb is asking sensible
> questions, and it's gfortran that's getting things wrong.
> 
> Or we could remove, or comment out, the offending test.
> 
> Leaving the test in place unchanged would be a bit of a shame, I know
> that 3G of memory isn't much on a lot of machines, but it's still
> making my experience running the testsuite pretty painful; I can't be
> the only one.
> 
> I think that this issue has highlighted a bigger issue that effects
> gdb, when dealing with languages that support dynamic types, like
> fortran.  An incorrect program can result in gdb having invalid type
> information, it would be nice if this invalid information did not
> cause gdb to do something so obviously wrong that it brings the
> maching running gdb down.
> 
> The following patches are my attempt to address this issue.  The first
> adds a mechansim to stop gdb allocating overly large arrays for value
> contents, the second enables this mechanism withn the test suite,
> while the third makes additional changes to the mi-vla-fortran.exp
> test to prevent additional errors that are a consequence of the above
> issue.
> 
> Thoughts and comments welcome.

Thanks for the detailed analysis!

I agree with you that we have to protect GDB against this kind of
bad data. As it happens, this used to happen all the time with Ada
programs, where dynamic data is common practice, especially back
in the days of stabs. For that, the Ada support code (in ada-lang.c)
introduced a setting called "varsize-limit", and we used it as
a limit for detecting probable issues in the debugging info.
I was sure that we had contributed that setting, but it turns out
that we contributed only the backbone (the code has access to that
setting), but forgot to contribute the command allowing the user to
change it! Arg... For the record, here is the hunk:

    @@ -14209,6 +14464,15 @@ With an argument, catch only exceptions
                         CATCH_TEMPORARY);

       varsize_limit = 65536;
    +  add_setshow_uinteger_cmd ("varsize-limit", class_support,
    +                           &varsize_limit, _("\
    +Set the maximum number of bytes allowed in a dynamic-sized object."), _("\
    +Show the maximum number of bytes allowed in a dynamic-sized object."), _("\
    +Attempts to access an object whose size is not a compile-time constant\n\
    +and exceeds this limit will cause an error."),
    +                           NULL, NULL, &setlist, &showlist);
    +  /* Add this alias to prevent ambiguity in 'set var ...' command. */
    +  add_alias_cmd ("var", "variable", class_vars, 1, &setlist);

       add_info ("exceptions", info_exceptions_command,
                _("\

So, all in all, I agree with the direction you'd like us to take.
I will followup on the patches you sent.

-- 
Joel

Re: [PATCH 1/3] gdb: New set/show max-value-size command.

[Joel Brobecker]

On Fri, Dec 11, 2015 at 09:38:35PM +0000, Andrew Burgess wrote:
> For languages with dynamic types, an incorrect program could result in
> an incorrect, overly large type being associated with a value.
> Currently, attempting to print such a variable will result in gdb
> attempting to allocate an overly large buffer.
> 
> This could result in the allocation failing, and gdb terminating, or the
> buffer might be allocated, but the machine on which gdb is being run
> might struggle due to the high memory requirement of gdb.
> 
> A new user visible variable in gdb helps guard against such problems,
> two new commands are available:
> 
>    set max-value-size
>    show max-value-size
> 
> The 'max-value-size' is the maximum size in bytes that the contents of a
> value may allocate.  Any attempt to allocate a value with size greater
> than this will result in an error.  The default for this variable is
> currently unlimited.
> 
> Setting the default to unlimited reduces the use of this variable a
> little bit, if a user hits one of these rogue types without realising
> it, then gdb will still allocate the large buffer, with all the problems
> that this entails.  The user will then be forced to probably restart
> their gdb session, and then set the 'max-value-size' variable to guard
> against future crashes.
> 
> However, it's not clear what a good default would actually be, given the
> huge range of resources that are available on different machines.  One
> solution might be to introduce platform specific code that attempts to
> figure out what memory resources are available on the machine running
> gdb, we could then set the max-value-size to some percentage of the
> available memory, while still defaulting to unlimited for those machines
> where we can't figure out a good alternative.  Such a feature is not
> implemented in this commit, but could be added later.

I think it is important to activate this feature by default, precisely
for the reasons you mention. It sounds like one of the reasons why
you elected not to provide a default is that you weren't sure about
what value to use as the default. But, luckily, AdaCore has been using
65536 for as long as I remember as its default varsize_limit. We have
customers of all kinds, and I don't remember anyone telling us that
the default limit is too small.

I think a default of 65536 is a good start. It's large enough that
I can't see any user wanting to display an object that large.
But it's small enough that it'll prevent GDB from unexpectedly
allocate such a chunk of memory that it'll take the user's GDB
session down.

By the way, here is the documentation we have in our manual regarding
this setting. Part of it was prompted by customer feedback. In
particular, it touches on something that made me think when
looking at this new setting: how this limit is applied can be
fairly obscure, when you think about it from the user's perspective.
For instance, when you consider the following expression...

    (gdb) print huge_array[2]

... it is likely that the expression evaluation will first create
a struct value for huge_array. Fortunately, the evalutor is smart
and creates a lazy value, meaning that the limit does not kick in.
Therefore, the limit would only apply to the element of that huge_array,
which hopefully for the user is small enough to pass that check.

| @table @code
| @kindex set varsize-limit
| @item set varsize-limit @var{size}
| Limit the size of the types of objects to @var{size} bytes
| when those sizes are computed from run-time quantities.
| When this limit is set to 0, there is no
| limit.  By default, it is about 65K.  The purpose of having such a limit is
| to prevent @value{GDBN} from trying to grab enormous chunks of virtual
| memory when asked to evaluate a quantity whose bounds have been corrupted
| or have not yet been fully initialized.
| The limit applies to the results
| of some subexpressions as well as to complete expressions.  For example, an
| expression denoting a simple integer component, such as @code{x.y.z}, may
| fail if the size of @var{x.y} is dynamic and exceeds @var{size}.
| On the other hand, @value{GDBN} is sometimes clever;
| the expression @code{A(i)}, where @var{A} is an
| array variable with non-constant size, will generally succeed regardless of the
| bounds on @var{A}, as long as the component size is less than @var{size}.

> gdb/ChangeLog:
> 
> 	* value.c (max_value_size): New variable.
> 	(show_max_value_size): New function.
> 	(check_type_length_before_alloc): New function.
> 	(allocate_value_contents): Call check_type_length_before_alloc.
> 	(set_value_enclosing_type): Likewise.
> 	(_initialize_values): Add set/show handler for max-value-size.
> 	* NEWS: Mention new set/show command.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Value Sizes): New section.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/max-value-size.c: New file.
> 	* gdb.base/max-value-size.exp: New file.

You probably already know this, but the NEWS and doco parts are
reviewed by Eli (FAOD).

> +Whenever @value{GDBN} prints a value memory will be allocated within
> +@value{GDBN} to hold the contents of the value.  It is possible in
> +some languages with dynamic typing systems, that an invalid program
> +may indicate a value that is incorrectly large, this in turn may cause
> +@value{GDBN} to try and allocate an overly large ammount of memory.

(not just an invalid program; also uninitialized variables);
you can use our documentation as inspiration, if you'd like.

> diff --git a/gdb/testsuite/gdb.base/max-value-size.c b/gdb/testsuite/gdb.base/max-value-size.c
> new file mode 100644
> index 0000000..4d47280
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/max-value-size.c
> @@ -0,0 +1,26 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2015 Free Software Foundation, Inc.

You'll need to change the year to be 2015-2016 for all the files
your patch adds :-).  Also, while at it, I think the FSF prefers
the (C) to be there. Thus:

   Copyright (C) 2015-2016 Free Software Foundation, Inc.

> +char one;
> +char ten [10];
> +char one_hundred [100];
> +
> +int
> +main ()

[we decided that, as much as possible, example programs in the testsuite
would also follow the GDB Coding Style]

Can you change "main ()" to "main (void)"?

> +with_test_prefix "max-value-size is 'unlimited'" {
> +    gdb_test "p/d one" " = 0"
> +    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
> +}
> +
> +# Check that setting it low does prevent values being allocated.
> +gdb_test_no_output "set max-value-size 25"
> +with_test_prefix "max-value-size is '25'" {
> +    gdb_test "p/d one" " = 0"
> +    gdb_test "p/d one_hundred" "value contents too large \\(100 bytes\\)"

> +# Check we can set it to something "large", and then view our values.
> +gdb_test_no_output "set max-value-size 200"
> +gdb_test "show max-value-size" \
> +    "Maximum value size is 200 bytes." \
> +    "check that the value shows as 200 bytes"
> +with_test_prefix "max-value-size is '200'" {
> +    gdb_test "p/d one" " = 0"
> +    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
> +}
> +
> +# Check we can restore it too unlimited.
> +gdb_test_no_output "set max-value-size unlimited"
> +gdb_test "show max-value-size" \
> +    "Maximum value size is unlimited." \
> +    "check that the unlimited maximum restored correctly"
> +with_test_prefix "max-value-size is 'unlimited' again" {
> +    gdb_test "p/d one" " = 0"
> +    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
> +}
In all "with_test_prefix" cases, I would add a test to print
one element of one_hundred. This is particularly interesting
in the second situation, where although we can't print one_hundred
in its entirety, we can still print one element. This will help
catch situations where an array value is unexpectedly fetched.

> diff --git a/gdb/value.c b/gdb/value.c
> index 91bf49e..9554333 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -955,13 +955,60 @@ allocate_value_lazy (struct type *type)
>    return val;
>  }
>  
> +/* The maximum size, in bytes, that the contents of a value might have.
> +   Setting this to -1 indicates unlimited.  Adjust this variable does not
> +   invalidate already allocated values, only prevents future large values
> +   being allocated.  */
> +
> +static int max_value_size = -1;

What I would do, to document this static global, is just reference
the user-visible setting.  That way, you do not have to repeat
the user documentation.

Please add an empty line after this variable, to separate it from
the rest of the code.

> +static void
> +set_max_value_size (char *args, int from_tty,
> +		    struct cmd_list_element *c)

Every new function should be documented individually. In this case,
it's implementing the "set max-value-size" command, so it's sufficient
to just say that in the function's intro comment. Eg:

/* Implement the "set max-value-size" command.  */

> +{
> +  if (max_value_size > -1 && max_value_size < sizeof (LONGEST))
> +    {
> +      max_value_size = sizeof (LONGEST);
> +      error (_("max-value-size set too low, increasing to %u bytes"),
> +	     ((unsigned int) max_value_size));
> +    }

sizeof (LONGEST) depends on the host, so it opens the door for
GDB behaving differently depending on the host it runs on, which
can be surprising.

I suggest either: remove the minimum limit entirely, or just choose
an arbitrary one (Eg: 8).

Also, why are you printing max_value_size as an unsigned int, rather
than an int in the error message? This forces us to add a cast, and
its purpose is unclear to me.

> +static void
> +show_max_value_size (struct ui_file *file, int from_tty,
> +		     struct cmd_list_element *c, const char *value)

Same as above; a trivial function intro is suffficient.

> +/* Called before we attempt to allocate or reallocate a buffer for the
> +   contents of a value.  TYPE is the type of the value for which we are
> +   allocating the buffer.  If the buffer is too large (based on the user
> +   controllable setting) then throw an error.  If this function returns
> +   then we should attempt to allocate the buffer.  */
> +
> +static void
> +check_type_length_before_alloc (const struct type *type)
> +{
> +  int length = TYPE_LENGTH (type);
> +  if (max_value_size > -1 && (length < 0 || length > max_value_size))
> +    error (_("value contents too large (%u bytes)"),
> +	   ((unsigned int) length));
> +}

Small style issue - the GDB coding style requires that we have
an empty line after local variable declarations. So can you add
an empty line after "int length = ....", please?

More importantly, why is length an int when TYPE_LENGTH (type)
is an unsigned int? Similarly to be fore, it then opens the door
for having to check for length being negative which does not make
sense, and it also then forces a cast when using it to print
the type's length.

In terms of the function's behavior, I would add a couple of things
to the error message:
  1. Tell the user that he can increase the maximum using
     the "set max-value-size SIZE" command;
  2. print the type's name, if avaiable, in the error message,
     for easier tracking of the issue.

>  /* Allocate the contents of VAL if it has not been allocated yet.  */
>  
>  static void
>  allocate_value_contents (struct value *val)
>  {
>    if (!val->contents)
> -    val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
> +    {
> +      check_type_length_before_alloc (val->enclosing_type);
> +      val->contents =
> +	(gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));

smll formatting nit, binary operators should be at the start of
the next line, rather than the end of the previous one. For the
assignement, this gives us:

      val->contents
        = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));

>  void
>  set_value_enclosing_type (struct value *val, struct type *new_encl_type)
>  {
> -  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val))) 
> -    val->contents =
> -      (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
> +  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val)))
> +    {
> +      check_type_length_before_alloc (new_encl_type);
> +      val->contents =
> +	(gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));

Same here.

-- 
Joel

Re: [PATCH 2/3] gdb: Set max-value-size before running tests.

[Joel Brobecker]

> Setting the max-value-size before running any tests should prevent any
> issues where a failing test causes gdb to bring down the testing
> machine.
> 
> The maximum size is set to 1G, this is large enough to handle all the
> well behaving tests, and should make no difference to the test results.
> 
> There is one test, gdb.mi/mi-vla-fortran.exp that contains undefined
> behaviour, one some machines this test is known to trigger the

I suggest changing "behavior," to "behavior;".
Also "one some" -> "on some".

> max-value-size error.  However, on those machines the test would have
> failed anyway, so this commit does not change the PASS / FAIL nature of
> the test.  A later commit should modify the mi-vla-fortran.exp test to
> expect the failure case.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* lib/gdb.exp (default_gdb_start): Set max-value-size.
> 	* lib/mi-support.exp (default_mi_gdb_start): Likewise.
> 	* gdb.base/max-value-size.exp: Don't check the initial value.

Looks good. This patch is approved to go in after patch #1 is
approved.

Thank you,
-- 
Joel

Re: [PATCH 2/3] gdb: Set max-value-size before running tests.

[Joel Brobecker]

> > Setting the max-value-size before running any tests should prevent any
> > issues where a failing test causes gdb to bring down the testing
> > machine.
> > 
> > The maximum size is set to 1G, this is large enough to handle all the
> > well behaving tests, and should make no difference to the test results.
> > 
> > There is one test, gdb.mi/mi-vla-fortran.exp that contains undefined
> > behaviour, one some machines this test is known to trigger the
> 
> I suggest changing "behavior," to "behavior;".
> Also "one some" -> "on some".
> 
> > max-value-size error.  However, on those machines the test would have
> > failed anyway, so this commit does not change the PASS / FAIL nature of
> > the test.  A later commit should modify the mi-vla-fortran.exp test to
> > expect the failure case.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> > 	* lib/gdb.exp (default_gdb_start): Set max-value-size.
> > 	* lib/mi-support.exp (default_mi_gdb_start): Likewise.
> > 	* gdb.base/max-value-size.exp: Don't check the initial value.
> 
> Looks good. This patch is approved to go in after patch #1 is
> approved.

Actually, I take that back!!!

I just realized that this only makes sense if we left the default to
unlimited, but as you saw in my previous message, this is not the case.
So I think we should simply drop the lib/ part of the patch and
just keep the gdb.base/max-value-size.exp part. We'll the wait and
see if any test triggers an unexpected failure on some platforms,
but those would be. But, my guess is that those unexpected failures
might actually be indicative of a real issue ;-).

Thanks, and sorry for the confusion!
-- 
Joel

Re: [PATCH 3/3] gdb: Guard against undefined behaviour in mi-vla-fortran.exp

[Joel Brobecker]

> gdb/testsuite/ChangeLog:
> 
> 	* gdb.mi/mi-vla-fortran.exp: Add XFAIL for accessing unassociated
> 	pointer.  Don't perform further tests on the unassociated pointer
> 	if the first test fails.

Look good to me.


Thanks,
-- 
Joel

Re: [PATCH 1/3] gdb: New set/show max-value-size command.

[Andrew Burgess]

Eli included for docs review (with thanks).

* Joel Brobecker <brobecker@adacore.com> [2016-01-01 13:43:09 +0400]:

> On Fri, Dec 11, 2015 at 09:38:35PM +0000, Andrew Burgess wrote:
> > For languages with dynamic types, an incorrect program could result in
> > an incorrect, overly large type being associated with a value.
> > Currently, attempting to print such a variable will result in gdb
> > attempting to allocate an overly large buffer.
> > 
> > This could result in the allocation failing, and gdb terminating, or the
> > buffer might be allocated, but the machine on which gdb is being run
> > might struggle due to the high memory requirement of gdb.
> > 
> > A new user visible variable in gdb helps guard against such problems,
> > two new commands are available:
> > 
> >    set max-value-size
> >    show max-value-size
> > 
> > The 'max-value-size' is the maximum size in bytes that the contents of a
> > value may allocate.  Any attempt to allocate a value with size greater
> > than this will result in an error.  The default for this variable is
> > currently unlimited.
> > 
> > Setting the default to unlimited reduces the use of this variable a
> > little bit, if a user hits one of these rogue types without realising
> > it, then gdb will still allocate the large buffer, with all the problems
> > that this entails.  The user will then be forced to probably restart
> > their gdb session, and then set the 'max-value-size' variable to guard
> > against future crashes.
> > 
> > However, it's not clear what a good default would actually be, given the
> > huge range of resources that are available on different machines.  One
> > solution might be to introduce platform specific code that attempts to
> > figure out what memory resources are available on the machine running
> > gdb, we could then set the max-value-size to some percentage of the
> > available memory, while still defaulting to unlimited for those machines
> > where we can't figure out a good alternative.  Such a feature is not
> > implemented in this commit, but could be added later.
> 
> I think it is important to activate this feature by default, precisely
> for the reasons you mention. It sounds like one of the reasons why
> you elected not to provide a default is that you weren't sure about
> what value to use as the default. But, luckily, AdaCore has been using
> 65536 for as long as I remember as its default varsize_limit. We have
> customers of all kinds, and I don't remember anyone telling us that
> the default limit is too small.
> 
> I think a default of 65536 is a good start. It's large enough that
> I can't see any user wanting to display an object that large.
> But it's small enough that it'll prevent GDB from unexpectedly
> allocate such a chunk of memory that it'll take the user's GDB
> session down.

Thanks for the guidance, I've adopted 64k as the default as you suggest.

> 
> By the way, here is the documentation we have in our manual regarding
> this setting. Part of it was prompted by customer feedback. In
> particular, it touches on something that made me think when
> looking at this new setting: how this limit is applied can be
> fairly obscure, when you think about it from the user's perspective.
> For instance, when you consider the following expression...
> 
>     (gdb) print huge_array[2]
> 
> ... it is likely that the expression evaluation will first create
> a struct value for huge_array. Fortunately, the evalutor is smart
> and creates a lazy value, meaning that the limit does not kick in.
> Therefore, the limit would only apply to the element of that huge_array,
> which hopefully for the user is small enough to pass that check.
> 
> | @table @code
> | @kindex set varsize-limit
> | @item set varsize-limit @var{size}
> | Limit the size of the types of objects to @var{size} bytes
> | when those sizes are computed from run-time quantities.
> | When this limit is set to 0, there is no
> | limit.  By default, it is about 65K.  The purpose of having such a limit is
> | to prevent @value{GDBN} from trying to grab enormous chunks of virtual
> | memory when asked to evaluate a quantity whose bounds have been corrupted
> | or have not yet been fully initialized.
> | The limit applies to the results
> | of some subexpressions as well as to complete expressions.  For example, an
> | expression denoting a simple integer component, such as @code{x.y.z}, may
> | fail if the size of @var{x.y} is dynamic and exceeds @var{size}.
> | On the other hand, @value{GDBN} is sometimes clever;
> | the expression @code{A(i)}, where @var{A} is an
> | array variable with non-constant size, will generally succeed regardless of the
> | bounds on @var{A}, as long as the component size is less than @var{size}.

Thanks.  I've stolen some of these words almost verbatim into my
patch, I assume that's not a problem.

> 
> > gdb/ChangeLog:
> > 
> > 	* value.c (max_value_size): New variable.
> > 	(show_max_value_size): New function.
> > 	(check_type_length_before_alloc): New function.
> > 	(allocate_value_contents): Call check_type_length_before_alloc.
> > 	(set_value_enclosing_type): Likewise.
> > 	(_initialize_values): Add set/show handler for max-value-size.
> > 	* NEWS: Mention new set/show command.
> > 
> > gdb/doc/ChangeLog:
> > 
> > 	* gdb.texinfo (Value Sizes): New section.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> > 	* gdb.base/max-value-size.c: New file.
> > 	* gdb.base/max-value-size.exp: New file.
> 
> You probably already know this, but the NEWS and doco parts are
> reviewed by Eli (FAOD).
> 
> > +Whenever @value{GDBN} prints a value memory will be allocated within
> > +@value{GDBN} to hold the contents of the value.  It is possible in
> > +some languages with dynamic typing systems, that an invalid program
> > +may indicate a value that is incorrectly large, this in turn may cause
> > +@value{GDBN} to try and allocate an overly large ammount of memory.
> 
> (not just an invalid program; also uninitialized variables);
> you can use our documentation as inspiration, if you'd like.

I've updated the documentation to better explain this (I hope).

All the remaining issues were minor style issues which I believe are
all now addresses in the revised patch below.

Thanks, for the review.

---

For languages with dynamic types, an incorrect program, or uninitialised
variables within a program, could result in an incorrect, overly large
type being associated with a value.  Currently, attempting to print such
a variable will result in gdb trying to allocate an overly large buffer.

If this large memory allocation fails then the result can be gdb either
terminating, or (due to memory contention) becoming unresponsive for the
user.

A new user visible variable in gdb helps guard against such problems,
two new commands are available:

   set max-value-size
   show max-value-size

The 'max-value-size' is the maximum size in bytes that gdb will allocate
for the contents of a value.  Any attempt to allocate a value with a
size greater than this will result in an error.  The initial default for
this limit is set at 64k, this is based on a similar limit that exists
within the ada specific code.

It is possible for the user to set max-value-size to unlimited, in which
case the old behaviour is restored.

gdb/ChangeLog:

	* value.c (max_value_size): New variable.
        (MIN_VALUE_FOR_MAX_VALUE_SIZE): New define.
	(show_max_value_size): New function.
	(check_type_length_before_alloc): New function.
	(allocate_value_contents): Call check_type_length_before_alloc.
	(set_value_enclosing_type): Likewise.
	(_initialize_values): Add set/show handler for max-value-size.
	* NEWS: Mention new set/show command.

gdb/doc/ChangeLog:

	* gdb.texinfo (Value Sizes): New section, also add the new section
	to the menu.

gdb/testsuite/ChangeLog:

	* gdb.base/max-value-size.c: New file.
	* gdb.base/max-value-size.exp: New file.
---
 gdb/ChangeLog                             | 12 ++++
 gdb/NEWS                                  |  6 ++
 gdb/doc/ChangeLog                         |  5 ++
 gdb/doc/gdb.texinfo                       | 43 ++++++++++++++
 gdb/testsuite/ChangeLog                   |  5 ++
 gdb/testsuite/gdb.base/max-value-size.c   | 26 +++++++++
 gdb/testsuite/gdb.base/max-value-size.exp | 97 +++++++++++++++++++++++++++++++
 gdb/value.c                               | 97 +++++++++++++++++++++++++++++--
 8 files changed, 287 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/max-value-size.c
 create mode 100644 gdb/testsuite/gdb.base/max-value-size.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3d8923b..42633d2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2016-01-05  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* value.c (max_value_size): New variable.
+	(MIN_VALUE_FOR_MAX_VALUE_SIZE): New define.
+	(set_max_value_size): New function.
+	(show_max_value_size): New function.
+	(check_type_length_before_alloc): New function.
+	(allocate_value_contents): Call check_type_length_before_alloc.
+	(set_value_enclosing_type): Likewise.
+	(_initialize_values): Add set/show handler for max-value-size.
+	* NEWS: Mention new set/show command.
+
 2016-01-04  Markus Metzger  <markus.t.metzger@intel.com>
 
 	* btrace.c (btrace_pt_readmem_callback): Do not return in TRY/CATCH.
diff --git a/gdb/NEWS b/gdb/NEWS
index 484d98d..332808e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -60,6 +60,12 @@ show ada print-signatures"
   Control whether parameter types and return types are displayed in overloads
   selection menus.  It is activaled (@code{on}) by default.
 
+set max-value-size
+show max-value-size
+  Control the maximum size, in bytes, that GDB will allocate for value
+  contents.  Prevent incorrect programs from causing GDB to allocate
+  overly large buffers.  Default is unlimited.
+
 * The "disassemble" command accepts a new modifier: /s.
   It prints mixed source+disassembly like /m with two differences:
   - disassembled instructions are now printed in program order, and
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index ef3bdaa..1465f6d 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2016-01-05  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.texinfo (Value Sizes): New section, also add the new section
+	to the menu.
+
 2015-12-11  Don Breazeal  <donb@codesourcery.com>
 
 	* gdb.texinfo (Forks): Correct Linux kernel version where
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0778383..ae4b4ed 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -8531,6 +8531,7 @@ being passed the type of @var{arg} as the argument.
                                 character set than GDB does
 * Caching Target Data::         Data caching for targets
 * Searching Memory::            Searching memory for a sequence of bytes
+* Value Sizes::                 Managing memory allocated for values
 @end menu
 
 @node Expressions
@@ -11596,6 +11597,48 @@ $1 = 1
 $2 = (void *) 0x8049560
 @end smallexample
 
+@node Value Sizes
+@section Value Sizes
+
+Whenever @value{GDBN} prints a value memory will be allocated within
+@value{GDBN} to hold the contents of the value.  It is possible in
+some languages with dynamic typing systems, that an invalid program
+may indicate a value that is incorrectly large, this in turn may cause
+@value{GDBN} to try and allocate an overly large ammount of memory.
+
+@table @code
+@kindex set max-value-size
+@itemx set max-value-size @var{bytes}
+@itemx set max-value-size unlimited
+Set the maximum size, in bytes, that @value{GDBN} will allocate for
+the contents of a value to @var{bytes}.  Any value whose contents
+require more than this number of bytes can't be displayed by
+@value{GDBN}, and trying to display the value will result in an error.
+
+Setting this variable does not effect values that have already been
+allocated within gdb, only future allocations.
+
+There's a minimum size that @code{max-value-size} can be set too in
+order that @value{GDBN} can still operate correctly, this minimum is
+currently 16 bytes.
+
+The limit applies to the results of some subexpressions as well as to
+complete expressions.  For example, an expression denoting a simple
+integer component, such as @code{x.y.z}, may fail if the size of
+@var{x.y} is dynamic and exceeds @var{bytes}.  On the other hand,
+@value{GDBN} is sometimes clever; the expression @code{A[i]}, where
+@var{A} is an array variable with non-constant size, will generally
+succeed regardless of the bounds on @var{A}, as long as the component
+size is less than @var{bytes}.
+
+The default value of @code{max-value-size} is currently 64k.
+
+@kindex show max-value-size
+@item show max-value-size
+Show the maximum size, in bytes, that @value{GDBN} will allocate for
+the contents of a value.
+@end table
+
 @node Optimized Code
 @chapter Debugging Optimized Code
 @cindex optimized code, debugging
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 45a6c6a..b10da8c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2016-01-05  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.base/max-value-size.c: New file.
+	* gdb.base/max-value-size.exp: New file.
+
 2016-01-04  Markus Metzger  <markus.t.metzger@intel.com>
 
 	* gdb.btrace/dlopen.exp: New.
diff --git a/gdb/testsuite/gdb.base/max-value-size.c b/gdb/testsuite/gdb.base/max-value-size.c
new file mode 100644
index 0000000..b6a6fe5
--- /dev/null
+++ b/gdb/testsuite/gdb.base/max-value-size.c
@@ -0,0 +1,26 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2016 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/>.  */
+
+char one;
+char ten [10];
+char one_hundred [100];
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/max-value-size.exp b/gdb/testsuite/gdb.base/max-value-size.exp
new file mode 100644
index 0000000..63a97a0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/max-value-size.exp
@@ -0,0 +1,97 @@
+# Copyright (C) 2016 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/>.
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+    untested $testfile.exp
+    return -1
+}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 0
+}
+
+# Run "show max-value-size" and return the interesting bit of the
+# result.  This is either the maximum size in bytes, or the string
+# "unlimited".
+proc get_max_value_size {} {
+    global gdb_prompt
+    global decimal
+
+    gdb_test_multiple "show max-value-size" "" {
+	-re "Maximum value size is ($decimal) bytes.*$gdb_prompt $" {
+	    return $expect_out(1,string)
+	}
+	-re "Maximum value size is unlimited.*$gdb_prompt $" {
+	    return "unlimited"
+	}
+    }
+}
+
+# Assuming that MAX_VALUE_SIZE is the current value for
+# max-value-size, print the test values.  Use TEST_PREFIX to make the
+# test names unique.
+proc do_value_printing { max_value_size test_prefix } {
+    with_test_prefix ${test_prefix} {
+	gdb_test "p/d one" " = 0"
+	if { $max_value_size != "unlimited" && $max_value_size < 100 } then {
+	    gdb_test "p/d one_hundred" \
+		"value requires 100 bytes, which is more than max-value-size"
+	} else {
+	    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
+	}
+	gdb_test "p/d one_hundred \[99\]" " = 0"
+    }
+}
+
+# Install SET_VALUE as the value for max-value-size, then print the
+# test values.
+proc set_and_check_max_value_size { set_value } {
+    if { $set_value == "unlimited" } then {
+	set check_pattern "unlimited"
+    } else {
+	set check_pattern "${set_value} bytes"
+    }
+
+    gdb_test_no_output "set max-value-size ${set_value}"
+    gdb_test "show max-value-size" \
+	"Maximum value size is ${check_pattern}." \
+	"check that the value shows as ${check_pattern}"
+
+    do_value_printing ${set_value} "max-value-size is '${set_value}'"
+}
+
+# Check the default value is sufficient.
+do_value_printing [get_max_value_size] "using initial max-value-size"
+
+# Check some values for max-value-size that should prevent some
+# allocations.
+set_and_check_max_value_size 25
+set_and_check_max_value_size 99
+
+# Check values for max-value-size that should allow all allocations.
+set_and_check_max_value_size 100
+set_and_check_max_value_size 200
+set_and_check_max_value_size "unlimited"
+
+# Check that we can't set the maximum size stupidly low.
+gdb_test "set max-value-size 1" \
+    "max-value-size set too low, increasing to \[0-9\]+ bytes"
+gdb_test "set max-value-size 0" \
+    "max-value-size set too low, increasing to \[0-9\]+ bytes"
+gdb_test "set max-value-size -5" \
+    "only -1 is allowed to set as unlimited"
diff --git a/gdb/value.c b/gdb/value.c
index eeb2b7d..41a5513 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -955,13 +955,86 @@ allocate_value_lazy (struct type *type)
   return val;
 }
 
+/* The maximum size, in bytes, that GDB will try to allocate for a value.
+   The initial value of 64k was not selected for any specific reason, it is
+   just a reasonable starting point.  */
+
+static int max_value_size = 65536; /* 64k bytes */
+
+/* It is critical that the MAX_VALUE_SIZE is always greater than the size
+   of LONGEST, otherwise GDB will not be able to parse integer values from
+   the CLI; for example if the MAX_VALUE_SIZE could be set to 1 then GDB
+   would be unable to parse "set max-value-size 2".
+
+   As we want a consistent GDB experience across hosts with different sizes
+   of LONGEST, this arbitrary minimum value was selected, so long as this
+   is bigger than LONGEST of all GDB supported hosts we're fine.  */
+
+#define MIN_VALUE_FOR_MAX_VALUE_SIZE 16
+
+/* Implement the "set max-value-size" command.  */
+
+static void
+set_max_value_size (char *args, int from_tty,
+		    struct cmd_list_element *c)
+{
+  gdb_assert (max_value_size == -1 || max_value_size >= 0);
+  gdb_assert (sizeof (LONGEST) < MIN_VALUE_FOR_MAX_VALUE_SIZE);
+
+  if (max_value_size > -1 && max_value_size < MIN_VALUE_FOR_MAX_VALUE_SIZE)
+    {
+      max_value_size = MIN_VALUE_FOR_MAX_VALUE_SIZE;
+      error (_("max-value-size set too low, increasing to %d bytes"),
+	     max_value_size);
+    }
+}
+
+/* Implement the "show max-value-size" command.  */
+
+static void
+show_max_value_size (struct ui_file *file, int from_tty,
+		     struct cmd_list_element *c, const char *value)
+{
+  if (max_value_size == -1)
+    fprintf_filtered (file, _("Maximum value size is unlimited.\n"));
+  else
+    fprintf_filtered (file, _("Maximum value size is %d bytes.\n"),
+		      max_value_size);
+}
+
+/* Called before we attempt to allocate or reallocate a buffer for the
+   contents of a value.  TYPE is the type of the value for which we are
+   allocating the buffer.  If the buffer is too large (based on the user
+   controllable setting) then throw an error.  If this function returns
+   then we should attempt to allocate the buffer.  */
+
+static void
+check_type_length_before_alloc (const struct type *type)
+{
+  unsigned int length = TYPE_LENGTH (type);
+
+  if (max_value_size > -1 && length > max_value_size)
+    {
+      if (TYPE_NAME (type) != NULL)
+	error (_("value of type `%s' requires %d bytes, which is more "
+		 "than max-value-size"), TYPE_NAME (type), length);
+      else
+	error (_("value requires %d bytes, which is more than "
+		 "max-value-size"), length);
+    }
+}
+
 /* Allocate the contents of VAL if it has not been allocated yet.  */
 
 static void
 allocate_value_contents (struct value *val)
 {
   if (!val->contents)
-    val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
+    {
+      check_type_length_before_alloc (val->enclosing_type);
+      val->contents
+	= (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
+    }
 }
 
 /* Allocate a  value  and its contents for type TYPE.  */
@@ -2986,9 +3059,12 @@ value_static_field (struct type *type, int fieldno)
 void
 set_value_enclosing_type (struct value *val, struct type *new_encl_type)
 {
-  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val))) 
-    val->contents =
-      (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
+  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val)))
+    {
+      check_type_length_before_alloc (new_encl_type);
+      val->contents
+	= (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
+    }
 
   val->enclosing_type = new_encl_type;
 }
@@ -4013,4 +4089,17 @@ Check whether an expression is void.\n\
 Usage: $_isvoid (expression)\n\
 Return 1 if the expression is void, zero otherwise."),
 			 isvoid_internal_fn, NULL);
+
+  add_setshow_zuinteger_unlimited_cmd ("max-value-size",
+				       class_support, &max_value_size, _("\
+Set maximum sized value gdb will load from the inferior."), _("\
+Show maximum sized value gdb will load from the inferior."), _("\
+Use this to control the maximum size, in bytes, of a value that gdb\n\
+will load from the inferior.  Setting this value to 'unlimited'\n\
+disables checking.\n\
+Setting this does not invalidate already allocated values, it only\n\
+prevents future values, larger than this size, from being allocated."),
+			    set_max_value_size,
+			    show_max_value_size,
+			    &setlist, &showlist);
 }
-- 
2.5.1

Re: [PATCH 2/3] gdb: Set max-value-size before running tests.

[Andrew Burgess]

* Joel Brobecker <brobecker@adacore.com> [2016-01-01 13:52:54 +0400]:

> > > Setting the max-value-size before running any tests should prevent any
> > > issues where a failing test causes gdb to bring down the testing
> > > machine.
> > > 
> > > The maximum size is set to 1G, this is large enough to handle all the
> > > well behaving tests, and should make no difference to the test results.
> > > 
> > > There is one test, gdb.mi/mi-vla-fortran.exp that contains undefined
> > > behaviour, one some machines this test is known to trigger the
> > 
> > I suggest changing "behavior," to "behavior;".
> > Also "one some" -> "on some".
> > 
> > > max-value-size error.  However, on those machines the test would have
> > > failed anyway, so this commit does not change the PASS / FAIL nature of
> > > the test.  A later commit should modify the mi-vla-fortran.exp test to
> > > expect the failure case.
> > > 
> > > gdb/testsuite/ChangeLog:
> > > 
> > > 	* lib/gdb.exp (default_gdb_start): Set max-value-size.
> > > 	* lib/mi-support.exp (default_mi_gdb_start): Likewise.
> > > 	* gdb.base/max-value-size.exp: Don't check the initial value.
> > 
> > Looks good. This patch is approved to go in after patch #1 is
> > approved.
> 
> Actually, I take that back!!!
> 
> I just realized that this only makes sense if we left the default to
> unlimited, but as you saw in my previous message, this is not the case.
> So I think we should simply drop the lib/ part of the patch and
> just keep the gdb.base/max-value-size.exp part. We'll the wait and
> see if any test triggers an unexpected failure on some platforms,
> but those would be. But, my guess is that those unexpected failures
> might actually be indicative of a real issue ;-).
> 
> Thanks, and sorry for the confusion!

No problem.  With the default now being set in patch #1, the *.exp
changes naturally move there too.  As a result this patch is now
empty.

I've applied it as obvious ;-)

Thanks,
Andrew

Re: [PATCH 3/3] gdb: Guard against undefined behaviour in mi-vla-fortran.exp

[Andrew Burgess]

* Joel Brobecker <brobecker@adacore.com> [2016-01-01 15:08:42 +0400]:

> > gdb/testsuite/ChangeLog:
> > 
> > 	* gdb.mi/mi-vla-fortran.exp: Add XFAIL for accessing unassociated
> > 	pointer.  Don't perform further tests on the unassociated pointer
> > 	if the first test fails.
> 
> Look good to me.

Thanks for the review, I'll apply once #1 is approved.

Andrew

Re: [PATCH 1/3] gdb: New set/show max-value-size command.

[Pedro Alves]

On 01/05/2016 02:12 PM, Andrew Burgess wrote:

> +/* It is critical that the MAX_VALUE_SIZE is always greater than the size
> +   of LONGEST, otherwise GDB will not be able to parse integer values from
> +   the CLI; for example if the MAX_VALUE_SIZE could be set to 1 then GDB
> +   would be unable to parse "set max-value-size 2".
> +
> +   As we want a consistent GDB experience across hosts with different sizes
> +   of LONGEST, this arbitrary minimum value was selected, so long as this
> +   is bigger than LONGEST of all GDB supported hosts we're fine.  */
> +
> +#define MIN_VALUE_FOR_MAX_VALUE_SIZE 16
> +
> +/* Implement the "set max-value-size" command.  */
> +
> +static void
> +set_max_value_size (char *args, int from_tty,
> +		    struct cmd_list_element *c)
> +{
> +  gdb_assert (max_value_size == -1 || max_value_size >= 0);
> +  gdb_assert (sizeof (LONGEST) < MIN_VALUE_FOR_MAX_VALUE_SIZE);

Make the second one a gdb_static_assert, and put it next to the
MIN_VALUE_FOR_MAX_VALUE_SIZE definition.

Thanks,
Pedro Alves

Re: [PATCH 1/3] gdb: New set/show max-value-size command.

[Eli Zaretskii]

> Date: Tue, 5 Jan 2016 14:12:41 +0000
> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: gdb-patches@sourceware.org
> 
> For languages with dynamic types, an incorrect program, or uninitialised
> variables within a program, could result in an incorrect, overly large
> type being associated with a value.  Currently, attempting to print such
> a variable will result in gdb trying to allocate an overly large buffer.
> 
> If this large memory allocation fails then the result can be gdb either
> terminating, or (due to memory contention) becoming unresponsive for the
> user.
> 
> A new user visible variable in gdb helps guard against such problems,
> two new commands are available:
> 
>    set max-value-size
>    show max-value-size
> 
> The 'max-value-size' is the maximum size in bytes that gdb will allocate
> for the contents of a value.  Any attempt to allocate a value with a
> size greater than this will result in an error.  The initial default for
> this limit is set at 64k, this is based on a similar limit that exists
> within the ada specific code.
> 
> It is possible for the user to set max-value-size to unlimited, in which
> case the old behaviour is restored.

Thanks.

> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Value Sizes): New section, also add the new section
> 	to the menu.

The addition to the menu is probably in a different node, so it needs
a separate entry in the ChangeLog.

> +set max-value-size
> +show max-value-size
> +  Control the maximum size, in bytes, that GDB will allocate for value
> +  contents.  Prevent incorrect programs from causing GDB to allocate
> +  overly large buffers.  Default is unlimited.

"Controls" and "Prevents".  Also "maximum size of memory" (we allocate
memory, not size).

> +@table @code
> +@kindex set max-value-size
> +@itemx set max-value-size @var{bytes}
> +@itemx set max-value-size unlimited
> +Set the maximum size, in bytes, that @value{GDBN} will allocate for

Same here:

  Set the maximum size of memory, in bytes, that @value{GDBN} will ...

Also, the "in bytes" part is redundant, since the parameter is called
"bytes", which is self-explanatory.

> +the contents of a value to @var{bytes}.  Any value whose contents
> +require more than this number of bytes can't be displayed by
> +@value{GDBN}, and trying to display the value will result in an error.

I would remove the "can't be displayed by @value{GDBN}" part.  It can
be interpreted to mean some limitation inherent in GDB, which is not
what you want to convey.  The rest of the sentence says it all: trying
to display a value that requires more memory than that will result in
an error.

> +Setting this variable does not effect values that have already been
> +allocated within gdb, only future allocations.
                    ^^^
@value{GDBN}

> +There's a minimum size that @code{max-value-size} can be set too in
                                                                ^^^
"to"

OK with those fixed.

Thanks.

Re: [PATCH 1/3] gdb: New set/show max-value-size command.

[Andrew Burgess]

* Eli Zaretskii <eliz@gnu.org> [2016-01-05 18:24:32 +0200]:

> > Date: Tue, 5 Jan 2016 14:12:41 +0000
> > From: Andrew Burgess <andrew.burgess@embecosm.com>
> > Cc: gdb-patches@sourceware.org
> > 
> > For languages with dynamic types, an incorrect program, or uninitialised
> > variables within a program, could result in an incorrect, overly large
> > type being associated with a value.  Currently, attempting to print such
> > a variable will result in gdb trying to allocate an overly large buffer.
> > 
> > If this large memory allocation fails then the result can be gdb either
> > terminating, or (due to memory contention) becoming unresponsive for the
> > user.
> > 
> > A new user visible variable in gdb helps guard against such problems,
> > two new commands are available:
> > 
> >    set max-value-size
> >    show max-value-size
> > 
> > The 'max-value-size' is the maximum size in bytes that gdb will allocate
> > for the contents of a value.  Any attempt to allocate a value with a
> > size greater than this will result in an error.  The initial default for
> > this limit is set at 64k, this is based on a similar limit that exists
> > within the ada specific code.
> > 
> > It is possible for the user to set max-value-size to unlimited, in which
> > case the old behaviour is restored.
> 
> Thanks.
> 
> > gdb/doc/ChangeLog:
> > 
> > 	* gdb.texinfo (Value Sizes): New section, also add the new section
> > 	to the menu.
> 
> The addition to the menu is probably in a different node, so it needs
> a separate entry in the ChangeLog.
> 
> > +set max-value-size
> > +show max-value-size
> > +  Control the maximum size, in bytes, that GDB will allocate for value
> > +  contents.  Prevent incorrect programs from causing GDB to allocate
> > +  overly large buffers.  Default is unlimited.
> 
> "Controls" and "Prevents".  Also "maximum size of memory" (we allocate
> memory, not size).
> 
> > +@table @code
> > +@kindex set max-value-size
> > +@itemx set max-value-size @var{bytes}
> > +@itemx set max-value-size unlimited
> > +Set the maximum size, in bytes, that @value{GDBN} will allocate for
> 
> Same here:
> 
>   Set the maximum size of memory, in bytes, that @value{GDBN} will ...
> 
> Also, the "in bytes" part is redundant, since the parameter is called
> "bytes", which is self-explanatory.
> 
> > +the contents of a value to @var{bytes}.  Any value whose contents
> > +require more than this number of bytes can't be displayed by
> > +@value{GDBN}, and trying to display the value will result in an error.
> 
> I would remove the "can't be displayed by @value{GDBN}" part.  It can
> be interpreted to mean some limitation inherent in GDB, which is not
> what you want to convey.  The rest of the sentence says it all: trying
> to display a value that requires more memory than that will result in
> an error.
> 
> > +Setting this variable does not effect values that have already been
> > +allocated within gdb, only future allocations.
>                     ^^^
> @value{GDBN}
> 
> > +There's a minimum size that @code{max-value-size} can be set too in
>                                                                 ^^^
> "to"
> 
> OK with those fixed.

New revision resolves all of Eli's comments, and also addresses
Pedro's comment.

OK to apply?

Thanks,
Andrew

---

For languages with dynamic types, an incorrect program, or uninitialised
variables within a program, could result in an incorrect, overly large
type being associated with a value.  Currently, attempting to print such
a variable will result in gdb trying to allocate an overly large buffer.

If this large memory allocation fails then the result can be gdb either
terminating, or (due to memory contention) becoming unresponsive for the
user.

A new user visible variable in gdb helps guard against such problems,
two new commands are available:

   set max-value-size
   show max-value-size

The 'max-value-size' is the maximum size of memory in bytes that gdb
will allocate for the contents of a value.  Any attempt to allocate a
value with a size greater than this will result in an error.  The
initial default for this limit is set at 64k, this is based on a similar
limit that exists within the ada specific code.

It is possible for the user to set max-value-size to unlimited, in which
case the old behaviour is restored.

gdb/ChangeLog:

	* value.c (max_value_size): New variable.
	(MIN_VALUE_FOR_MAX_VALUE_SIZE): New define.
	(show_max_value_size): New function.
	(check_type_length_before_alloc): New function.
	(allocate_value_contents): Call check_type_length_before_alloc.
	(set_value_enclosing_type): Likewise.
	(_initialize_values): Add set/show handler for max-value-size.
	* NEWS: Mention new set/show command.

gdb/doc/ChangeLog:

	* gdb.texinfo (Value Sizes): New section.
	(Data): Add the 'Value Sizes' node to the menu.

gdb/testsuite/ChangeLog:

	* gdb.base/max-value-size.c: New file.
	* gdb.base/max-value-size.exp: New file.
---
 gdb/ChangeLog                             | 12 ++++
 gdb/NEWS                                  |  6 ++
 gdb/doc/ChangeLog                         |  5 ++
 gdb/doc/gdb.texinfo                       | 42 +++++++++++++
 gdb/testsuite/ChangeLog                   |  5 ++
 gdb/testsuite/gdb.base/max-value-size.c   | 26 +++++++++
 gdb/testsuite/gdb.base/max-value-size.exp | 97 +++++++++++++++++++++++++++++++
 gdb/value.c                               | 97 +++++++++++++++++++++++++++++--
 8 files changed, 286 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/max-value-size.c
 create mode 100644 gdb/testsuite/gdb.base/max-value-size.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3d8923b..42633d2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2016-01-05  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* value.c (max_value_size): New variable.
+	(MIN_VALUE_FOR_MAX_VALUE_SIZE): New define.
+	(set_max_value_size): New function.
+	(show_max_value_size): New function.
+	(check_type_length_before_alloc): New function.
+	(allocate_value_contents): Call check_type_length_before_alloc.
+	(set_value_enclosing_type): Likewise.
+	(_initialize_values): Add set/show handler for max-value-size.
+	* NEWS: Mention new set/show command.
+
 2016-01-04  Markus Metzger  <markus.t.metzger@intel.com>
 
 	* btrace.c (btrace_pt_readmem_callback): Do not return in TRY/CATCH.
diff --git a/gdb/NEWS b/gdb/NEWS
index 484d98d..861b125 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -60,6 +60,12 @@ show ada print-signatures"
   Control whether parameter types and return types are displayed in overloads
   selection menus.  It is activaled (@code{on}) by default.
 
+set max-value-size
+show max-value-size
+  Controls the maximum size of memory, in bytes, that GDB will
+  allocate for value contents.  Prevents incorrect programs from
+  causing GDB to allocate overly large buffers.  Default is 64k.
+
 * The "disassemble" command accepts a new modifier: /s.
   It prints mixed source+disassembly like /m with two differences:
   - disassembled instructions are now printed in program order, and
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index ef3bdaa..97a1b25 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2016-01-05  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.texinfo (Value Sizes): New section.
+	(Data): Add the 'Value Sizes' node to the menu.
+
 2015-12-11  Don Breazeal  <donb@codesourcery.com>
 
 	* gdb.texinfo (Forks): Correct Linux kernel version where
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0778383..9dea873 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -8531,6 +8531,7 @@ being passed the type of @var{arg} as the argument.
                                 character set than GDB does
 * Caching Target Data::         Data caching for targets
 * Searching Memory::            Searching memory for a sequence of bytes
+* Value Sizes::                 Managing memory allocated for values
 @end menu
 
 @node Expressions
@@ -11596,6 +11597,47 @@ $1 = 1
 $2 = (void *) 0x8049560
 @end smallexample
 
+@node Value Sizes
+@section Value Sizes
+
+Whenever @value{GDBN} prints a value memory will be allocated within
+@value{GDBN} to hold the contents of the value.  It is possible in
+some languages with dynamic typing systems, that an invalid program
+may indicate a value that is incorrectly large, this in turn may cause
+@value{GDBN} to try and allocate an overly large ammount of memory.
+
+@table @code
+@kindex set max-value-size
+@itemx set max-value-size @var{bytes}
+@itemx set max-value-size unlimited
+Set the maximum size of memory that @value{GDBN} will allocate for the
+contents of a value to @var{bytes}, trying to display a value that
+requires more memory than that will result in an error.
+
+Setting this variable does not effect values that have already been
+allocated within @value{GDBN}, only future allocations.
+
+There's a minimum size that @code{max-value-size} can be set to in
+order that @value{GDBN} can still operate correctly, this minimum is
+currently 16 bytes.
+
+The limit applies to the results of some subexpressions as well as to
+complete expressions.  For example, an expression denoting a simple
+integer component, such as @code{x.y.z}, may fail if the size of
+@var{x.y} is dynamic and exceeds @var{bytes}.  On the other hand,
+@value{GDBN} is sometimes clever; the expression @code{A[i]}, where
+@var{A} is an array variable with non-constant size, will generally
+succeed regardless of the bounds on @var{A}, as long as the component
+size is less than @var{bytes}.
+
+The default value of @code{max-value-size} is currently 64k.
+
+@kindex show max-value-size
+@item show max-value-size
+Show the maximum size of memory, in bytes, that @value{GDBN} will
+allocate for the contents of a value.
+@end table
+
 @node Optimized Code
 @chapter Debugging Optimized Code
 @cindex optimized code, debugging
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 45a6c6a..b10da8c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2016-01-05  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.base/max-value-size.c: New file.
+	* gdb.base/max-value-size.exp: New file.
+
 2016-01-04  Markus Metzger  <markus.t.metzger@intel.com>
 
 	* gdb.btrace/dlopen.exp: New.
diff --git a/gdb/testsuite/gdb.base/max-value-size.c b/gdb/testsuite/gdb.base/max-value-size.c
new file mode 100644
index 0000000..b6a6fe5
--- /dev/null
+++ b/gdb/testsuite/gdb.base/max-value-size.c
@@ -0,0 +1,26 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2016 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/>.  */
+
+char one;
+char ten [10];
+char one_hundred [100];
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/max-value-size.exp b/gdb/testsuite/gdb.base/max-value-size.exp
new file mode 100644
index 0000000..63a97a0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/max-value-size.exp
@@ -0,0 +1,97 @@
+# Copyright (C) 2016 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/>.
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+    untested $testfile.exp
+    return -1
+}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 0
+}
+
+# Run "show max-value-size" and return the interesting bit of the
+# result.  This is either the maximum size in bytes, or the string
+# "unlimited".
+proc get_max_value_size {} {
+    global gdb_prompt
+    global decimal
+
+    gdb_test_multiple "show max-value-size" "" {
+	-re "Maximum value size is ($decimal) bytes.*$gdb_prompt $" {
+	    return $expect_out(1,string)
+	}
+	-re "Maximum value size is unlimited.*$gdb_prompt $" {
+	    return "unlimited"
+	}
+    }
+}
+
+# Assuming that MAX_VALUE_SIZE is the current value for
+# max-value-size, print the test values.  Use TEST_PREFIX to make the
+# test names unique.
+proc do_value_printing { max_value_size test_prefix } {
+    with_test_prefix ${test_prefix} {
+	gdb_test "p/d one" " = 0"
+	if { $max_value_size != "unlimited" && $max_value_size < 100 } then {
+	    gdb_test "p/d one_hundred" \
+		"value requires 100 bytes, which is more than max-value-size"
+	} else {
+	    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
+	}
+	gdb_test "p/d one_hundred \[99\]" " = 0"
+    }
+}
+
+# Install SET_VALUE as the value for max-value-size, then print the
+# test values.
+proc set_and_check_max_value_size { set_value } {
+    if { $set_value == "unlimited" } then {
+	set check_pattern "unlimited"
+    } else {
+	set check_pattern "${set_value} bytes"
+    }
+
+    gdb_test_no_output "set max-value-size ${set_value}"
+    gdb_test "show max-value-size" \
+	"Maximum value size is ${check_pattern}." \
+	"check that the value shows as ${check_pattern}"
+
+    do_value_printing ${set_value} "max-value-size is '${set_value}'"
+}
+
+# Check the default value is sufficient.
+do_value_printing [get_max_value_size] "using initial max-value-size"
+
+# Check some values for max-value-size that should prevent some
+# allocations.
+set_and_check_max_value_size 25
+set_and_check_max_value_size 99
+
+# Check values for max-value-size that should allow all allocations.
+set_and_check_max_value_size 100
+set_and_check_max_value_size 200
+set_and_check_max_value_size "unlimited"
+
+# Check that we can't set the maximum size stupidly low.
+gdb_test "set max-value-size 1" \
+    "max-value-size set too low, increasing to \[0-9\]+ bytes"
+gdb_test "set max-value-size 0" \
+    "max-value-size set too low, increasing to \[0-9\]+ bytes"
+gdb_test "set max-value-size -5" \
+    "only -1 is allowed to set as unlimited"
diff --git a/gdb/value.c b/gdb/value.c
index eeb2b7d..41f44c8 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -955,13 +955,86 @@ allocate_value_lazy (struct type *type)
   return val;
 }
 
+/* The maximum size, in bytes, that GDB will try to allocate for a value.
+   The initial value of 64k was not selected for any specific reason, it is
+   just a reasonable starting point.  */
+
+static int max_value_size = 65536; /* 64k bytes */
+
+/* It is critical that the MAX_VALUE_SIZE is at least as bit as the size of
+   LONGEST, otherwise GDB will not be able to parse integer values from the
+   CLI; for example if the MAX_VALUE_SIZE could be set to 1 then GDB would
+   be unable to parse "set max-value-size 2".
+
+   As we want a consistent GDB experience across hosts with different sizes
+   of LONGEST, this arbitrary minimum value was selected, so long as this
+   is bigger than LONGEST of all GDB supported hosts we're fine.  */
+
+#define MIN_VALUE_FOR_MAX_VALUE_SIZE 16
+gdb_static_assert (sizeof (LONGEST) <= MIN_VALUE_FOR_MAX_VALUE_SIZE);
+
+/* Implement the "set max-value-size" command.  */
+
+static void
+set_max_value_size (char *args, int from_tty,
+		    struct cmd_list_element *c)
+{
+  gdb_assert (max_value_size == -1 || max_value_size >= 0);
+
+  if (max_value_size > -1 && max_value_size < MIN_VALUE_FOR_MAX_VALUE_SIZE)
+    {
+      max_value_size = MIN_VALUE_FOR_MAX_VALUE_SIZE;
+      error (_("max-value-size set too low, increasing to %d bytes"),
+	     max_value_size);
+    }
+}
+
+/* Implement the "show max-value-size" command.  */
+
+static void
+show_max_value_size (struct ui_file *file, int from_tty,
+		     struct cmd_list_element *c, const char *value)
+{
+  if (max_value_size == -1)
+    fprintf_filtered (file, _("Maximum value size is unlimited.\n"));
+  else
+    fprintf_filtered (file, _("Maximum value size is %d bytes.\n"),
+		      max_value_size);
+}
+
+/* Called before we attempt to allocate or reallocate a buffer for the
+   contents of a value.  TYPE is the type of the value for which we are
+   allocating the buffer.  If the buffer is too large (based on the user
+   controllable setting) then throw an error.  If this function returns
+   then we should attempt to allocate the buffer.  */
+
+static void
+check_type_length_before_alloc (const struct type *type)
+{
+  unsigned int length = TYPE_LENGTH (type);
+
+  if (max_value_size > -1 && length > max_value_size)
+    {
+      if (TYPE_NAME (type) != NULL)
+	error (_("value of type `%s' requires %d bytes, which is more "
+		 "than max-value-size"), TYPE_NAME (type), length);
+      else
+	error (_("value requires %d bytes, which is more than "
+		 "max-value-size"), length);
+    }
+}
+
 /* Allocate the contents of VAL if it has not been allocated yet.  */
 
 static void
 allocate_value_contents (struct value *val)
 {
   if (!val->contents)
-    val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
+    {
+      check_type_length_before_alloc (val->enclosing_type);
+      val->contents
+	= (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
+    }
 }
 
 /* Allocate a  value  and its contents for type TYPE.  */
@@ -2986,9 +3059,12 @@ value_static_field (struct type *type, int fieldno)
 void
 set_value_enclosing_type (struct value *val, struct type *new_encl_type)
 {
-  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val))) 
-    val->contents =
-      (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
+  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val)))
+    {
+      check_type_length_before_alloc (new_encl_type);
+      val->contents
+	= (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
+    }
 
   val->enclosing_type = new_encl_type;
 }
@@ -4013,4 +4089,17 @@ Check whether an expression is void.\n\
 Usage: $_isvoid (expression)\n\
 Return 1 if the expression is void, zero otherwise."),
 			 isvoid_internal_fn, NULL);
+
+  add_setshow_zuinteger_unlimited_cmd ("max-value-size",
+				       class_support, &max_value_size, _("\
+Set maximum sized value gdb will load from the inferior."), _("\
+Show maximum sized value gdb will load from the inferior."), _("\
+Use this to control the maximum size, in bytes, of a value that gdb\n\
+will load from the inferior.  Setting this value to 'unlimited'\n\
+disables checking.\n\
+Setting this does not invalidate already allocated values, it only\n\
+prevents future values, larger than this size, from being allocated."),
+			    set_max_value_size,
+			    show_max_value_size,
+			    &setlist, &showlist);
 }
-- 
2.5.1

PING: Re: [PATCH 1/3] gdb: New set/show max-value-size command.

[Andrew Burgess]

Ping!  I believe I've addressed all review comments.

Thanks,
Andrew

* Andrew Burgess <andrew.burgess@embecosm.com> [2016-01-06 11:40:50 +0000]:

> * Eli Zaretskii <eliz@gnu.org> [2016-01-05 18:24:32 +0200]:
> 
> > > Date: Tue, 5 Jan 2016 14:12:41 +0000
> > > From: Andrew Burgess <andrew.burgess@embecosm.com>
> > > Cc: gdb-patches@sourceware.org
> > > 
> > > For languages with dynamic types, an incorrect program, or uninitialised
> > > variables within a program, could result in an incorrect, overly large
> > > type being associated with a value.  Currently, attempting to print such
> > > a variable will result in gdb trying to allocate an overly large buffer.
> > > 
> > > If this large memory allocation fails then the result can be gdb either
> > > terminating, or (due to memory contention) becoming unresponsive for the
> > > user.
> > > 
> > > A new user visible variable in gdb helps guard against such problems,
> > > two new commands are available:
> > > 
> > >    set max-value-size
> > >    show max-value-size
> > > 
> > > The 'max-value-size' is the maximum size in bytes that gdb will allocate
> > > for the contents of a value.  Any attempt to allocate a value with a
> > > size greater than this will result in an error.  The initial default for
> > > this limit is set at 64k, this is based on a similar limit that exists
> > > within the ada specific code.
> > > 
> > > It is possible for the user to set max-value-size to unlimited, in which
> > > case the old behaviour is restored.
> > 
> > Thanks.
> > 
> > > gdb/doc/ChangeLog:
> > > 
> > > 	* gdb.texinfo (Value Sizes): New section, also add the new section
> > > 	to the menu.
> > 
> > The addition to the menu is probably in a different node, so it needs
> > a separate entry in the ChangeLog.
> > 
> > > +set max-value-size
> > > +show max-value-size
> > > +  Control the maximum size, in bytes, that GDB will allocate for value
> > > +  contents.  Prevent incorrect programs from causing GDB to allocate
> > > +  overly large buffers.  Default is unlimited.
> > 
> > "Controls" and "Prevents".  Also "maximum size of memory" (we allocate
> > memory, not size).
> > 
> > > +@table @code
> > > +@kindex set max-value-size
> > > +@itemx set max-value-size @var{bytes}
> > > +@itemx set max-value-size unlimited
> > > +Set the maximum size, in bytes, that @value{GDBN} will allocate for
> > 
> > Same here:
> > 
> >   Set the maximum size of memory, in bytes, that @value{GDBN} will ...
> > 
> > Also, the "in bytes" part is redundant, since the parameter is called
> > "bytes", which is self-explanatory.
> > 
> > > +the contents of a value to @var{bytes}.  Any value whose contents
> > > +require more than this number of bytes can't be displayed by
> > > +@value{GDBN}, and trying to display the value will result in an error.
> > 
> > I would remove the "can't be displayed by @value{GDBN}" part.  It can
> > be interpreted to mean some limitation inherent in GDB, which is not
> > what you want to convey.  The rest of the sentence says it all: trying
> > to display a value that requires more memory than that will result in
> > an error.
> > 
> > > +Setting this variable does not effect values that have already been
> > > +allocated within gdb, only future allocations.
> >                     ^^^
> > @value{GDBN}
> > 
> > > +There's a minimum size that @code{max-value-size} can be set too in
> >                                                                 ^^^
> > "to"
> > 
> > OK with those fixed.
> 
> New revision resolves all of Eli's comments, and also addresses
> Pedro's comment.
> 
> OK to apply?
> 
> Thanks,
> Andrew
> 
> ---
> 
> For languages with dynamic types, an incorrect program, or uninitialised
> variables within a program, could result in an incorrect, overly large
> type being associated with a value.  Currently, attempting to print such
> a variable will result in gdb trying to allocate an overly large buffer.
> 
> If this large memory allocation fails then the result can be gdb either
> terminating, or (due to memory contention) becoming unresponsive for the
> user.
> 
> A new user visible variable in gdb helps guard against such problems,
> two new commands are available:
> 
>    set max-value-size
>    show max-value-size
> 
> The 'max-value-size' is the maximum size of memory in bytes that gdb
> will allocate for the contents of a value.  Any attempt to allocate a
> value with a size greater than this will result in an error.  The
> initial default for this limit is set at 64k, this is based on a similar
> limit that exists within the ada specific code.
> 
> It is possible for the user to set max-value-size to unlimited, in which
> case the old behaviour is restored.
> 
> gdb/ChangeLog:
> 
> 	* value.c (max_value_size): New variable.
> 	(MIN_VALUE_FOR_MAX_VALUE_SIZE): New define.
> 	(show_max_value_size): New function.
> 	(check_type_length_before_alloc): New function.
> 	(allocate_value_contents): Call check_type_length_before_alloc.
> 	(set_value_enclosing_type): Likewise.
> 	(_initialize_values): Add set/show handler for max-value-size.
> 	* NEWS: Mention new set/show command.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Value Sizes): New section.
> 	(Data): Add the 'Value Sizes' node to the menu.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/max-value-size.c: New file.
> 	* gdb.base/max-value-size.exp: New file.
> ---
>  gdb/ChangeLog                             | 12 ++++
>  gdb/NEWS                                  |  6 ++
>  gdb/doc/ChangeLog                         |  5 ++
>  gdb/doc/gdb.texinfo                       | 42 +++++++++++++
>  gdb/testsuite/ChangeLog                   |  5 ++
>  gdb/testsuite/gdb.base/max-value-size.c   | 26 +++++++++
>  gdb/testsuite/gdb.base/max-value-size.exp | 97 +++++++++++++++++++++++++++++++
>  gdb/value.c                               | 97 +++++++++++++++++++++++++++++--
>  8 files changed, 286 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/max-value-size.c
>  create mode 100644 gdb/testsuite/gdb.base/max-value-size.exp
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 3d8923b..42633d2 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,15 @@
> +2016-01-05  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* value.c (max_value_size): New variable.
> +	(MIN_VALUE_FOR_MAX_VALUE_SIZE): New define.
> +	(set_max_value_size): New function.
> +	(show_max_value_size): New function.
> +	(check_type_length_before_alloc): New function.
> +	(allocate_value_contents): Call check_type_length_before_alloc.
> +	(set_value_enclosing_type): Likewise.
> +	(_initialize_values): Add set/show handler for max-value-size.
> +	* NEWS: Mention new set/show command.
> +
>  2016-01-04  Markus Metzger  <markus.t.metzger@intel.com>
>  
>  	* btrace.c (btrace_pt_readmem_callback): Do not return in TRY/CATCH.
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 484d98d..861b125 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -60,6 +60,12 @@ show ada print-signatures"
>    Control whether parameter types and return types are displayed in overloads
>    selection menus.  It is activaled (@code{on}) by default.
>  
> +set max-value-size
> +show max-value-size
> +  Controls the maximum size of memory, in bytes, that GDB will
> +  allocate for value contents.  Prevents incorrect programs from
> +  causing GDB to allocate overly large buffers.  Default is 64k.
> +
>  * The "disassemble" command accepts a new modifier: /s.
>    It prints mixed source+disassembly like /m with two differences:
>    - disassembled instructions are now printed in program order, and
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index ef3bdaa..97a1b25 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-01-05  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* gdb.texinfo (Value Sizes): New section.
> +	(Data): Add the 'Value Sizes' node to the menu.
> +
>  2015-12-11  Don Breazeal  <donb@codesourcery.com>
>  
>  	* gdb.texinfo (Forks): Correct Linux kernel version where
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 0778383..9dea873 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -8531,6 +8531,7 @@ being passed the type of @var{arg} as the argument.
>                                  character set than GDB does
>  * Caching Target Data::         Data caching for targets
>  * Searching Memory::            Searching memory for a sequence of bytes
> +* Value Sizes::                 Managing memory allocated for values
>  @end menu
>  
>  @node Expressions
> @@ -11596,6 +11597,47 @@ $1 = 1
>  $2 = (void *) 0x8049560
>  @end smallexample
>  
> +@node Value Sizes
> +@section Value Sizes
> +
> +Whenever @value{GDBN} prints a value memory will be allocated within
> +@value{GDBN} to hold the contents of the value.  It is possible in
> +some languages with dynamic typing systems, that an invalid program
> +may indicate a value that is incorrectly large, this in turn may cause
> +@value{GDBN} to try and allocate an overly large ammount of memory.
> +
> +@table @code
> +@kindex set max-value-size
> +@itemx set max-value-size @var{bytes}
> +@itemx set max-value-size unlimited
> +Set the maximum size of memory that @value{GDBN} will allocate for the
> +contents of a value to @var{bytes}, trying to display a value that
> +requires more memory than that will result in an error.
> +
> +Setting this variable does not effect values that have already been
> +allocated within @value{GDBN}, only future allocations.
> +
> +There's a minimum size that @code{max-value-size} can be set to in
> +order that @value{GDBN} can still operate correctly, this minimum is
> +currently 16 bytes.
> +
> +The limit applies to the results of some subexpressions as well as to
> +complete expressions.  For example, an expression denoting a simple
> +integer component, such as @code{x.y.z}, may fail if the size of
> +@var{x.y} is dynamic and exceeds @var{bytes}.  On the other hand,
> +@value{GDBN} is sometimes clever; the expression @code{A[i]}, where
> +@var{A} is an array variable with non-constant size, will generally
> +succeed regardless of the bounds on @var{A}, as long as the component
> +size is less than @var{bytes}.
> +
> +The default value of @code{max-value-size} is currently 64k.
> +
> +@kindex show max-value-size
> +@item show max-value-size
> +Show the maximum size of memory, in bytes, that @value{GDBN} will
> +allocate for the contents of a value.
> +@end table
> +
>  @node Optimized Code
>  @chapter Debugging Optimized Code
>  @cindex optimized code, debugging
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 45a6c6a..b10da8c 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-01-05  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* gdb.base/max-value-size.c: New file.
> +	* gdb.base/max-value-size.exp: New file.
> +
>  2016-01-04  Markus Metzger  <markus.t.metzger@intel.com>
>  
>  	* gdb.btrace/dlopen.exp: New.
> diff --git a/gdb/testsuite/gdb.base/max-value-size.c b/gdb/testsuite/gdb.base/max-value-size.c
> new file mode 100644
> index 0000000..b6a6fe5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/max-value-size.c
> @@ -0,0 +1,26 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright (C) 2016 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/>.  */
> +
> +char one;
> +char ten [10];
> +char one_hundred [100];
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/max-value-size.exp b/gdb/testsuite/gdb.base/max-value-size.exp
> new file mode 100644
> index 0000000..63a97a0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/max-value-size.exp
> @@ -0,0 +1,97 @@
> +# Copyright (C) 2016 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/>.
> +
> +standard_testfile
> +
> +if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
> +    untested $testfile.exp
> +    return -1
> +}
> +
> +if ![runto_main] then {
> +    fail "Can't run to main"
> +    return 0
> +}
> +
> +# Run "show max-value-size" and return the interesting bit of the
> +# result.  This is either the maximum size in bytes, or the string
> +# "unlimited".
> +proc get_max_value_size {} {
> +    global gdb_prompt
> +    global decimal
> +
> +    gdb_test_multiple "show max-value-size" "" {
> +	-re "Maximum value size is ($decimal) bytes.*$gdb_prompt $" {
> +	    return $expect_out(1,string)
> +	}
> +	-re "Maximum value size is unlimited.*$gdb_prompt $" {
> +	    return "unlimited"
> +	}
> +    }
> +}
> +
> +# Assuming that MAX_VALUE_SIZE is the current value for
> +# max-value-size, print the test values.  Use TEST_PREFIX to make the
> +# test names unique.
> +proc do_value_printing { max_value_size test_prefix } {
> +    with_test_prefix ${test_prefix} {
> +	gdb_test "p/d one" " = 0"
> +	if { $max_value_size != "unlimited" && $max_value_size < 100 } then {
> +	    gdb_test "p/d one_hundred" \
> +		"value requires 100 bytes, which is more than max-value-size"
> +	} else {
> +	    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
> +	}
> +	gdb_test "p/d one_hundred \[99\]" " = 0"
> +    }
> +}
> +
> +# Install SET_VALUE as the value for max-value-size, then print the
> +# test values.
> +proc set_and_check_max_value_size { set_value } {
> +    if { $set_value == "unlimited" } then {
> +	set check_pattern "unlimited"
> +    } else {
> +	set check_pattern "${set_value} bytes"
> +    }
> +
> +    gdb_test_no_output "set max-value-size ${set_value}"
> +    gdb_test "show max-value-size" \
> +	"Maximum value size is ${check_pattern}." \
> +	"check that the value shows as ${check_pattern}"
> +
> +    do_value_printing ${set_value} "max-value-size is '${set_value}'"
> +}
> +
> +# Check the default value is sufficient.
> +do_value_printing [get_max_value_size] "using initial max-value-size"
> +
> +# Check some values for max-value-size that should prevent some
> +# allocations.
> +set_and_check_max_value_size 25
> +set_and_check_max_value_size 99
> +
> +# Check values for max-value-size that should allow all allocations.
> +set_and_check_max_value_size 100
> +set_and_check_max_value_size 200
> +set_and_check_max_value_size "unlimited"
> +
> +# Check that we can't set the maximum size stupidly low.
> +gdb_test "set max-value-size 1" \
> +    "max-value-size set too low, increasing to \[0-9\]+ bytes"
> +gdb_test "set max-value-size 0" \
> +    "max-value-size set too low, increasing to \[0-9\]+ bytes"
> +gdb_test "set max-value-size -5" \
> +    "only -1 is allowed to set as unlimited"
> diff --git a/gdb/value.c b/gdb/value.c
> index eeb2b7d..41f44c8 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -955,13 +955,86 @@ allocate_value_lazy (struct type *type)
>    return val;
>  }
>  
> +/* The maximum size, in bytes, that GDB will try to allocate for a value.
> +   The initial value of 64k was not selected for any specific reason, it is
> +   just a reasonable starting point.  */
> +
> +static int max_value_size = 65536; /* 64k bytes */
> +
> +/* It is critical that the MAX_VALUE_SIZE is at least as bit as the size of
> +   LONGEST, otherwise GDB will not be able to parse integer values from the
> +   CLI; for example if the MAX_VALUE_SIZE could be set to 1 then GDB would
> +   be unable to parse "set max-value-size 2".
> +
> +   As we want a consistent GDB experience across hosts with different sizes
> +   of LONGEST, this arbitrary minimum value was selected, so long as this
> +   is bigger than LONGEST of all GDB supported hosts we're fine.  */
> +
> +#define MIN_VALUE_FOR_MAX_VALUE_SIZE 16
> +gdb_static_assert (sizeof (LONGEST) <= MIN_VALUE_FOR_MAX_VALUE_SIZE);
> +
> +/* Implement the "set max-value-size" command.  */
> +
> +static void
> +set_max_value_size (char *args, int from_tty,
> +		    struct cmd_list_element *c)
> +{
> +  gdb_assert (max_value_size == -1 || max_value_size >= 0);
> +
> +  if (max_value_size > -1 && max_value_size < MIN_VALUE_FOR_MAX_VALUE_SIZE)
> +    {
> +      max_value_size = MIN_VALUE_FOR_MAX_VALUE_SIZE;
> +      error (_("max-value-size set too low, increasing to %d bytes"),
> +	     max_value_size);
> +    }
> +}
> +
> +/* Implement the "show max-value-size" command.  */
> +
> +static void
> +show_max_value_size (struct ui_file *file, int from_tty,
> +		     struct cmd_list_element *c, const char *value)
> +{
> +  if (max_value_size == -1)
> +    fprintf_filtered (file, _("Maximum value size is unlimited.\n"));
> +  else
> +    fprintf_filtered (file, _("Maximum value size is %d bytes.\n"),
> +		      max_value_size);
> +}
> +
> +/* Called before we attempt to allocate or reallocate a buffer for the
> +   contents of a value.  TYPE is the type of the value for which we are
> +   allocating the buffer.  If the buffer is too large (based on the user
> +   controllable setting) then throw an error.  If this function returns
> +   then we should attempt to allocate the buffer.  */
> +
> +static void
> +check_type_length_before_alloc (const struct type *type)
> +{
> +  unsigned int length = TYPE_LENGTH (type);
> +
> +  if (max_value_size > -1 && length > max_value_size)
> +    {
> +      if (TYPE_NAME (type) != NULL)
> +	error (_("value of type `%s' requires %d bytes, which is more "
> +		 "than max-value-size"), TYPE_NAME (type), length);
> +      else
> +	error (_("value requires %d bytes, which is more than "
> +		 "max-value-size"), length);
> +    }
> +}
> +
>  /* Allocate the contents of VAL if it has not been allocated yet.  */
>  
>  static void
>  allocate_value_contents (struct value *val)
>  {
>    if (!val->contents)
> -    val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
> +    {
> +      check_type_length_before_alloc (val->enclosing_type);
> +      val->contents
> +	= (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
> +    }
>  }
>  
>  /* Allocate a  value  and its contents for type TYPE.  */
> @@ -2986,9 +3059,12 @@ value_static_field (struct type *type, int fieldno)
>  void
>  set_value_enclosing_type (struct value *val, struct type *new_encl_type)
>  {
> -  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val))) 
> -    val->contents =
> -      (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
> +  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val)))
> +    {
> +      check_type_length_before_alloc (new_encl_type);
> +      val->contents
> +	= (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
> +    }
>  
>    val->enclosing_type = new_encl_type;
>  }
> @@ -4013,4 +4089,17 @@ Check whether an expression is void.\n\
>  Usage: $_isvoid (expression)\n\
>  Return 1 if the expression is void, zero otherwise."),
>  			 isvoid_internal_fn, NULL);
> +
> +  add_setshow_zuinteger_unlimited_cmd ("max-value-size",
> +				       class_support, &max_value_size, _("\
> +Set maximum sized value gdb will load from the inferior."), _("\
> +Show maximum sized value gdb will load from the inferior."), _("\
> +Use this to control the maximum size, in bytes, of a value that gdb\n\
> +will load from the inferior.  Setting this value to 'unlimited'\n\
> +disables checking.\n\
> +Setting this does not invalidate already allocated values, it only\n\
> +prevents future values, larger than this size, from being allocated."),
> +			    set_max_value_size,
> +			    show_max_value_size,
> +			    &setlist, &showlist);
>  }
> -- 
> 2.5.1
> 

Re: PING: Re: [PATCH 1/3] gdb: New set/show max-value-size command.

[Eli Zaretskii]

> Date: Wed, 20 Jan 2016 11:59:21 +0100
> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: brobecker@adacore.com, Pedro Alves <palves@redhat.com>,
> 	Eli Zaretskii <eliz@gnu.org>
> 
> Ping!  I believe I've addressed all review comments.

I said "OK with those fixed", which means you have my approval for the
documentation parts as soon as you address the comments.

Thanks.

Re: PING: Re: [PATCH 1/3] gdb: New set/show max-value-size command.

[Andrew Burgess]

* Andrew Burgess <andrew.burgess@embecosm.com> [2016-01-20 11:59:21 +0100]:

> Ping!  I believe I've addressed all review comments.

Following up on this again, I have another version of this patch below
which has two changes:

  - Switch from using '%d' to '%u' to display an unsigned int in the
    error message..

  - Set max-value-size to 'unlimited' for the test gdb.base/huge.exp.

I've tested this patch, and patch 3/3 [1] against the gdb testsuite on
x86-64 linux, on a machine that has gfortran 5.3.1 installed.  I do
see the max-value-size exceeded error message crop up in 6 fortran
tests, though I think that these are all legitimate errors,
representing undefined, potentially unsafe behaviour being prevented.

[1]  https://sourceware.org/ml/gdb-patches/2015-12/msg00247.html)

Thanks,
Andrew

---

For languages with dynamic types, an incorrect program, or uninitialised
variables within a program, could result in an incorrect, overly large
type being associated with a value.  Currently, attempting to print such
a variable will result in gdb trying to allocate an overly large buffer.

If this large memory allocation fails then the result can be gdb either
terminating, or (due to memory contention) becoming unresponsive for the
user.

A new user visible variable in gdb helps guard against such problems,
two new commands are available:

   set max-value-size
   show max-value-size

The 'max-value-size' is the maximum size of memory in bytes that gdb
will allocate for the contents of a value.  Any attempt to allocate a
value with a size greater than this will result in an error.  The
initial default for this limit is set at 64k, this is based on a similar
limit that exists within the ada specific code.

It is possible for the user to set max-value-size to unlimited, in which
case the old behaviour is restored.

gdb/ChangeLog:

	* value.c (max_value_size): New variable.
	(MIN_VALUE_FOR_MAX_VALUE_SIZE): New define.
	(show_max_value_size): New function.
	(check_type_length_before_alloc): New function.
	(allocate_value_contents): Call check_type_length_before_alloc.
	(set_value_enclosing_type): Likewise.
	(_initialize_values): Add set/show handler for max-value-size.
	* NEWS: Mention new set/show command.

gdb/doc/ChangeLog:

	* gdb.texinfo (Value Sizes): New section.
	(Data): Add the 'Value Sizes' node to the menu.

gdb/testsuite/ChangeLog:

	* gdb.base/max-value-size.c: New file.
	* gdb.base/max-value-size.exp: New file.
	* gdb.base/huge.exp: Disable max-value-size for this test.
---
 gdb/ChangeLog                             | 12 ++++
 gdb/NEWS                                  |  6 ++
 gdb/doc/ChangeLog                         |  5 ++
 gdb/doc/gdb.texinfo                       | 42 +++++++++++++
 gdb/testsuite/ChangeLog                   |  6 ++
 gdb/testsuite/gdb.base/huge.exp           |  2 +
 gdb/testsuite/gdb.base/max-value-size.c   | 26 +++++++++
 gdb/testsuite/gdb.base/max-value-size.exp | 97 +++++++++++++++++++++++++++++++
 gdb/value.c                               | 97 +++++++++++++++++++++++++++++--
 9 files changed, 289 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/max-value-size.c
 create mode 100644 gdb/testsuite/gdb.base/max-value-size.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d1a6069..5cb98fb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2016-01-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* value.c (max_value_size): New variable.
+	(MIN_VALUE_FOR_MAX_VALUE_SIZE): New define.
+	(set_max_value_size): New function.
+	(show_max_value_size): New function.
+	(check_type_length_before_alloc): New function.
+	(allocate_value_contents): Call check_type_length_before_alloc.
+	(set_value_enclosing_type): Likewise.
+	(_initialize_values): Add set/show handler for max-value-size.
+	* NEWS: Mention new set/show command.
+
 2016-01-20  Joel Brobecker  <brobecker@adacore.com>
 
 	* printcmd.c (print_scalar_formatted): move binary operator from
diff --git a/gdb/NEWS b/gdb/NEWS
index 4312117..962eae4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -119,6 +119,12 @@ show ada print-signatures"
   Control whether parameter types and return types are displayed in overloads
   selection menus.  It is activaled (@code{on}) by default.
 
+set max-value-size
+show max-value-size
+  Controls the maximum size of memory, in bytes, that GDB will
+  allocate for value contents.  Prevents incorrect programs from
+  causing GDB to allocate overly large buffers.  Default is 64k.
+
 * The "disassemble" command accepts a new modifier: /s.
   It prints mixed source+disassembly like /m with two differences:
   - disassembled instructions are now printed in program order, and
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 990f2ec..83710c0 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2016-01-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.texinfo (Value Sizes): New section.
+	(Data): Add the 'Value Sizes' node to the menu.
+
 2016-01-19  John Baldwin  <jhb@FreeBSD.org>
 
 	* gdb.texinfo (Debugging Output): Document "set/show debug fbsd-lwp".
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index cd311db..27db877 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -8636,6 +8636,7 @@ being passed the type of @var{arg} as the argument.
                                 character set than GDB does
 * Caching Target Data::         Data caching for targets
 * Searching Memory::            Searching memory for a sequence of bytes
+* Value Sizes::                 Managing memory allocated for values
 @end menu
 
 @node Expressions
@@ -11711,6 +11712,47 @@ $1 = 1
 $2 = (void *) 0x8049560
 @end smallexample
 
+@node Value Sizes
+@section Value Sizes
+
+Whenever @value{GDBN} prints a value memory will be allocated within
+@value{GDBN} to hold the contents of the value.  It is possible in
+some languages with dynamic typing systems, that an invalid program
+may indicate a value that is incorrectly large, this in turn may cause
+@value{GDBN} to try and allocate an overly large ammount of memory.
+
+@table @code
+@kindex set max-value-size
+@itemx set max-value-size @var{bytes}
+@itemx set max-value-size unlimited
+Set the maximum size of memory that @value{GDBN} will allocate for the
+contents of a value to @var{bytes}, trying to display a value that
+requires more memory than that will result in an error.
+
+Setting this variable does not effect values that have already been
+allocated within @value{GDBN}, only future allocations.
+
+There's a minimum size that @code{max-value-size} can be set to in
+order that @value{GDBN} can still operate correctly, this minimum is
+currently 16 bytes.
+
+The limit applies to the results of some subexpressions as well as to
+complete expressions.  For example, an expression denoting a simple
+integer component, such as @code{x.y.z}, may fail if the size of
+@var{x.y} is dynamic and exceeds @var{bytes}.  On the other hand,
+@value{GDBN} is sometimes clever; the expression @code{A[i]}, where
+@var{A} is an array variable with non-constant size, will generally
+succeed regardless of the bounds on @var{A}, as long as the component
+size is less than @var{bytes}.
+
+The default value of @code{max-value-size} is currently 64k.
+
+@kindex show max-value-size
+@item show max-value-size
+Show the maximum size of memory, in bytes, that @value{GDBN} will
+allocate for the contents of a value.
+@end table
+
 @node Optimized Code
 @chapter Debugging Optimized Code
 @cindex optimized code, debugging
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b96a4ed..f440ef4 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2016-01-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.base/max-value-size.c: New file.
+	* gdb.base/max-value-size.exp: New file.
+	* gdb.base/huge.exp: Disable max-value-size for this test.
+
 2016-01-19  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* Makefile.in (DO_RUNTEST): Add --status and update usages.
diff --git a/gdb/testsuite/gdb.base/huge.exp b/gdb/testsuite/gdb.base/huge.exp
index 018f305..4dca89b 100644
--- a/gdb/testsuite/gdb.base/huge.exp
+++ b/gdb/testsuite/gdb.base/huge.exp
@@ -47,6 +47,8 @@ if { ! [ runto_main ] } then {
     return -1
 }
 
+gdb_test_no_output "set max-value-size unlimited"
+
 gdb_test "print a" ".1 = .0 .repeats \[0123456789\]+ times.." "print a very large data object"
 
 set timeout $prev_timeout
diff --git a/gdb/testsuite/gdb.base/max-value-size.c b/gdb/testsuite/gdb.base/max-value-size.c
new file mode 100644
index 0000000..b6a6fe5
--- /dev/null
+++ b/gdb/testsuite/gdb.base/max-value-size.c
@@ -0,0 +1,26 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2016 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/>.  */
+
+char one;
+char ten [10];
+char one_hundred [100];
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/max-value-size.exp b/gdb/testsuite/gdb.base/max-value-size.exp
new file mode 100644
index 0000000..63a97a0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/max-value-size.exp
@@ -0,0 +1,97 @@
+# Copyright (C) 2016 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/>.
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+    untested $testfile.exp
+    return -1
+}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 0
+}
+
+# Run "show max-value-size" and return the interesting bit of the
+# result.  This is either the maximum size in bytes, or the string
+# "unlimited".
+proc get_max_value_size {} {
+    global gdb_prompt
+    global decimal
+
+    gdb_test_multiple "show max-value-size" "" {
+	-re "Maximum value size is ($decimal) bytes.*$gdb_prompt $" {
+	    return $expect_out(1,string)
+	}
+	-re "Maximum value size is unlimited.*$gdb_prompt $" {
+	    return "unlimited"
+	}
+    }
+}
+
+# Assuming that MAX_VALUE_SIZE is the current value for
+# max-value-size, print the test values.  Use TEST_PREFIX to make the
+# test names unique.
+proc do_value_printing { max_value_size test_prefix } {
+    with_test_prefix ${test_prefix} {
+	gdb_test "p/d one" " = 0"
+	if { $max_value_size != "unlimited" && $max_value_size < 100 } then {
+	    gdb_test "p/d one_hundred" \
+		"value requires 100 bytes, which is more than max-value-size"
+	} else {
+	    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
+	}
+	gdb_test "p/d one_hundred \[99\]" " = 0"
+    }
+}
+
+# Install SET_VALUE as the value for max-value-size, then print the
+# test values.
+proc set_and_check_max_value_size { set_value } {
+    if { $set_value == "unlimited" } then {
+	set check_pattern "unlimited"
+    } else {
+	set check_pattern "${set_value} bytes"
+    }
+
+    gdb_test_no_output "set max-value-size ${set_value}"
+    gdb_test "show max-value-size" \
+	"Maximum value size is ${check_pattern}." \
+	"check that the value shows as ${check_pattern}"
+
+    do_value_printing ${set_value} "max-value-size is '${set_value}'"
+}
+
+# Check the default value is sufficient.
+do_value_printing [get_max_value_size] "using initial max-value-size"
+
+# Check some values for max-value-size that should prevent some
+# allocations.
+set_and_check_max_value_size 25
+set_and_check_max_value_size 99
+
+# Check values for max-value-size that should allow all allocations.
+set_and_check_max_value_size 100
+set_and_check_max_value_size 200
+set_and_check_max_value_size "unlimited"
+
+# Check that we can't set the maximum size stupidly low.
+gdb_test "set max-value-size 1" \
+    "max-value-size set too low, increasing to \[0-9\]+ bytes"
+gdb_test "set max-value-size 0" \
+    "max-value-size set too low, increasing to \[0-9\]+ bytes"
+gdb_test "set max-value-size -5" \
+    "only -1 is allowed to set as unlimited"
diff --git a/gdb/value.c b/gdb/value.c
index eeb2b7d..aa551ae 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -955,13 +955,86 @@ allocate_value_lazy (struct type *type)
   return val;
 }
 
+/* The maximum size, in bytes, that GDB will try to allocate for a value.
+   The initial value of 64k was not selected for any specific reason, it is
+   just a reasonable starting point.  */
+
+static int max_value_size = 65536; /* 64k bytes */
+
+/* It is critical that the MAX_VALUE_SIZE is at least as bit as the size of
+   LONGEST, otherwise GDB will not be able to parse integer values from the
+   CLI; for example if the MAX_VALUE_SIZE could be set to 1 then GDB would
+   be unable to parse "set max-value-size 2".
+
+   As we want a consistent GDB experience across hosts with different sizes
+   of LONGEST, this arbitrary minimum value was selected, so long as this
+   is bigger than LONGEST of all GDB supported hosts we're fine.  */
+
+#define MIN_VALUE_FOR_MAX_VALUE_SIZE 16
+gdb_static_assert (sizeof (LONGEST) <= MIN_VALUE_FOR_MAX_VALUE_SIZE);
+
+/* Implement the "set max-value-size" command.  */
+
+static void
+set_max_value_size (char *args, int from_tty,
+		    struct cmd_list_element *c)
+{
+  gdb_assert (max_value_size == -1 || max_value_size >= 0);
+
+  if (max_value_size > -1 && max_value_size < MIN_VALUE_FOR_MAX_VALUE_SIZE)
+    {
+      max_value_size = MIN_VALUE_FOR_MAX_VALUE_SIZE;
+      error (_("max-value-size set too low, increasing to %d bytes"),
+	     max_value_size);
+    }
+}
+
+/* Implement the "show max-value-size" command.  */
+
+static void
+show_max_value_size (struct ui_file *file, int from_tty,
+		     struct cmd_list_element *c, const char *value)
+{
+  if (max_value_size == -1)
+    fprintf_filtered (file, _("Maximum value size is unlimited.\n"));
+  else
+    fprintf_filtered (file, _("Maximum value size is %d bytes.\n"),
+		      max_value_size);
+}
+
+/* Called before we attempt to allocate or reallocate a buffer for the
+   contents of a value.  TYPE is the type of the value for which we are
+   allocating the buffer.  If the buffer is too large (based on the user
+   controllable setting) then throw an error.  If this function returns
+   then we should attempt to allocate the buffer.  */
+
+static void
+check_type_length_before_alloc (const struct type *type)
+{
+  unsigned int length = TYPE_LENGTH (type);
+
+  if (max_value_size > -1 && length > max_value_size)
+    {
+      if (TYPE_NAME (type) != NULL)
+	error (_("value of type `%s' requires %u bytes, which is more "
+		 "than max-value-size"), TYPE_NAME (type), length);
+      else
+	error (_("value requires %u bytes, which is more than "
+		 "max-value-size"), length);
+    }
+}
+
 /* Allocate the contents of VAL if it has not been allocated yet.  */
 
 static void
 allocate_value_contents (struct value *val)
 {
   if (!val->contents)
-    val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
+    {
+      check_type_length_before_alloc (val->enclosing_type);
+      val->contents
+	= (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
+    }
 }
 
 /* Allocate a  value  and its contents for type TYPE.  */
@@ -2986,9 +3059,12 @@ value_static_field (struct type *type, int fieldno)
 void
 set_value_enclosing_type (struct value *val, struct type *new_encl_type)
 {
-  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val))) 
-    val->contents =
-      (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
+  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val)))
+    {
+      check_type_length_before_alloc (new_encl_type);
+      val->contents
+	= (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
+    }
 
   val->enclosing_type = new_encl_type;
 }
@@ -4013,4 +4089,17 @@ Check whether an expression is void.\n\
 Usage: $_isvoid (expression)\n\
 Return 1 if the expression is void, zero otherwise."),
 			 isvoid_internal_fn, NULL);
+
+  add_setshow_zuinteger_unlimited_cmd ("max-value-size",
+				       class_support, &max_value_size, _("\
+Set maximum sized value gdb will load from the inferior."), _("\
+Show maximum sized value gdb will load from the inferior."), _("\
+Use this to control the maximum size, in bytes, of a value that gdb\n\
+will load from the inferior.  Setting this value to 'unlimited'\n\
+disables checking.\n\
+Setting this does not invalidate already allocated values, it only\n\
+prevents future values, larger than this size, from being allocated."),
+			    set_max_value_size,
+			    show_max_value_size,
+			    &setlist, &showlist);
 }
-- 
2.5.1

PING #2: Re: [PATCH 1/3] gdb: New set/show max-value-size command.

[Andrew Burgess]

* Andrew Burgess <andrew.burgess@embecosm.com> [2016-01-20 16:22:57 +0100]:

> * Andrew Burgess <andrew.burgess@embecosm.com> [2016-01-20 11:59:21 +0100]:
> 
> > Ping!  I believe I've addressed all review comments.
> 
> Following up on this again, I have another version of this patch below
> which has two changes:
> 
>   - Switch from using '%d' to '%u' to display an unsigned int in the
>     error message..
> 
>   - Set max-value-size to 'unlimited' for the test gdb.base/huge.exp.
> 
> I've tested this patch, and patch 3/3 [1] against the gdb testsuite on
> x86-64 linux, on a machine that has gfortran 5.3.1 installed.  I do
> see the max-value-size exceeded error message crop up in 6 fortran
> tests, though I think that these are all legitimate errors,
> representing undefined, potentially unsafe behaviour being prevented.
> 
> [1]  https://sourceware.org/ml/gdb-patches/2015-12/msg00247.html)
>

I've had doc approval from Eli, and I believe I've addresses all of
the technical points raised by Pedro and Joel, I'm just hoping for
approval to merge this change.

Thanks,
Andrew




> Thanks,
> Andrew
> 
> ---
> 
> For languages with dynamic types, an incorrect program, or uninitialised
> variables within a program, could result in an incorrect, overly large
> type being associated with a value.  Currently, attempting to print such
> a variable will result in gdb trying to allocate an overly large buffer.
> 
> If this large memory allocation fails then the result can be gdb either
> terminating, or (due to memory contention) becoming unresponsive for the
> user.
> 
> A new user visible variable in gdb helps guard against such problems,
> two new commands are available:
> 
>    set max-value-size
>    show max-value-size
> 
> The 'max-value-size' is the maximum size of memory in bytes that gdb
> will allocate for the contents of a value.  Any attempt to allocate a
> value with a size greater than this will result in an error.  The
> initial default for this limit is set at 64k, this is based on a similar
> limit that exists within the ada specific code.
> 
> It is possible for the user to set max-value-size to unlimited, in which
> case the old behaviour is restored.
> 
> gdb/ChangeLog:
> 
> 	* value.c (max_value_size): New variable.
> 	(MIN_VALUE_FOR_MAX_VALUE_SIZE): New define.
> 	(show_max_value_size): New function.
> 	(check_type_length_before_alloc): New function.
> 	(allocate_value_contents): Call check_type_length_before_alloc.
> 	(set_value_enclosing_type): Likewise.
> 	(_initialize_values): Add set/show handler for max-value-size.
> 	* NEWS: Mention new set/show command.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Value Sizes): New section.
> 	(Data): Add the 'Value Sizes' node to the menu.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/max-value-size.c: New file.
> 	* gdb.base/max-value-size.exp: New file.
> 	* gdb.base/huge.exp: Disable max-value-size for this test.
> ---
>  gdb/ChangeLog                             | 12 ++++
>  gdb/NEWS                                  |  6 ++
>  gdb/doc/ChangeLog                         |  5 ++
>  gdb/doc/gdb.texinfo                       | 42 +++++++++++++
>  gdb/testsuite/ChangeLog                   |  6 ++
>  gdb/testsuite/gdb.base/huge.exp           |  2 +
>  gdb/testsuite/gdb.base/max-value-size.c   | 26 +++++++++
>  gdb/testsuite/gdb.base/max-value-size.exp | 97 +++++++++++++++++++++++++++++++
>  gdb/value.c                               | 97 +++++++++++++++++++++++++++++--
>  9 files changed, 289 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/max-value-size.c
>  create mode 100644 gdb/testsuite/gdb.base/max-value-size.exp
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index d1a6069..5cb98fb 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,15 @@
> +2016-01-20  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* value.c (max_value_size): New variable.
> +	(MIN_VALUE_FOR_MAX_VALUE_SIZE): New define.
> +	(set_max_value_size): New function.
> +	(show_max_value_size): New function.
> +	(check_type_length_before_alloc): New function.
> +	(allocate_value_contents): Call check_type_length_before_alloc.
> +	(set_value_enclosing_type): Likewise.
> +	(_initialize_values): Add set/show handler for max-value-size.
> +	* NEWS: Mention new set/show command.
> +
>  2016-01-20  Joel Brobecker  <brobecker@adacore.com>
>  
>  	* printcmd.c (print_scalar_formatted): move binary operator from
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 4312117..962eae4 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -119,6 +119,12 @@ show ada print-signatures"
>    Control whether parameter types and return types are displayed in overloads
>    selection menus.  It is activaled (@code{on}) by default.
>  
> +set max-value-size
> +show max-value-size
> +  Controls the maximum size of memory, in bytes, that GDB will
> +  allocate for value contents.  Prevents incorrect programs from
> +  causing GDB to allocate overly large buffers.  Default is 64k.
> +
>  * The "disassemble" command accepts a new modifier: /s.
>    It prints mixed source+disassembly like /m with two differences:
>    - disassembled instructions are now printed in program order, and
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 990f2ec..83710c0 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-01-20  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* gdb.texinfo (Value Sizes): New section.
> +	(Data): Add the 'Value Sizes' node to the menu.
> +
>  2016-01-19  John Baldwin  <jhb@FreeBSD.org>
>  
>  	* gdb.texinfo (Debugging Output): Document "set/show debug fbsd-lwp".
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index cd311db..27db877 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -8636,6 +8636,7 @@ being passed the type of @var{arg} as the argument.
>                                  character set than GDB does
>  * Caching Target Data::         Data caching for targets
>  * Searching Memory::            Searching memory for a sequence of bytes
> +* Value Sizes::                 Managing memory allocated for values
>  @end menu
>  
>  @node Expressions
> @@ -11711,6 +11712,47 @@ $1 = 1
>  $2 = (void *) 0x8049560
>  @end smallexample
>  
> +@node Value Sizes
> +@section Value Sizes
> +
> +Whenever @value{GDBN} prints a value memory will be allocated within
> +@value{GDBN} to hold the contents of the value.  It is possible in
> +some languages with dynamic typing systems, that an invalid program
> +may indicate a value that is incorrectly large, this in turn may cause
> +@value{GDBN} to try and allocate an overly large ammount of memory.
> +
> +@table @code
> +@kindex set max-value-size
> +@itemx set max-value-size @var{bytes}
> +@itemx set max-value-size unlimited
> +Set the maximum size of memory that @value{GDBN} will allocate for the
> +contents of a value to @var{bytes}, trying to display a value that
> +requires more memory than that will result in an error.
> +
> +Setting this variable does not effect values that have already been
> +allocated within @value{GDBN}, only future allocations.
> +
> +There's a minimum size that @code{max-value-size} can be set to in
> +order that @value{GDBN} can still operate correctly, this minimum is
> +currently 16 bytes.
> +
> +The limit applies to the results of some subexpressions as well as to
> +complete expressions.  For example, an expression denoting a simple
> +integer component, such as @code{x.y.z}, may fail if the size of
> +@var{x.y} is dynamic and exceeds @var{bytes}.  On the other hand,
> +@value{GDBN} is sometimes clever; the expression @code{A[i]}, where
> +@var{A} is an array variable with non-constant size, will generally
> +succeed regardless of the bounds on @var{A}, as long as the component
> +size is less than @var{bytes}.
> +
> +The default value of @code{max-value-size} is currently 64k.
> +
> +@kindex show max-value-size
> +@item show max-value-size
> +Show the maximum size of memory, in bytes, that @value{GDBN} will
> +allocate for the contents of a value.
> +@end table
> +
>  @node Optimized Code
>  @chapter Debugging Optimized Code
>  @cindex optimized code, debugging
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index b96a4ed..f440ef4 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2016-01-20  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* gdb.base/max-value-size.c: New file.
> +	* gdb.base/max-value-size.exp: New file.
> +	* gdb.base/huge.exp: Disable max-value-size for this test.
> +
>  2016-01-19  Simon Marchi  <simon.marchi@ericsson.com>
>  
>  	* Makefile.in (DO_RUNTEST): Add --status and update usages.
> diff --git a/gdb/testsuite/gdb.base/huge.exp b/gdb/testsuite/gdb.base/huge.exp
> index 018f305..4dca89b 100644
> --- a/gdb/testsuite/gdb.base/huge.exp
> +++ b/gdb/testsuite/gdb.base/huge.exp
> @@ -47,6 +47,8 @@ if { ! [ runto_main ] } then {
>      return -1
>  }
>  
> +gdb_test_no_output "set max-value-size unlimited"
> +
>  gdb_test "print a" ".1 = .0 .repeats \[0123456789\]+ times.." "print a very large data object"
>  
>  set timeout $prev_timeout
> diff --git a/gdb/testsuite/gdb.base/max-value-size.c b/gdb/testsuite/gdb.base/max-value-size.c
> new file mode 100644
> index 0000000..b6a6fe5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/max-value-size.c
> @@ -0,0 +1,26 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright (C) 2016 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/>.  */
> +
> +char one;
> +char ten [10];
> +char one_hundred [100];
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/max-value-size.exp b/gdb/testsuite/gdb.base/max-value-size.exp
> new file mode 100644
> index 0000000..63a97a0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/max-value-size.exp
> @@ -0,0 +1,97 @@
> +# Copyright (C) 2016 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/>.
> +
> +standard_testfile
> +
> +if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
> +    untested $testfile.exp
> +    return -1
> +}
> +
> +if ![runto_main] then {
> +    fail "Can't run to main"
> +    return 0
> +}
> +
> +# Run "show max-value-size" and return the interesting bit of the
> +# result.  This is either the maximum size in bytes, or the string
> +# "unlimited".
> +proc get_max_value_size {} {
> +    global gdb_prompt
> +    global decimal
> +
> +    gdb_test_multiple "show max-value-size" "" {
> +	-re "Maximum value size is ($decimal) bytes.*$gdb_prompt $" {
> +	    return $expect_out(1,string)
> +	}
> +	-re "Maximum value size is unlimited.*$gdb_prompt $" {
> +	    return "unlimited"
> +	}
> +    }
> +}
> +
> +# Assuming that MAX_VALUE_SIZE is the current value for
> +# max-value-size, print the test values.  Use TEST_PREFIX to make the
> +# test names unique.
> +proc do_value_printing { max_value_size test_prefix } {
> +    with_test_prefix ${test_prefix} {
> +	gdb_test "p/d one" " = 0"
> +	if { $max_value_size != "unlimited" && $max_value_size < 100 } then {
> +	    gdb_test "p/d one_hundred" \
> +		"value requires 100 bytes, which is more than max-value-size"
> +	} else {
> +	    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
> +	}
> +	gdb_test "p/d one_hundred \[99\]" " = 0"
> +    }
> +}
> +
> +# Install SET_VALUE as the value for max-value-size, then print the
> +# test values.
> +proc set_and_check_max_value_size { set_value } {
> +    if { $set_value == "unlimited" } then {
> +	set check_pattern "unlimited"
> +    } else {
> +	set check_pattern "${set_value} bytes"
> +    }
> +
> +    gdb_test_no_output "set max-value-size ${set_value}"
> +    gdb_test "show max-value-size" \
> +	"Maximum value size is ${check_pattern}." \
> +	"check that the value shows as ${check_pattern}"
> +
> +    do_value_printing ${set_value} "max-value-size is '${set_value}'"
> +}
> +
> +# Check the default value is sufficient.
> +do_value_printing [get_max_value_size] "using initial max-value-size"
> +
> +# Check some values for max-value-size that should prevent some
> +# allocations.
> +set_and_check_max_value_size 25
> +set_and_check_max_value_size 99
> +
> +# Check values for max-value-size that should allow all allocations.
> +set_and_check_max_value_size 100
> +set_and_check_max_value_size 200
> +set_and_check_max_value_size "unlimited"
> +
> +# Check that we can't set the maximum size stupidly low.
> +gdb_test "set max-value-size 1" \
> +    "max-value-size set too low, increasing to \[0-9\]+ bytes"
> +gdb_test "set max-value-size 0" \
> +    "max-value-size set too low, increasing to \[0-9\]+ bytes"
> +gdb_test "set max-value-size -5" \
> +    "only -1 is allowed to set as unlimited"
> diff --git a/gdb/value.c b/gdb/value.c
> index eeb2b7d..aa551ae 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -955,13 +955,86 @@ allocate_value_lazy (struct type *type)
>    return val;
>  }
>  
> +/* The maximum size, in bytes, that GDB will try to allocate for a value.
> +   The initial value of 64k was not selected for any specific reason, it is
> +   just a reasonable starting point.  */
> +
> +static int max_value_size = 65536; /* 64k bytes */
> +
> +/* It is critical that the MAX_VALUE_SIZE is at least as bit as the size of
> +   LONGEST, otherwise GDB will not be able to parse integer values from the
> +   CLI; for example if the MAX_VALUE_SIZE could be set to 1 then GDB would
> +   be unable to parse "set max-value-size 2".
> +
> +   As we want a consistent GDB experience across hosts with different sizes
> +   of LONGEST, this arbitrary minimum value was selected, so long as this
> +   is bigger than LONGEST of all GDB supported hosts we're fine.  */
> +
> +#define MIN_VALUE_FOR_MAX_VALUE_SIZE 16
> +gdb_static_assert (sizeof (LONGEST) <= MIN_VALUE_FOR_MAX_VALUE_SIZE);
> +
> +/* Implement the "set max-value-size" command.  */
> +
> +static void
> +set_max_value_size (char *args, int from_tty,
> +		    struct cmd_list_element *c)
> +{
> +  gdb_assert (max_value_size == -1 || max_value_size >= 0);
> +
> +  if (max_value_size > -1 && max_value_size < MIN_VALUE_FOR_MAX_VALUE_SIZE)
> +    {
> +      max_value_size = MIN_VALUE_FOR_MAX_VALUE_SIZE;
> +      error (_("max-value-size set too low, increasing to %d bytes"),
> +	     max_value_size);
> +    }
> +}
> +
> +/* Implement the "show max-value-size" command.  */
> +
> +static void
> +show_max_value_size (struct ui_file *file, int from_tty,
> +		     struct cmd_list_element *c, const char *value)
> +{
> +  if (max_value_size == -1)
> +    fprintf_filtered (file, _("Maximum value size is unlimited.\n"));
> +  else
> +    fprintf_filtered (file, _("Maximum value size is %d bytes.\n"),
> +		      max_value_size);
> +}
> +
> +/* Called before we attempt to allocate or reallocate a buffer for the
> +   contents of a value.  TYPE is the type of the value for which we are
> +   allocating the buffer.  If the buffer is too large (based on the user
> +   controllable setting) then throw an error.  If this function returns
> +   then we should attempt to allocate the buffer.  */
> +
> +static void
> +check_type_length_before_alloc (const struct type *type)
> +{
> +  unsigned int length = TYPE_LENGTH (type);
> +
> +  if (max_value_size > -1 && length > max_value_size)
> +    {
> +      if (TYPE_NAME (type) != NULL)
> +	error (_("value of type `%s' requires %u bytes, which is more "
> +		 "than max-value-size"), TYPE_NAME (type), length);
> +      else
> +	error (_("value requires %u bytes, which is more than "
> +		 "max-value-size"), length);
> +    }
> +}
> +
>  /* Allocate the contents of VAL if it has not been allocated yet.  */
>  
>  static void
>  allocate_value_contents (struct value *val)
>  {
>    if (!val->contents)
> -    val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
> +    {
> +      check_type_length_before_alloc (val->enclosing_type);
> +      val->contents
> +	= (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
> +    }
>  }
>  
>  /* Allocate a  value  and its contents for type TYPE.  */
> @@ -2986,9 +3059,12 @@ value_static_field (struct type *type, int fieldno)
>  void
>  set_value_enclosing_type (struct value *val, struct type *new_encl_type)
>  {
> -  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val))) 
> -    val->contents =
> -      (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
> +  if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val)))
> +    {
> +      check_type_length_before_alloc (new_encl_type);
> +      val->contents
> +	= (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
> +    }
>  
>    val->enclosing_type = new_encl_type;
>  }
> @@ -4013,4 +4089,17 @@ Check whether an expression is void.\n\
>  Usage: $_isvoid (expression)\n\
>  Return 1 if the expression is void, zero otherwise."),
>  			 isvoid_internal_fn, NULL);
> +
> +  add_setshow_zuinteger_unlimited_cmd ("max-value-size",
> +				       class_support, &max_value_size, _("\
> +Set maximum sized value gdb will load from the inferior."), _("\
> +Show maximum sized value gdb will load from the inferior."), _("\
> +Use this to control the maximum size, in bytes, of a value that gdb\n\
> +will load from the inferior.  Setting this value to 'unlimited'\n\
> +disables checking.\n\
> +Setting this does not invalidate already allocated values, it only\n\
> +prevents future values, larger than this size, from being allocated."),
> +			    set_max_value_size,
> +			    show_max_value_size,
> +			    &setlist, &showlist);
>  }
> -- 
> 2.5.1

Re: PING #2: Re: [PATCH 1/3] gdb: New set/show max-value-size command.

[Joel Brobecker]

> > gdb/ChangeLog:
> > 
> > 	* value.c (max_value_size): New variable.
> > 	(MIN_VALUE_FOR_MAX_VALUE_SIZE): New define.
> > 	(show_max_value_size): New function.
> > 	(check_type_length_before_alloc): New function.
> > 	(allocate_value_contents): Call check_type_length_before_alloc.
> > 	(set_value_enclosing_type): Likewise.
> > 	(_initialize_values): Add set/show handler for max-value-size.
> > 	* NEWS: Mention new set/show command.
> > 
> > gdb/doc/ChangeLog:
> > 
> > 	* gdb.texinfo (Value Sizes): New section.
> > 	(Data): Add the 'Value Sizes' node to the menu.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> > 	* gdb.base/max-value-size.c: New file.
> > 	* gdb.base/max-value-size.exp: New file.
> > 	* gdb.base/huge.exp: Disable max-value-size for this test.

Sorry about the delay in reviewing this. Pre-approved after
a couple of trivial fixes...

> > +++ b/gdb/testsuite/gdb.base/max-value-size.c
> > @@ -0,0 +1,26 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > +   Copyright (C) 2016 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/>.  */
> > +
> > +char one;
> > +char ten [10];
> > +char one_hundred [100];

Tiny formatting nit: No space before the '[' in the two lines
above.

> > --- a/gdb/value.c
> > +++ b/gdb/value.c
> > @@ -955,13 +955,86 @@ allocate_value_lazy (struct type *type)
> >    return val;
> >  }
> >  
> > +/* The maximum size, in bytes, that GDB will try to allocate for a value.
> > +   The initial value of 64k was not selected for any specific reason, it is
> > +   just a reasonable starting point.  */
> > +
> > +static int max_value_size = 65536; /* 64k bytes */
> > +
> > +/* It is critical that the MAX_VALUE_SIZE is at least as bit as the size of

bit -> big

That's it. Thanks for this patch!
-- 
Joel

[testsuite patch] testsuite regression: gdb.fortran/vla-value-sub.exp gdb.fortran/vla-value-sub-finish.exp [Re: [PATCH 1/3] gdb: New set/show max-value-size command.]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

On Wed, 06 Jan 2016 12:40:50 +0100, Andrew Burgess wrote:
> +static int max_value_size = 65536; /* 64k bytes */

FAIL: gdb.fortran/vla-value-sub.exp: print array2 in foo after it was filled (passed fixed array)
FAIL: gdb.fortran/vla-value-sub.exp: print array2 in foo after it was mofified in debugger (passed fixed array)
FAIL: gdb.fortran/vla-value-sub-finish.exp: print array2 in foo after it was filled
FAIL: gdb.fortran/vla-value-sub-finish.exp: print array2 in foo after it was mofified in debugger

print array2
value requires 296352 bytes, which is more than max-value-size
(gdb) FAIL: gdb.fortran/vla-value-sub.exp: print array2 in foo after it was filled (passed fixed array)

OK for check-in?

Tested on x86_64-fedora23-linux-gnu.


Jan

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 1545 bytes --]

gdb/testsuite/ChangeLog
2016-02-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.fortran/vla-value-sub-finish.exp (set max-value-size 1024*1024):
	New test.
	* gdb.fortran/vla-value-sub.exp: Likewise.

diff --git a/gdb/testsuite/gdb.fortran/vla-value-sub-finish.exp b/gdb/testsuite/gdb.fortran/vla-value-sub-finish.exp
index c47ef2c..fde6c9f 100644
--- a/gdb/testsuite/gdb.fortran/vla-value-sub-finish.exp
+++ b/gdb/testsuite/gdb.fortran/vla-value-sub-finish.exp
@@ -32,6 +32,8 @@ if ![runto_main] {
 
 gdb_breakpoint [gdb_get_line_number "array2-almost-filled"]
 gdb_continue_to_breakpoint "array2-almost-filled"
+# array2 size is 296352 bytes.
+gdb_test_no_output "set max-value-size 1024*1024"
 gdb_test "print array2" " = \\( *\\( *\\( *30, *3, *3,\[()3, .\]*\\)" \
   "print array2 in foo after it was filled"
 gdb_test "print array2(2,1,1)=20" " = 20" \
diff --git a/gdb/testsuite/gdb.fortran/vla-value-sub.exp b/gdb/testsuite/gdb.fortran/vla-value-sub.exp
index 361d7a9..179683d 100644
--- a/gdb/testsuite/gdb.fortran/vla-value-sub.exp
+++ b/gdb/testsuite/gdb.fortran/vla-value-sub.exp
@@ -42,6 +42,8 @@ gdb_test "print array1(1, 1)" " = 30" \
 
 gdb_breakpoint [gdb_get_line_number "array2-almost-filled"]
 gdb_continue_to_breakpoint "array2-almost-filled (1st)"
+# array2 size is 296352 bytes.
+gdb_test_no_output "set max-value-size 1024*1024"
 gdb_test "print array2" " = \\( *\\( *\\( *30, *3, *3,\[()3, .\]*\\)" \
   "print array2 in foo after it was filled (passed fixed array)"
 gdb_test "print array2(2,1,1)=20" " = 20" \

Re: [testsuite patch] testsuite regression: gdb.fortran/vla-value-sub.exp gdb.fortran/vla-value-sub-finish.exp [Re: [PATCH 1/3] gdb: New set/show max-value-size command.]

[Andrew Burgess]

* Jan Kratochvil <jan.kratochvil@redhat.com> [2016-02-13 22:40:46 +0100]:

> On Wed, 06 Jan 2016 12:40:50 +0100, Andrew Burgess wrote:
> > +static int max_value_size = 65536; /* 64k bytes */
> 
> FAIL: gdb.fortran/vla-value-sub.exp: print array2 in foo after it was filled (passed fixed array)
> FAIL: gdb.fortran/vla-value-sub.exp: print array2 in foo after it was mofified in debugger (passed fixed array)
> FAIL: gdb.fortran/vla-value-sub-finish.exp: print array2 in foo after it was filled
> FAIL: gdb.fortran/vla-value-sub-finish.exp: print array2 in foo after it was mofified in debugger
> 
> print array2
> value requires 296352 bytes, which is more than max-value-size
> (gdb) FAIL: gdb.fortran/vla-value-sub.exp: print array2 in foo after it was filled (passed fixed array)
> 
> OK for check-in?

Not a maintainer, but this looks fine to me.

Apologies for not spotting the breakage, this error triggered so often
in the fortran tests I admit I got a bit lazy and assumed they were
all legitimate.

Thanks for tracking these down,
Andrew

> 
> Tested on x86_64-fedora23-linux-gnu.
> 
> 
> Jan

> gdb/testsuite/ChangeLog
> 2016-02-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.fortran/vla-value-sub-finish.exp (set max-value-size 1024*1024):
> 	New test.
> 	* gdb.fortran/vla-value-sub.exp: Likewise.
> 
> diff --git a/gdb/testsuite/gdb.fortran/vla-value-sub-finish.exp b/gdb/testsuite/gdb.fortran/vla-value-sub-finish.exp
> index c47ef2c..fde6c9f 100644
> --- a/gdb/testsuite/gdb.fortran/vla-value-sub-finish.exp
> +++ b/gdb/testsuite/gdb.fortran/vla-value-sub-finish.exp
> @@ -32,6 +32,8 @@ if ![runto_main] {
>  
>  gdb_breakpoint [gdb_get_line_number "array2-almost-filled"]
>  gdb_continue_to_breakpoint "array2-almost-filled"
> +# array2 size is 296352 bytes.
> +gdb_test_no_output "set max-value-size 1024*1024"
>  gdb_test "print array2" " = \\( *\\( *\\( *30, *3, *3,\[()3, .\]*\\)" \
>    "print array2 in foo after it was filled"
>  gdb_test "print array2(2,1,1)=20" " = 20" \
> diff --git a/gdb/testsuite/gdb.fortran/vla-value-sub.exp b/gdb/testsuite/gdb.fortran/vla-value-sub.exp
> index 361d7a9..179683d 100644
> --- a/gdb/testsuite/gdb.fortran/vla-value-sub.exp
> +++ b/gdb/testsuite/gdb.fortran/vla-value-sub.exp
> @@ -42,6 +42,8 @@ gdb_test "print array1(1, 1)" " = 30" \
>  
>  gdb_breakpoint [gdb_get_line_number "array2-almost-filled"]
>  gdb_continue_to_breakpoint "array2-almost-filled (1st)"
> +# array2 size is 296352 bytes.
> +gdb_test_no_output "set max-value-size 1024*1024"
>  gdb_test "print array2" " = \\( *\\( *\\( *30, *3, *3,\[()3, .\]*\\)" \
>    "print array2 in foo after it was filled (passed fixed array)"
>  gdb_test "print array2(2,1,1)=20" " = 20" \

Re: [testsuite patch] testsuite regression: gdb.fortran/vla-value-sub.exp gdb.fortran/vla-value-sub-finish.exp [Re: [PATCH 1/3] gdb: New set/show max-value-size command.]

[Joel Brobecker]

> gdb/testsuite/ChangeLog
> 2016-02-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.fortran/vla-value-sub-finish.exp (set max-value-size 1024*1024):
> 	New test.
> 	* gdb.fortran/vla-value-sub.exp: Likewise.

Looks good to me. I would say that future patches of this kind can
be considered as obvious when it is obvious that the object being
printed is larger than the default max-value-size.

Thanks Jan,
-- 
Joel

[commit] [testsuite patch] testsuite regression: gdb.fortran/vla-value-sub.exp gdb.fortran/vla-value-sub-finish.exp [Re: [PATCH 1/3] gdb: New set/show max-value-size command.]

[Jan Kratochvil]

On Sun, 14 Feb 2016 01:51:08 +0100, Andrew Burgess wrote:
> Not a maintainer, but this looks fine to me.

On Sun, 14 Feb 2016 05:37:54 +0100, Joel Brobecker wrote:
> Looks good to me.

Checked in:
	e9fb005c0e95e642c2e5a65c02d026b4223082e6
And for 7.11:
	307c2facb242da4fc557baea06e63d8e8a621142


Jan