From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 119830 invoked by alias); 10 Jul 2019 15:34:29 -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 119821 invoked by uid 89); 10 Jul 2019 15:34:29 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=BAYES_00,GIT_PATCH_1,KAM_SHORT,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=throughout, mix, Cc, spirit X-HELO: mail-wr1-f66.google.com Received: from mail-wr1-f66.google.com (HELO mail-wr1-f66.google.com) (209.85.221.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 10 Jul 2019 15:34:26 +0000 Received: by mail-wr1-f66.google.com with SMTP id 31so2980480wrm.1 for ; Wed, 10 Jul 2019 08:34:26 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id u13sm2763559wrq.62.2019.07.10.08.34.23 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Wed, 10 Jul 2019 08:34:23 -0700 (PDT) Subject: Re: [RFAv3 0/3] Convenience functions $_gdb_setting/$_gdb_int_setting To: Philippe Waroquiers , gdb-patches@sourceware.org References: <20190706104947.30301-1-philippe.waroquiers@skynet.be> <1562625228.1521.3.camel@skynet.be> From: Pedro Alves Message-ID: Date: Wed, 10 Jul 2019 15:34:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <1562625228.1521.3.camel@skynet.be> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2019-07/txt/msg00248.txt.bz2 On 7/8/19 11:33 PM, Philippe Waroquiers wrote: > On Mon, 2019-07-08 at 17:34 +0100, Pedro Alves wrote: >> Hi Philippe, >> >> Some overall design comments below. >> >> On 7/6/19 11:49 AM, Philippe Waroquiers wrote: >>> As part of the discussion of 'show | set may-call-functions [on|off]', >>> Eli suggested to have a way to access the GDB settings from >>> user defined commands. >>> >> >> As I had mentioned back then: >> >> https://sourceware.org/ml/gdb-patches/2019-04/msg00562.html >> >> we can already access the settings via Python. E.g. see it >> done here from a gdbinit script: >> >> https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00135.html >> >> Copied here for convenience: >> >> python __gcc_prev_pagination=gdb.parameter("pagination") >> set pagination off >> ... >> python if __gcc_prev_pagination: gdb.execute("set pagination on") >> >> Given that, and the existence of the "with" command now for the >> common case, I'd like to at least see some mention/rationale/argument >> in the commit logs about why this is worth having/maintaining >> over just using gdb.parameter. > > Here is some background about the rational: > > The patch was developed following a comment of Eli about the > 'set may-call-functions'. Eli said that user-defined functions > should have a way to change their behavior according to this setting. > Rather than have a specialized $_may_call_functions, this patch > implements a general way to access any GDB setting. > > Compared to the above access via Python: > * The 'with' command is much better than the above python usage: > if the user types C-c or an error happens between the set pagination off > and the python "set pagination on", the above python  > does not restory the original setting. > > * Effectively, with the above python one liner, it is possible to do > simple 'if' conditions, such as set and restore pagination. > But as far as I can see, mixing the "python if" within canned > sequence of commands is cumbersome for non trivial combinations. > E.g. if several commands have to be done for a certain condition > accessed from python, I guess something like will be needed: > python if __some_setting: gdb.execute("some command") > python if __some_setting: gdb.execute("some other command") > python if __some_setting: gdb.execute("some different command") > (without speaking about nested "if-s"). > > With the convenience function: > if $_gdb_setting("some_setting") > some command > some other command > some different command > end > Integer settings (for example print elements) will also be more difficult > to use. > For example, a user defined function that scans and prints a linked list > might want to use the value of "set print elements" to stop printing > the linked list. > Doing that by mixing python expression/if is I guess doable, but seems > not easy with the above one liners. > > So, in summary, the $_gdb_setting=$_gdb_int_setting avoids to have the > heterogenous mix of python and GDB commands in one single script > (and of course, it works even if python is not configured, but that > must be an unusual setup I guess). Alright, sounds good. That's the sort of rationale that I'm looking for. > > >> >> BTW, did you look into how gdb.parameter is implemented, see if >> anything could be shared? > I quickly looked at the implementation of python gdb.parameter, > but could not find anything significant that can be shared. > Note that $_gdb_setting is (now) re-using the function > get_setshow_command_value_string that was developed for > the "with" command. > >> >> BTW², kind of unfortunate that Python used a different naming >> here (settings vs parameters). >> >>> So, this patch series implements this. >>> >>> 2 functions are provided: >>> * $_gdb_setting that gives access to all settings, giving back a string value. >>> * $_gdb_int_setting, giving access to boolean, auto-boolean and integers. >>> This is easier to use for such types than the string version. >> >> The naming of the functions kind of seems a bit reversed to me. Off hand, I'd >> expect $_gdb_setting to give me the setting in its native type, and then >> something like $_gdb_setting_str to give me a string version/representation. > Probably my Ada background that pushes me to have strongly typed functions :). > > I think it is easy to have $_gdb_setting returning 'int' (for types that can > be converted to integers) and strings for the others, > and have $_gdb_setting_str always returning a string. > >> >> Also, it seems like a design issue that settings that are unsigned >> internally get their values exposed as signed. > Not sure I understand the comment. > $_gdb_int_setting returns an integer value that is the same as what > the user can type. If the user can type -1 (as a synonym for unlimited), > then $_gdb_int_setting returns -1, otherwise it will show 0 for unlimited. For example: > + case var_uinteger: > + if (*(unsigned int *) cmd->var == UINT_MAX) > + return value_from_longest (builtin_type (gdbarch)->builtin_int, > + 0); > + else > + return value_from_longest (builtin_type (gdbarch)->builtin_int, > + *(unsigned int *) cmd->var); Why is this value_from_longest + builtin_int instead of value_from_ulongest + builtin_unsigned_int ? Also, looking at the values / mapping done for auto-boolean settings: + case var_auto_boolean: + { + int val = *(int *) cmd->var; + + if (val == 0) + val = 1; + else if (val == 1) + val = 0; + return value_from_longest (builtin_type (gdbarch)->builtin_int, + val); + } and: +@item $_gdb_int_setting (@var{setting}) +@findex $_gdb_int_setting@r{, convenience function} +Return the value of the @value{GDBN} @var{setting} as an integer. +This only works for boolean, auto boolean and integer settings. +The boolean values @code{off} and @code{on} are converted to ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +the integer values @code{0} and @code{1}. The value @code{auto} is ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +converted to the value @code{2}. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ and: enum auto_boolean { AUTO_BOOLEAN_TRUE, /* 0, not 1! */ AUTO_BOOLEAN_FALSE, /* 1, not 0! */ AUTO_BOOLEAN_AUTO /* 2 */ }; (comments mine) and: static enum auto_boolean parse_auto_binary_operation (const char *arg) { if (arg != NULL && *arg != '\0') { int length = strlen (arg); while (isspace (arg[length - 1]) && length > 0) length--; /* Note that "o" is ambiguous. */ if ((length == 2 && strncmp (arg, "on", length) == 0) || strncmp (arg, "1", length) == 0 || strncmp (arg, "yes", length) == 0 || strncmp (arg, "enable", length) == 0) return AUTO_BOOLEAN_TRUE; else if ((length >= 2 && strncmp (arg, "off", length) == 0) || strncmp (arg, "0", length) == 0 || strncmp (arg, "no", length) == 0 || strncmp (arg, "disable", length) == 0) return AUTO_BOOLEAN_FALSE; else if (strncmp (arg, "auto", length) == 0 || (length > 1 && strncmp (arg, "-1", length) == 0)) return AUTO_BOOLEAN_AUTO; } error (_("\"on\", \"off\" or \"auto\" expected.")); return AUTO_BOOLEAN_AUTO; /* Pacify GCC. */ } we see quite a mismatch. Particularly, note that the auto-boolean set commands accept -1 for auto, not 2: else if (strncmp (arg, "auto", length) == 0 || (length > 1 && strncmp (arg, "-1", length) == 0)) return AUTO_BOOLEAN_AUTO; Vis: (gdb) set breakpoint pending 1 (gdb) show breakpoint pending Debugger's behavior regarding pending breakpoints is on. (gdb) set breakpoint pending -1 (gdb) show breakpoint pending Debugger's behavior regarding pending breakpoints is auto. (gdb) set breakpoint pending 2 "on", "off" or "auto" expected. I think that $_gdb_int_setting should use the same convention as the "set" command. I.e., conceptually, this should leave the setting unmodified: (gdb) set breakpoint pending $_gdb_int_setting("breakpoint pending") Should we normalize that throughout by changing auto_boolean to this: enum auto_boolean { AUTO_BOOLEAN_TRUE = 1 AUTO_BOOLEAN_FALSE = 0, AUTO_BOOLEAN_AUTO = -1 }; ? I wonder whether gdb.parameter's handling of auto-booleans is also mismatched. On the testing side, would it be possible to convert the testing to use the "maint set/show test-settings" subcommands, and thus be able to test all the different kinds of settings, in the same spirit as gdb.base/settings.exp and gdb.base/with.exp? I think the only impediment is accessing the maint settings, which you could do by adding a $_gdb_maint_setting function, much like I had added "maint with" for testing the "with" command's infrastructure. It's bound to be useful anyway, so that scripts can access the main settings. > >> >> I guess it could be even better if the setting's types were some new built-in >> types specific for the settings, and then if you wanted to get a string >> representation, you'd use '$_as_string($_gdb_setting(...))'. (*) > Maybe I miss the idea: not too clear what this new built-in > 'setting type' would bring, as the user will have in any case to convert > it to a 'basic' type (e.g. int or string) to use it e.g. in an expression. > So, that seems significantly more work without clear gain. If $_gdb_setting(...) returned a "var_uinteger int" type, then $_as_string would be able to return "unlimited" instead of "0". Certainly something that could be added / experimented later on. A $_gdb_setting_str function seems fine with me as well. Thanks, Pedro Alves