From patchwork Fri Feb 23 04:59:14 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 26017 Received: (qmail 25128 invoked by alias); 23 Feb 2018 04:59:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 25106 invoked by uid 89); 23 Feb 2018 04:59:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=(unknown), attched X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 23 Feb 2018 04:59:21 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id B45041169BE; Thu, 22 Feb 2018 23:59:19 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id qzl55eTPq8Ho; Thu, 22 Feb 2018 23:59:19 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id EEB6611694F; Thu, 22 Feb 2018 23:59:18 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 7DFD183054; Fri, 23 Feb 2018 08:59:14 +0400 (+04) Date: Fri, 23 Feb 2018 08:59:14 +0400 From: Joel Brobecker To: Eli Zaretskii Cc: gdb-patches@sourceware.org Subject: Re: [RFA/doco] set varsize-limit: New GDB setting for maximum dynamic object size Message-ID: <20180223045914.47lm5byba3o6xrqo@adacore.com> References: <1519312873-6307-1-git-send-email-brobecker@adacore.com> <83efld58mu.fsf@gnu.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <83efld58mu.fsf@gnu.org> User-Agent: NeoMutt/20170113 (1.7.2) Hi Eli, Thanks for the review. Sometimes, I wonder what we would do without your help! > > +set|show varsize-limit > > + This new setting allows the user to control the maximum size of Ada > > + objects being printing when those objects have a dynamic type, > ^^^^^^^^ > "printed", I guess? Yes, absolutely. Corrected in the attached version. > > > + 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."), > > This is okay, but I wonder if "varsize" is a good name. The > explanatory text you've written doesn't talk of any "variables". It was before my time (so more than 15 years ago!), but I think that "var" (for variable) is the adjective, rather than the noun. We could discuss better names, but as I was trying to say towards the end of my initial email, I am trying to avoid that (if possible!), to avoid having users who need to transition between the old setting's name, and the new one. Believe it or not, I'm still hearing from users who are still stuck using "begin" (the name AdaCore chose to start the program and stop at the first line of the main subprogram -- this command was eventually contributed and the name chosen was "start"). I get the same regular grief regarding "break exception", which was changed to "catch exception" when contributed as well. Of course, if we come up with a name that is so much better, I will sure those impacted have a transition path. In this case, I think the name of the command is not all that bad. Also, error message raised when we exceed that limit gives a hint towards the setting that controls it: void ada_ensure_varsize_limit (const struct type *type) { if (TYPE_LENGTH (type) > varsize_limit) error (_("object size is larger than varsize-limit")); } So I would like to argue for us to keep the current name if people are OK with that. > > +@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. > > Is this only for printing, or is this more general? > > in any case, "limit the size of types of objects" is ambiguous (the > "types" part confuses things), and doesn't really tell what does this > limit. If it's only about printing, then let's say "don't print > objects whose size exceeds ...". If it isn't limited to printing, how > about "don't attempt to evaluate objects ...". > > > +to @code{unlimited}, there is no limit. > > I'd rephrase > > Setting @var{size} to @code{unlimited} removes the size limitations. Good suggestions. Thanks! New version attched. gdb/ChangeLog * NEWS: Add entry describing new "set|show varsize-limit" command. * ada-lang.c (_initialize_ada_language): Add "set/show varsize-limit" command. * printcmd.c (_initialize_printcmd): Add "set var" alias of "set variable". gdb/doc/ChangeLog: * gdb.texinfo (Ada Settings): New subsubsection. gdb/testsuite/ChangeLog: * gdb.ada/varsize_limit: New testcase. Tested on x86_64-linux. Thank you, From 5c11253be1c881b90f6f74fd8acb3535012a0b0c Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Thu, 22 Feb 2018 09:07:15 -0500 Subject: [PATCH] set varsize-limit: New GDB setting for maximum dynamic object size This is a command we somehow forgot to contribute at the time the Ada language was first contributed to the FSF. This command allows the user to change the maximum size we allow when reading memory from dynamic objects (the default is 65536 bytes). At the moment, this limit is only used by Ada, and so the implementation is kept inside ada-lang.c. However, it is conceivable that other language might want to use it also to handle the same kind of issues; for instance, this might be useful when handling dynamic types in C. So the name of the setting was made language-neutral, to allow for this. Note that an alias for "set var" needs to be introduced as well. We are not adding a test for that, since this is a feature that is already exercized by numerous existing tests. gdb/ChangeLog * NEWS: Add entry describing new "set|show varsize-limit" command. * ada-lang.c (_initialize_ada_language): Add "set/show varsize-limit" command. * printcmd.c (_initialize_printcmd): Add "set var" alias of "set variable". gdb/doc/ChangeLog: * gdb.texinfo (Ada Settings): New subsubsection. gdb/testsuite/ChangeLog: * gdb.ada/varsize_limit: New testcase. Tested on x86_64-linux. --- gdb/NEWS | 7 +++++ gdb/ada-lang.c | 7 +++++ gdb/doc/gdb.texinfo | 32 ++++++++++++++++++++ gdb/printcmd.c | 1 + gdb/testsuite/gdb.ada/varsize_limit.exp | 38 ++++++++++++++++++++++++ gdb/testsuite/gdb.ada/varsize_limit/pck.adb | 25 ++++++++++++++++ gdb/testsuite/gdb.ada/varsize_limit/pck.ads | 20 +++++++++++++ gdb/testsuite/gdb.ada/varsize_limit/vsizelim.adb | 23 ++++++++++++++ 8 files changed, 153 insertions(+) create mode 100644 gdb/testsuite/gdb.ada/varsize_limit.exp create mode 100644 gdb/testsuite/gdb.ada/varsize_limit/pck.adb create mode 100644 gdb/testsuite/gdb.ada/varsize_limit/pck.ads create mode 100644 gdb/testsuite/gdb.ada/varsize_limit/vsizelim.adb diff --git a/gdb/NEWS b/gdb/NEWS index 1767cef..be1f066 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -6,6 +6,13 @@ * 'info proc' now works on running processes on FreeBSD systems and core files created on FreeBSD systems. +* New commands + +set|show varsize-limit + This new setting allows the user to control the maximum size of Ada + objects being printed when those objects have a dynamic type, + instead of that maximum size being hardcoded to 65536 bytes. + *** Changes in GDB 8.1 * GDB now supports dynamically creating arbitrary register groups specified diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 3de9f14..d0aec80 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -14689,6 +14689,13 @@ With an argument, catch only exceptions with the given name."), 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_info ("exceptions", info_exceptions_command, _("\ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index ee7adc8..8303d19 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -16268,6 +16268,7 @@ to be difficult. * Ada Tasks and Core Files:: Tasking Support when Debugging Core Files * Ravenscar Profile:: Tasking Support when using the Ravenscar Profile +* Ada Settings:: New settable GDB parameters for Ada. * Ada Glitches:: Known peculiarities of Ada mode. @end menu @@ -16923,6 +16924,37 @@ using the Ravenscar Profile. @end table +@node Ada Settings +@subsubsection Ada Settings +@cindex Ada settings + +@table @code +@kindex set varsize-limit +@item set varsize-limit @var{size} +Prevent @value{GDBN} from attempting to evaluate objects whose size +is above the given limit (@var{size}) when those sizes are computed +from run-time quantities. This is typically the case when the object +has a dynamic type, such as an array whose bounds are not known at +compile time for example. Setting @var{size} to @code{unlimited} +removes the size limitations. By default, the limit is about 65KB. + +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 +@code{x.y} is dynamic and exceeds @code{size}. On the other hand, +@value{GDBN} is sometimes clever; the expression @code{A(i)}, where +@code{A} is an array variable with non-constant size, will generally +succeed regardless of the bounds on @code{A}, as long as the component +size is less than @var{size}. + +@kindex show varsize-limit +@item show varsize-limit +Show the limit on types whose size is determined by run-time quantities. +@end table + @node Ada Glitches @subsubsection Known Peculiarities of Ada Mode @cindex Ada, problems diff --git a/gdb/printcmd.c b/gdb/printcmd.c index fc9d7e4..6d671de 100644 --- a/gdb/printcmd.c +++ b/gdb/printcmd.c @@ -2725,6 +2725,7 @@ with $), a register (a few standard names starting with $), or an actual\n\ variable in the program being debugged. EXP is any valid expression.\n\ This may usually be abbreviated to simply \"set\"."), &setlist); + add_alias_cmd ("var", "variable", class_vars, 0, &setlist); c = add_com ("print", class_vars, print_command, _("\ Print value of expression EXP.\n\ diff --git a/gdb/testsuite/gdb.ada/varsize_limit.exp b/gdb/testsuite/gdb.ada/varsize_limit.exp new file mode 100644 index 0000000..b201755 --- /dev/null +++ b/gdb/testsuite/gdb.ada/varsize_limit.exp @@ -0,0 +1,38 @@ +# Copyright 2018 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 . + +load_lib "ada.exp" + +standard_ada_testfile vsizelim + +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } { + return -1 +} + +clean_restart ${testfile} + +set bp_location [gdb_get_line_number "STOP" ${testdir}/vsizelim.adb] +if ![runto "vsizelim.adb:$bp_location" ] then { + perror "Couldn't run ${testfile}" + return +} + +gdb_test_no_output "set varsize-limit 16" + +gdb_test "print small" " = \"1234567890\"" + +gdb_test "print larger" "object size is larger than varsize-limit.*" + + diff --git a/gdb/testsuite/gdb.ada/varsize_limit/pck.adb b/gdb/testsuite/gdb.ada/varsize_limit/pck.adb new file mode 100644 index 0000000..d4acf86 --- /dev/null +++ b/gdb/testsuite/gdb.ada/varsize_limit/pck.adb @@ -0,0 +1,25 @@ +-- Copyright 2018 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 . + +package body Pck is + procedure Do_Nothing (A : System.Address) is + begin + null; + end Do_Nothing; + function Ident (S : String) return String is + begin + return S; + end Ident; +end Pck; diff --git a/gdb/testsuite/gdb.ada/varsize_limit/pck.ads b/gdb/testsuite/gdb.ada/varsize_limit/pck.ads new file mode 100644 index 0000000..b0043fa --- /dev/null +++ b/gdb/testsuite/gdb.ada/varsize_limit/pck.ads @@ -0,0 +1,20 @@ +-- Copyright 2018 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 . + +with System; +package Pck is + procedure Do_Nothing (A : System.Address); + function Ident (S : String) return String; +end Pck; diff --git a/gdb/testsuite/gdb.ada/varsize_limit/vsizelim.adb b/gdb/testsuite/gdb.ada/varsize_limit/vsizelim.adb new file mode 100644 index 0000000..e65ebfd --- /dev/null +++ b/gdb/testsuite/gdb.ada/varsize_limit/vsizelim.adb @@ -0,0 +1,23 @@ +-- Copyright 2018 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 . + +with Pck; use Pck; +procedure VsizeLim is + Small : String := Ident ("1234567890"); + Larger : String := Ident ("1234567890|1234567890|1234567890"); +begin + Do_Nothing (Small'Address); -- STOP + Do_Nothing (Larger'Address); +end VsizeLim; -- 2.1.4