From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120148 invoked by alias); 20 Jan 2016 10:59:30 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 120137 invoked by uid 89); 20 Jan 2016 10:59:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL,BAYES_50,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=sized, @code, D*embecosm.com, @section X-HELO: mail-wm0-f49.google.com Received: from mail-wm0-f49.google.com (HELO mail-wm0-f49.google.com) (74.125.82.49) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 20 Jan 2016 10:59:26 +0000 Received: by mail-wm0-f49.google.com with SMTP id b14so22859169wmb.1 for ; Wed, 20 Jan 2016 02:59:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=zE4xUe8KMoHEJjp0hJAqtmXFxH3NfRUmy6xDCH7QCQY=; b=hoW75bQRktk/nqDUzP26AVgetUV8FAYp4dK6t7Xdm9nl1XrCSkqP3Of2YjKMZCXH4B 9FzmL6P+JMkQWqdyQoIWyVRb9q/ycaT5a8WUTMgl/F9c9qk3X3ZcSKcFCgCZSC8VIFSf 9mv2+GAT4Rk7Sq3XxInWpX8dgJiWAQgYweZFe3ZextM2o9zqEdMDvXn9CB7C0NyvHJcF lNc7rJlWDYqcGOduWzuANrJfWgiuriRq8T0crWCi2VbZQSHdBvyoW3I9X0BjCMMpQXgq VtYFAIk25viGiwCCpeP1Gn/jORBBtZ4WAjTyYntCj6i3oLKLYxB2yQAqeBGtuEdAGIEZ prVQ== X-Gm-Message-State: AG10YORzg0JFLsMnSrbKY7nNWrE6E0dTJO6iXU1L47l9o+TjwfVZ+1BMoMdy2gmsAw/J4g== X-Received: by 10.28.195.212 with SMTP id t203mr3531859wmf.46.1453287562742; Wed, 20 Jan 2016 02:59:22 -0800 (PST) Received: from localhost (TK158115.telekabel.at. [195.34.158.115]) by smtp.gmail.com with ESMTPSA id l194sm24693726wmb.14.2016.01.20.02.59.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Jan 2016 02:59:22 -0800 (PST) Date: Wed, 20 Jan 2016 10:59:00 -0000 From: Andrew Burgess To: gdb-patches@sourceware.org Cc: brobecker@adacore.com, Pedro Alves , Eli Zaretskii Subject: PING: Re: [PATCH 1/3] gdb: New set/show max-value-size command. Message-ID: <20160120105921.GW4242@embecosm.com> References: <57e2731e179d11c584e8cde994ab1e822a9893b0.1449869722.git.andrew.burgess@embecosm.com> <20160101094309.GC12416@adacore.com> <20160105141241.GG4242@embecosm.com> <83a8ok570f.fsf@gnu.org> <20160106114049.GJ4242@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160106114049.GJ4242@embecosm.com> X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes X-SW-Source: 2016-01/txt/msg00468.txt.bz2 Ping! I believe I've addressed all review comments. Thanks, Andrew * Andrew Burgess [2016-01-06 11:40:50 +0000]: > * Eli Zaretskii [2016-01-05 18:24:32 +0200]: > > > > Date: Tue, 5 Jan 2016 14:12:41 +0000 > > > From: Andrew Burgess > > > 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 > + > + * 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 > > * 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 > + > + * gdb.texinfo (Value Sizes): New section. > + (Data): Add the 'Value Sizes' node to the menu. > + > 2015-12-11 Don Breazeal > > * 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 > + > + * gdb.base/max-value-size.c: New file. > + * gdb.base/max-value-size.exp: New file. > + > 2016-01-04 Markus Metzger > > * 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 . */ > + > +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 . > + > +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 \\}" > + } > + 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 >