Re: [RFAv3 0/3] Convenience functions $_gdb_setting/$_gdb_int_setting

Pedro Alves
2019-07-10T15:34:29Z

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:
\f
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