[RFAv3 1/3] Implement convenience functions to examine GDB settings.

[RFAv3 1/3] Implement convenience functions to examine GDB settings.

[Philippe Waroquiers]

The new convenience functions $_gdb_setting and $_gdb_int_setting
provide access to the GDB settings in user-defined commands.

gdb/ChangeLog
2019-07-06  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* cli/cli-cmds.c (setting_cmd, gdb_int_setting_internal_fn,
	gdb_setting_internal_fn): New functions.
	(_initialize_cli_cmds): Define the new convenience functions.
---
 gdb/cli/cli-cmds.c | 103 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 368ddf526d..4316b0d578 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1905,6 +1905,98 @@ show_max_user_call_depth (struct ui_file *file, int from_tty,
 		    value);
 }
 
+/* Returns the cmd_list_element corresponding to the first argument
+   of ARGV, which must contain one single value.
+   Throws an error if no value provided, or value not correct.
+   FNNAME is used in the error message.  */
+
+static cmd_list_element *
+setting_cmd (const char *fnname, int argc, struct value **argv)
+{
+  if (argc == 0)
+    error (_("You must provide an argument to %s"), fnname);
+  if (argc != 1)
+    error (_("You can only provide one argument to %s"), fnname);
+
+  struct type *type0 = check_typedef (value_type (argv[0]));
+
+  if (TYPE_CODE (type0) != TYPE_CODE_ARRAY
+      && TYPE_CODE (type0) != TYPE_CODE_STRING)
+    error (_("First argument of %s must be a string."), fnname);
+
+  int len0 = TYPE_LENGTH (type0);
+  std::vector<char> arg0 (len0);
+
+  memcpy (arg0.data (), value_contents (argv[0]), len0);
+
+  const char *a0 = arg0.data ();
+  cmd_list_element *cmd = lookup_cmd (&a0, showlist, "", -1, 0);
+
+  if (cmd == nullptr || cmd_type (cmd) != show_cmd)
+    error (_("First argument of %s must be a "
+	     "valid setting of the 'show' command."), fnname);
+
+  return cmd;
+}
+
+/* Implementation of the convenience function $_gdb_int_setting.  */
+
+static struct value *
+gdb_int_setting_internal_fn (struct gdbarch *gdbarch,
+			     const struct language_defn *language,
+			     void *cookie, int argc, struct value **argv)
+{
+  cmd_list_element *cmd = setting_cmd ("$_gdb_int_setting", argc, argv);
+
+  switch (cmd->var_type)
+    {
+    case var_boolean:
+    case var_integer:
+    case var_zinteger:
+    case var_zuinteger_unlimited:
+      return value_from_longest (builtin_type (gdbarch)->builtin_int,
+				 *(int *) cmd->var);
+    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);
+      }
+    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);
+    case var_zuinteger:
+      return value_from_longest (builtin_type (gdbarch)->builtin_int,
+				 *(unsigned int *) cmd->var);
+    default:
+      error (_("Setting type not convertible to integer"));
+    }
+}
+
+/* Implementation of the convenience function $_gdb_setting.  */
+
+static struct value *
+gdb_setting_internal_fn (struct gdbarch *gdbarch,
+			 const struct language_defn *language,
+			 void *cookie, int argc, struct value **argv)
+{
+  cmd_list_element *cmd = setting_cmd ("$_gdb_setting", argc, argv);
+  std::string cmd_val = get_setshow_command_value_string (cmd);
+
+  return value_cstring (cmd_val.c_str (), strlen (cmd_val.c_str ()),
+			builtin_type (gdbarch)->builtin_char);
+}
+
+
 void
 _initialize_cli_cmds (void)
 {
@@ -2045,6 +2137,17 @@ abbreviations for commands and/or values.  E.g.:\n\
   set_cmd_completer_handle_brkchars (c, with_command_completer);
   add_com_alias ("w", "with", class_vars, 1);
 
+  add_internal_function ("_gdb_setting", _("\
+$_gdb_setting - returns the value of a GDB setting as a string.\n\
+Usage: $_gdb_setting (setting)"),
+			 gdb_setting_internal_fn, NULL);
+
+  add_internal_function ("_gdb_int_setting", _("\
+$_gdb_int_setting - returns the value of an integer GDB setting as an integer.\n\
+Usage: $_int_gdb_int_setting (setting)\n\
+Throws an error if SETTING cannot be converted to an integer."),
+			 gdb_int_setting_internal_fn, NULL);
+
   add_cmd ("commands", no_set_class, show_commands, _("\
 Show the history of commands you typed.\n\
 You can supply a command number to start with, or a `+' to start after\n\
-- 
2.20.1

[RFAv3 2/3] Test the convenience functions $_gdb_setting and $_gdb_int_setting.

[Philippe Waroquiers]

gdb/testsuite/ChangeLog
2019-06-07  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/setshow.exp: Test $_gdb_setting and $_gdb_int_setting.
	* gdb.base/default.exp: Update show_conv_list.
---
 gdb/testsuite/gdb.base/default.exp |  2 ++
 gdb/testsuite/gdb.base/setshow.exp | 57 +++++++++++++++++++++++++++++-
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index 0325b8045d..8e79cbc85d 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -604,6 +604,8 @@ set show_conv_list \
 	{$_cimag = <internal function _cimag>} \
 	{$_creal = <internal function _creal>} \
 	{$_isvoid = <internal function _isvoid>} \
+	{$_gdb_int_setting = <internal function _gdb_int_setting>} \
+	{$_gdb_setting = <internal function _gdb_setting>} \
 	{$_gdb_major = 8} \
 	{$_gdb_minor = 4} \
 	{$_shell_exitsignal = void} \
diff --git a/gdb/testsuite/gdb.base/setshow.exp b/gdb/testsuite/gdb.base/setshow.exp
index d807d75a66..6ab6e6d923 100644
--- a/gdb/testsuite/gdb.base/setshow.exp
+++ b/gdb/testsuite/gdb.base/setshow.exp
@@ -91,6 +91,9 @@ gdb_test "show args" "Argument list to give program being debugged when it is st
 gdb_test_no_output "set args foo bar blup baz bubble" "set args" 
 #test show args
 gdb_test "show args" "Argument list to give program being debugged when it is started is \"foo bar blup baz bubble\"..*" "show args"
+gdb_test "p \$_gdb_setting(\"args\")" ".*\"foo bar blup baz bubble\"" \
+    "_gdb_setting args"
+
 
 # Don't test if we can't pass args or if we're using a stub.
 if { !$use_gdb_stub && ![target_info exists noargs] } {
@@ -100,17 +103,23 @@ if { !$use_gdb_stub && ![target_info exists noargs] } {
     gdb_test "run" "Starting program:.*foo bar blup baz bubble.*" "passing args"
 }
 #test set check range on
-gdb_test "set check range on" ".*" "set check range on" 
+gdb_test "set check range on" ".*" "set check range on"
+gdb_test "p \$_gdb_setting(\"check range\")" ".*\"on\"" \
+    "_gdb_setting check range on"
 #test show check range on
 gdb_test "show check range" "Range checking is \"on\"\..*" "show check range (on)" 
 #test set check range off with trailing space
 gdb_test_no_output "set check range off " "set check range off" 
 #test show check range off
 gdb_test "show check range" "Range checking is \"off\"\..*" "show check range (off)" 
+gdb_test "p \$_gdb_setting(\"check range\")" ".*\"off\"" \
+    "_gdb_setting check range off"
 #test set check range auto
 gdb_test_no_output "set check range auto" "set check range auto" 
 #test show check range auto
 gdb_test "show check range" "Range checking is \"auto; currently .*" "show check range (auto)"
+gdb_test "p \$_gdb_setting(\"check range\")" ".*\"auto\"" \
+    "_gdb_setting check range auto"
 
 # Test set check type on
 gdb_test "set check type on" ".*" "set check type on"
@@ -118,14 +127,49 @@ gdb_test "set check type on" ".*" "set check type on"
 # Test show check type on
 gdb_test "show check type" "Strict type checking is on\..*" \
     "show check type (on)"
+gdb_test "p \$_gdb_setting(\"check type\")" ".*\"on\"" \
+    "_gdb_setting check type on"
+gdb_test "p \$_gdb_int_setting(\"check type\")" ".*= 1" \
+    "_gdb_setting check type on 1"
 
 # Test set check type off with trailing space
 gdb_test_no_output "set check type off " "set check type off"
+gdb_test "p \$_gdb_setting(\"check type\")" ".*\"off\"" \
+    "_gdb_setting check type off"
+gdb_test "p \$_gdb_int_setting(\"check type\")" ".*= 0" \
+    "_gdb_setting check type off 0"
 
 # Test show check type off
 gdb_test "show check type" "Strict type checking is off\..*" \
     "show check type (off)"
 
+#test set breakpoint pending
+#test set breakpoint pending on
+gdb_test "set breakpoint pending on" ".*" "set breakpoint pending on"
+gdb_test "p \$_gdb_setting(\"breakpoint pending\")" ".*\"on\"" \
+    "_gdb_setting breakpoint pending on"
+gdb_test "p \$_gdb_int_setting(\"breakpoint pending\")" ".*= 1" \
+    "_gdb_int_setting breakpoint pending 1"
+#test show breakpoint pending on
+gdb_test "show breakpoint pending" ".* is on\..*" "show breakpoint pending on"
+#test show breakpoint pending off
+gdb_test "set breakpoint pending off" ".*" "set breakpoint pending off"
+gdb_test "show breakpoint pending" ".* is off\..*" "show breakpoint pending off"
+gdb_test "p \$_gdb_setting(\"breakpoint pending\")" ".*\"off\"" \
+    "_gdb_setting breakpoint pending off"
+gdb_test "p \$_gdb_int_setting(\"breakpoint pending\")" ".* = 0" \
+    "_gdb_int_setting breakpoint pending 0"
+#test set breakpoint pending auto
+gdb_test_no_output "set breakpoint pending auto" "set breakpoint pending auto"
+#test show breakpoint pending auto
+gdb_test "show breakpoint pending" " is auto.*" "show breakpoint pending auto"
+gdb_test "p \$_gdb_setting(\"breakpoint pending\")" ".*\"auto\"" \
+    "_gdb_setting breakpoint pending auto"
+gdb_test "p \$_gdb_int_setting(\"breakpoint pending\")" ".* = 2" \
+    "_gdb_setting breakpoint pending 2"
+
+
+
 #test set complaints 100
 gdb_test_no_output "set complaints 100" "set complaints 100" 
 #test show complaints 100
@@ -159,9 +203,17 @@ gdb_test "show environment FOOBARBAZ" "FOOBARBAZ = grbxgrbxgrbx.*"  "show enviro
 gdb_test_no_output "set height 100" "set height 100" 
 #test show height 100
 gdb_test "show height" "Number of lines gdb thinks are in a page is 100..*" "show height" 
+gdb_test "p \$_gdb_setting(\"height\")" ".*\"100\"" \
+    "_gdb_setting height 100"
+gdb_test "p \$_gdb_int_setting(\"height\")" ".*= 100" \
+    "_gdb_int_setting height 100"
 # Back to infinite height to avoid pagers.  While at it, check that
 # literal "unlimited" works just as well as 0.
 gdb_test_no_output "set height unlimited"
+gdb_test "p \$_gdb_setting(\"height\")" ".*\"unlimited\"" \
+    "_gdb_setting height unlimited"
+gdb_test "p \$_gdb_int_setting(\"height\")" ".*= 0" \
+    "_gdb_int_setting height unlimited"
 #test set history expansion on
 gdb_test_no_output "set history expansion on" "set history expansion on" 
 #test show history expansion on
@@ -182,6 +234,9 @@ gdb_test_no_output "set history filename ~/foobar.baz" \
 gdb_test "show history filename" \
     "The filename in which to record the command history is \"[string_to_regexp $HOME]/foobar.baz\"..*" \
     "show history filename (~/foobar.baz)"
+gdb_test "p \$_gdb_setting(\"history filename\")" \
+    ".*\"[string_to_regexp $HOME]/foobar.baz\"..*" \
+    "_gdb_setting history filename"
 #get current working directory
 set PWD ""
 set test "show working directory"
-- 
2.20.1

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

[Philippe Waroquiers]

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.

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.

This is v3.
Compared to v1, it rebases to a recent master, and handles the comments
of Eli about the documentation.

(there was in fact no changes between v2 and v1, because I forgot to
commit the changes before sending the RFAv2 mail).

[RFAv3 3/3] NEWS and documentation for $_gdb_setting and $_gdb_int_setting.

[Philippe Waroquiers]

gdb/ChangeLog
2019-06-07  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* NEWS: Mention $_gdb_setting and $_gdb_int_setting.

gdb/doc/ChangeLog
2019-06-07  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.texinfo (Convenience Funs): Document the new
	$_gdb_setting and $_gdb_int_setting convenience functions.
---
 gdb/NEWS            |  5 ++++
 gdb/doc/gdb.texinfo | 66 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 34c544c3d5..d17d37ceff 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -11,6 +11,11 @@
   scripts that should work error-free with many different versions,
   such as in system-wide init files.
 
+* New built-in convenience functions $_gdb_setting and $_gdb_int_setting
+  provide access to values of the GDB settings.  They are handy
+  for changing the logic of user defined commands depending on the
+  current GDB settings.
+
 * GDB now supports Thread Local Storage (TLS) variables on several
   FreeBSD architectures (amd64, i386, powerpc, riscv).  Other
   architectures require kernel changes.  TLS is not yet supported for
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 299c4a12a1..e31f43f417 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11820,9 +11820,73 @@ $3 = void
 $4 = 1
 @end smallexample
 
+@item $_gdb_setting (@var{setting})
+@findex $_gdb_setting@r{, convenience function}
+Return the value of the @value{GDBN} @var{setting} as a string.
+@var{setting} is any setting that can be used in a @code{set} or
+@code{show} command (@pxref{Controlling GDB}).
+@code{$_gdb_setting} works for for any setting kind.  In particular,
+it also works for non string setting kinds such as boolean, auto boolean
+and integer settings.
+
+@smallexample
+(@value{GDBP}) show print frame-arguments
+Printing of non-scalar frame arguments is "scalars".
+(@value{GDBP}) p $_gdb_setting("print frame-arguments")
+$2 = "scalars"
+(@value{GDBP}) p $_gdb_setting("height")
+$3 = "30"
+(@value{GDBP})
+@end smallexample
+
+@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}.
+Some integer settings accepts an @code{unlimited} value.
+Depending on the setting, the @code{set} command also accepts
+the value @code{0} or the value @code{@minus{}1} as a synonym for
+@code{unlimited}.
+For example, @code{set height unlimited} is equivalent to
+@code{set height 0}.
+Some other settings accepting the @code{unlimited} value
+are using the value @code{0} to literally mean zero.
+For example, @code{set history size 0} indicates to not
+record any @value{GDBN} commands in the command history.
+For such settings, @code{@minus{}1} is the synonym
+for @code{unlimited}.
+
+The function @code{$_gdb_int_setting} converts the unlimited value
+to a @code{0} or a @code{@minus{}1} value according to what the
+@code{set} command uses.
+
+@smallexample
+@group
+(@value{GDBP}) p $_gdb_setting("height")
+$3 = "30"
+(@value{GDBP}) p $_gdb_int_setting("height")
+$4 = 30
+(@value{GDBP}) set height unlimited
+(@value{GDBP}) p $_gdb_setting("height")
+$5 = "unlimited"
+(@value{GDBP}) p $_gdb_int_setting("height")
+$6 = 0
+@end group
+@group
+(@value{GDBP}) p $_gdb_setting("history size")
+$7 = "unlimited"
+(@value{GDBP}) p $_gdb_int_setting("history size")
+$8 = -1
+(@value{GDBP})
+@end group
+@end smallexample
+
 @end table
 
-These functions require @value{GDBN} to be configured with
+The following functions require @value{GDBN} to be configured with
 @code{Python} support.
 
 @table @code
-- 
2.20.1

Re: [RFAv3 3/3] NEWS and documentation for $_gdb_setting and $_gdb_int_setting.

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Sat,  6 Jul 2019 12:49:47 +0200
> 
> gdb/ChangeLog
> 2019-06-07  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* NEWS: Mention $_gdb_setting and $_gdb_int_setting.
> 
> gdb/doc/ChangeLog
> 2019-06-07  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.texinfo (Convenience Funs): Document the new
> 	$_gdb_setting and $_gdb_int_setting convenience functions.

The documentation part of the patch is OK, but see a single comment
about that in my other message.

Thanks.

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

[Pedro Alves]

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.

BTW, did you look into how gdb.parameter is implemented, see if
anything could be shared?

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.

Also, it seems like a design issue that settings that are unsigned
internally get their values exposed as signed.

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(...))'.  (*)

(*) this made me wonder about a special $_python convenience function,
which would take some python code as argument, and return a value, so you
could write:

 if $_python(gdb.parameter ("pagination"))
   ...
 end

etc.

> This is v3.
> Compared to v1, it rebases to a recent master, and handles the comments
> of Eli about the documentation.
> 
> (there was in fact no changes between v2 and v1, because I forgot to
> commit the changes before sending the RFAv2 mail).
Thanks,
Pedro Alves

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

[Philippe Waroquiers]

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).


> 
> 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.

> 
> 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.

> 
> (*) this made me wonder about a special $_python convenience function,
> which would take some python code as argument, and return a value, so you
> could write:
> 
>  if $_python(gdb.parameter ("pagination"))
>    ...
>  end
That would be a less cumbersome way to mix python expressions and canned
sequence of commands, but I am wondering if this mix is preferable
to the above convenience functions, as it implies the user has to
(at least somewhat) master both environments even for simple things.


Thanks

Philippe

> 
> etc.
> 
> > This is v3.
> > Compared to v1, it rebases to a recent master, and handles the comments
> > of Eli about the documentation.
> > 
> > (there was in fact no changes between v2 and v1, because I forgot to
> > commit the changes before sending the RFAv2 mail).
> 
> Thanks,
> Pedro Alves

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

[Pedro Alves]

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

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

[Philippe Waroquiers]

On Wed, 2019-07-10 at 16:34 +0100, Pedro Alves wrote:
> Alright, sounds good.  That's the sort of rationale that I'm looking
> for.
Ok, I will add the rational in the commit log.


> > > 
> > > 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 ?
I understand now.  Effectively, that is bad.
And the mapping of auto-boolean is worse.
I will fix all this.


> 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")
Yes, that should have been the idea.

> 
> 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
>  };
> 
> ?
Doing this change will make the conversion of auto-boolean to int
a simple conversion (like boolean), rather than doing it with a
switch.
I guess that apart of making the conversion code simpler,
it will have no effect if all the code is properly doing comparisons
to AUTO_BOOLEAN_TRUE/FALSE/AUTO.


> I wonder whether gdb.parameter's handling of auto-booleans is
> also mismatched.

Here is an extract of the documentation:
   -- Function: gdb.parameter (parameter)
     ... Otherwise, the
     parameter's value is converted to a Python value of the appropriate
     type, and returned.

So, what is an 'appropriate type' for an auto-boolean ?

Doing the test:
  (gdb) set breakpoint pending 0
  (gdb) python print(gdb.parameter("breakpoint pending"))
  False
  (gdb) set breakpoint pending 1
  (gdb) python print(gdb.parameter("breakpoint pending"))
  True
  (gdb) set breakpoint pending -1
  (gdb) python print(gdb.parameter("breakpoint pending"))
  None
  (gdb) 

So, the above looks like a reasonable mapping for auto-boolean,
as soon as you understand that None is auto.

This mapping is described in python.texi, in the section
telling how to create new parameters.

However, the behaviour seems not fully consistent for all
integer settings:
  (gdb) set height 0
  (gdb) show height
  Number of lines gdb thinks are in a page is unlimited.
  (gdb) python print(gdb.parameter("height"))
  None
  (gdb) 

but ...
  (gdb) set history size unlimited
  (gdb) python print(gdb.parameter("history size"))
  -1
  (gdb) 

So, not too logical here ...

(the doc seems to not describe None being unlimited).


> 
> 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.
Good ideas ...


Thanks for all the comments, I will prepare a new version ...

Philippe