This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


* 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]