[patch] Implement $_version; for auto-load commands in ~/.gdbinit

[patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Jan Kratochvil]

Hi,

it was requested by Eli in mail:
	Re: GDB 7.5: Problems with the auto-load safe-path feature
	http://sourceware.org/ml/gdb-patches/2012-08/msg00508.html
	D:\usr\eli/.gdbinit:1: Error in sourced command file: "on" or "off" expected.

IIRC it was already discussed even in the past.

One solution would be some new mode where errors are only printed and script
execution does not stop there.

I have implemented a way to explicitly check for GDB version instead.

No regressions on {x86_64,x86_64-m32,i686}-fedora18-linux-gnu.


Thanks,
Jan


gdb/
2012-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* top.c (init_main): New variables val, type, version_count, version_i
	and version_a.  Create internal variable $_version.
	* valarith.c (value_logical_not): Return 0 for lval_internalvar in
	c_style_arrays being of type TYPE_CODE_ARRAY.

gdb/testsuite/
2012-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/default.exp (show convenience): Cope with array types.
	($_version works without inferior, $_version is >= 7): New tests.

gdb/doc/
2012-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.texinfo (Convenience Vars): New item $_version with new anchor
	Convenience variable $_version.
	(Auto-loading safe path): Make two references to $_version.

diff --git a/gdb/top.c b/gdb/top.c
index 8251d1b..2d1a8cf 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1574,6 +1574,11 @@ set_gdb_datadir (char *args, int from_tty, struct cmd_list_element *c)
 static void
 init_main (void)
 {
+  struct value *val;
+  struct type *type;
+  int version_count, version_i;
+  unsigned version_a[4];
+
   /* Initialize the prompt to a simple "(gdb) " prompt or to whatever
      the DEFAULT_PROMPT is.  */
   set_prompt (DEFAULT_PROMPT);
@@ -1681,6 +1686,25 @@ When set, GDB uses the specified path to search for data files."),
                            set_gdb_datadir, NULL,
                            &setlist,
                            &showlist);
+
+  /* Set up the $_version array.  */
+  version_count = sscanf (version, "%u.%u.%u.%u", &version_a[0], &version_a[1],
+			  &version_a[2], &version_a[3]);
+  if (version_count < 2)
+    internal_error (__FILE__, __LINE__, _("Cannot parse GDB version \"%s\"!"),
+		    version);
+  type = create_range_type (NULL, builtin_type (target_gdbarch)->builtin_int,
+			    0, version_count - 1);
+  type = create_array_type (NULL,
+			    builtin_type (target_gdbarch)->builtin_long,
+			    type);
+  val = allocate_value (type);
+  for (version_i = 0; version_i < version_count; version_i++)
+    pack_long ((value_contents_writeable (val)
+		+ TYPE_LENGTH (TYPE_TARGET_TYPE (type)) * version_i),
+	       TYPE_TARGET_TYPE (type),
+	       version_a[version_i]);
+  set_internalvar (lookup_internalvar ("_version"), val);
 }
 
 void
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 6858d2b..e9c1745 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -1434,6 +1434,15 @@ value_logical_not (struct value *arg1)
   const gdb_byte *p;
   struct type *type1;
 
+  /* Prevent value_coerce_to_target for convenience array variables as it is
+     both needless overhead and it would fail without running inferior.  */
+  if (VALUE_LVAL (arg1) == lval_internalvar && current_language->c_style_arrays)
+    {
+      type1 = check_typedef (value_type (arg1));
+      if (TYPE_CODE (type1) == TYPE_CODE_ARRAY)
+	return 0;
+    }
+
   arg1 = coerce_array (arg1);
   type1 = check_typedef (value_type (arg1));
 
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index cc66da2..d312d1d 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -600,6 +600,8 @@ gdb_test "show confirm" "Whether to confirm potentially dangerous operations is
 # This is trickier as there are multiple internal convenience vars and
 # functions and we can't assume any particular order.
 # And we have to handle the extra convenience funs provided by Python.
+# $_version content is changing with each GDB to use gdb_test_list_exact,
+# it is tested later in this file.
 set show_conv_list \
     { \
 	{$_sdata = void} \
@@ -628,9 +630,10 @@ if ![skip_python_tests] {
 	    {$_strlen = <internal function _strlen>} \
 	}
 }
+set regex "\[^{}\r\n\]+(?:{\[^{}\]+})?"
 gdb_test_list_exact "show convenience" "show convenience" \
-    "\[^\r\n\]+\[\r\n\]+" \
-    "\[^\r\n\]+" \
+    "$regex\[\r\n\]+" \
+    $regex \
     $show_conv_list
 
 #test show directories
@@ -848,6 +851,11 @@ gdb_test "where" "No stack." "where"
 #test x
 gdb_test "x" "Argument required .starting display address.*" "x"
 
+# This test leaves some values on the stack, run it last.
+gdb_test "if \$_version\nprint 1+1\nelse\nprint 3+1\nend" " = 2" \
+	 {$_version works without inferior}
+gdb_test {print $_version[0] >= 7} " = 1" {$_version is >= 7}
+
 gdb_exit
 
 set timeout $prev_timeout
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 08ba92d..561422a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -9307,6 +9307,22 @@ gdbserver that supports the @code{qGetTIBAddr} request.
 @xref{General Query Packets}.
 This variable contains the address of the thread information block.
 
+@anchor{Convenience variable $_version}
+@item $_version
+The variable @code{$_version} contains the version number of @value{GDBN}.
+It is an array type containing 2 integer elements for releases, 3 elements for
+fix-up releases and 4 elements for development snapshots.  Comparing its value
+has to be careful if the script code should remain compatible with @value{GDBN}
+versions before 7.5 where this convenience variable was introduced.
+
+@smallexample
+if $_version
+  if $_version[0] > 7 || ($_version[0] == 7 && $_version[1] >= 5)
+   add-auto-load-safe-path ~/src
+  end
+end
+@end smallexample
+
 @end table
 
 On HP-UX systems, if you refer to a function or variable name that
@@ -21537,6 +21553,10 @@ Specify this trusted directory (or a file) as additional component of the list.
 You have to specify also any existing directories displayed by
 by @samp{show auto-load safe-path} (such as @samp{/usr:/bin} in this example).
 
+@xref{Convenience variable $_version}, for how to make such @file{~/.gdbinit}
+setting compatible with @value{GDBN} versions before 7.5 not supporting this
+commend yet.
+
 @item @kbd{gdb -iex "set auto-load safe-path /usr:/bin:~/src/gdb" @dots{}}
 Specify this directory as in the previous case but just for a single
 @value{GDBN} session.
@@ -21563,6 +21583,10 @@ You can use @value{GDBN} command-line option for a single @value{GDBN} session.
 Disable auto-loading globally for the user
 (@pxref{Home Directory Init File}).  While it is improbable, you could also
 use system init file instead (@pxref{System-wide configuration}).
+
+@xref{Convenience variable $_version}, for how to make such @file{~/.gdbinit}
+setting compatible with @value{GDBN} versions before 7.5 not supporting this
+commend yet.
 @end table
 
 This setting applies to the file names as entered by user.  If no entry matches

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Eli Zaretskii]

> Date: Tue, 21 Aug 2012 16:49:16 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: Eli Zaretskii <eliz@gnu.org>
> 
> it was requested by Eli in mail:
> 	Re: GDB 7.5: Problems with the auto-load safe-path feature
> 	http://sourceware.org/ml/gdb-patches/2012-08/msg00508.html
> 	D:\usr\eli/.gdbinit:1: Error in sourced command file: "on" or "off" expected.
> 
> IIRC it was already discussed even in the past.
> 
> One solution would be some new mode where errors are only printed and script
> execution does not stop there.
> 
> I have implemented a way to explicitly check for GDB version instead.

This is a very useful feature, thanks.

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -9307,6 +9307,22 @@ gdbserver that supports the @code{qGetTIBAddr} request.
>  @xref{General Query Packets}.
>  This variable contains the address of the thread information block.
>  
> +@anchor{Convenience variable $_version}
> +@item $_version

Please add a @vindex entry here.

> +@xref{Convenience variable $_version}, for how to make such @file{~/.gdbinit}
> +setting compatible with @value{GDBN} versions before 7.5 not supporting this
> +commend yet.
   ^^^^^^^
"command"

> +@xref{Convenience variable $_version}, for how to make such @file{~/.gdbinit}
> +setting compatible with @value{GDBN} versions before 7.5 not supporting this
> +commend yet.

Likewise.

OK with those changes.

Thanks.

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Doug Evans]

On Tue, Aug 21, 2012 at 7:49 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hi,
>
> it was requested by Eli in mail:
>         Re: GDB 7.5: Problems with the auto-load safe-path feature
>         http://sourceware.org/ml/gdb-patches/2012-08/msg00508.html
>         D:\usr\eli/.gdbinit:1: Error in sourced command file: "on" or "off" expected.
>
> IIRC it was already discussed even in the past.
>
> One solution would be some new mode where errors are only printed and script
> execution does not stop there.
>
> I have implemented a way to explicitly check for GDB version instead.

There is already gdb.VERSION in the Python API, but I guess $_version
could be useful to access for those without Python.

> No regressions on {x86_64,x86_64-m32,i686}-fedora18-linux-gnu.
>
>
> Thanks,
> Jan
>
>
> gdb/
> 2012-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>         * top.c (init_main): New variables val, type, version_count, version_i
>         and version_a.  Create internal variable $_version.
>         * valarith.c (value_logical_not): Return 0 for lval_internalvar in
>         c_style_arrays being of type TYPE_CODE_ARRAY.
>
> gdb/testsuite/
> 2012-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>         * gdb.base/default.exp (show convenience): Cope with array types.
>         ($_version works without inferior, $_version is >= 7): New tests.
>
> gdb/doc/
> 2012-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>         * gdb.texinfo (Convenience Vars): New item $_version with new anchor
>         Convenience variable $_version.
>         (Auto-loading safe path): Make two references to $_version.
>
> diff --git a/gdb/top.c b/gdb/top.c
> index 8251d1b..2d1a8cf 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -1574,6 +1574,11 @@ set_gdb_datadir (char *args, int from_tty, struct cmd_list_element *c)
>  static void
>  init_main (void)
>  {
> +  struct value *val;
> +  struct type *type;
> +  int version_count, version_i;
> +  unsigned version_a[4];
> +
>    /* Initialize the prompt to a simple "(gdb) " prompt or to whatever
>       the DEFAULT_PROMPT is.  */
>    set_prompt (DEFAULT_PROMPT);
> @@ -1681,6 +1686,25 @@ When set, GDB uses the specified path to search for data files."),
>                             set_gdb_datadir, NULL,
>                             &setlist,
>                             &showlist);
> +
> +  /* Set up the $_version array.  */
> +  version_count = sscanf (version, "%u.%u.%u.%u", &version_a[0], &version_a[1],
> +                         &version_a[2], &version_a[3]);
> +  if (version_count < 2)
> +    internal_error (__FILE__, __LINE__, _("Cannot parse GDB version \"%s\"!"),
> +                   version);
> +  type = create_range_type (NULL, builtin_type (target_gdbarch)->builtin_int,
> +                           0, version_count - 1);
> +  type = create_array_type (NULL,
> +                           builtin_type (target_gdbarch)->builtin_long,
> +                           type);
> +  val = allocate_value (type);
> +  for (version_i = 0; version_i < version_count; version_i++)
> +    pack_long ((value_contents_writeable (val)
> +               + TYPE_LENGTH (TYPE_TARGET_TYPE (type)) * version_i),
> +              TYPE_TARGET_TYPE (type),
> +              version_a[version_i]);
> +  set_internalvar (lookup_internalvar ("_version"), val);
>  }

If there are concerns of $_version decaying to a pointer if the
current language is C, may it should be a vector.

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> One solution would be some new mode where errors are only printed
Jan> and script execution does not stop there.

I think that would be preferable.  It is better to test features rather
than versions.

It is trivial to write an 'ignore-errors' in python, or C for that matter.
Or you could resurrect the old try-catch patch for the CLI that is
languishing in bugzilla.

Jan> I have implemented a way to explicitly check for GDB version instead.

The problem I see is that distros often ship new features but can't
assign a meaningful new version number.

Tom

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Pedro Alves]

On 08/22/2012 09:06 PM, Tom Tromey wrote:
>>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> One solution would be some new mode where errors are only printed
> Jan> and script execution does not stop there.
> 
> I think that would be preferable.  It is better to test features rather
> than versions.
> 
> It is trivial to write an 'ignore-errors' in python, or C for that matter.
> Or you could resurrect the old try-catch patch for the CLI that is
> languishing in bugzilla.

+1

-- 
Pedro Alves

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Jan Kratochvil]

On Wed, 22 Aug 2012 22:06:40 +0200, Tom Tromey wrote:
> It is trivial to write an 'ignore-errors' in python, or C for that matter.
> Or you could resurrect the old try-catch patch for the CLI that is
> languishing in bugzilla.

Any new GDB command cannot be safely used in older versions, even a possible
GDB command 'ignore-errors'.

Before writing it is there a consensus for example for ~/.gdbinit to use
	set $_ignore_errors=1
	add-auto-load-safe-path ~/src
	set $_ignore_errors=0
?


Thanks,
Jan

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Jan Kratochvil]

On Fri, 24 Aug 2012 18:32:15 +0200, Jan Kratochvil wrote:
> Before writing it is there a consensus for example for ~/.gdbinit to use
> 	set $_ignore_errors=1
> 	add-auto-load-safe-path ~/src
> 	set $_ignore_errors=0
> ?

Apparently that would not work.  Probably either:
	if $_have_ignore_errors
		set ignore-errors yes
		add-auto-load-safe-path ~/src
		set ignore-errors no
	end
or:
	if $_have_ignore_errors
		set $_ignore-errors=1
		add-auto-load-safe-path ~/src
		set $_ignore-errors=0
	end
or:
	if $_have_ignore_errors
		ignore-errors
			add-auto-load-safe-path ~/src
		end
	end


Thanks,
Jan

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[dje]

Jan Kratochvil writes:
 > On Fri, 24 Aug 2012 18:32:15 +0200, Jan Kratochvil wrote:
 > > Before writing it is there a consensus for example for ~/.gdbinit to use
 > > 	set $_ignore_errors=1
 > > 	add-auto-load-safe-path ~/src
 > > 	set $_ignore_errors=0
 > > ?
 > 
 > Apparently that would not work.  Probably either:
 > 	if $_have_ignore_errors
 > 		set ignore-errors yes
 > 		add-auto-load-safe-path ~/src
 > 		set ignore-errors no
 > 	end
 > or:
 > 	if $_have_ignore_errors
 > 		set $_ignore-errors=1
 > 		add-auto-load-safe-path ~/src
 > 		set $_ignore-errors=0
 > 	end
 > or:
 > 	if $_have_ignore_errors
 > 		ignore-errors
 > 			add-auto-load-safe-path ~/src
 > 		end
 > 	end

For my own education, why is $_have_ignore_errors needed?
[It doesn't, for example, evaluate to 0 if it doesn't exist.]

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Jan Kratochvil]

On Fri, 24 Aug 2012 18:54:07 +0200, dje@google.com wrote:
> Jan Kratochvil writes:
>  > 	if $_have_ignore_errors
>  > 		set ignore-errors yes
>  > 		add-auto-load-safe-path ~/src
>  > 		set ignore-errors no
>  > 	end
[...]
> For my own education, why is $_have_ignore_errors needed?
> [It doesn't, for example, evaluate to 0 if it doesn't exist.]

It evaluates to void, which is false:

(gdb) p $_x
$1 = void
(gdb) ptype $_x
type = void

$ cat x
if $_x
 echo YES\n
else
 echo NO\n
end
$ gdb -q -x ./x
NO


Thanks,
Jan

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> Apparently that would not work.  Probably either:
Jan> 	if $_have_ignore_errors

That would be ok with me.

Jan> 		set ignore-errors yes
Jan> 		add-auto-load-safe-path ~/src
Jan> 		set ignore-errors no

The python ignore-errors I wrote works like:

    ignore-errors add-auto-load-safe-path ~/src

I find it simpler.

Tom

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Jan Kratochvil]

On Fri, 24 Aug 2012 19:41:22 +0200, Tom Tromey wrote:
> The python ignore-errors I wrote works like:
> 
>     ignore-errors add-auto-load-safe-path ~/src
> 
> I find it simpler.

How does it work for:

ignore-errors commands
 set confirm no
 quit
end


Thanks,
Jan

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Tom Tromey]

Jan> How does it work for:
Jan> ignore-errors commands
Jan>  set confirm no
Jan>  quit
Jan> end

It doesn't.

I think you should just do what you want.

Tom

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Doug Evans]

On Fri, Aug 24, 2012 at 9:57 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> It evaluates to void, which is false:

Yikes.
[though I understand the utility]

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Eli Zaretskii]

> Date: Fri, 24 Aug 2012 18:38:56 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org, Eli Zaretskii <eliz@gnu.org>
> 
> On Fri, 24 Aug 2012 18:32:15 +0200, Jan Kratochvil wrote:
> > Before writing it is there a consensus for example for ~/.gdbinit to use
> > 	set $_ignore_errors=1
> > 	add-auto-load-safe-path ~/src
> > 	set $_ignore_errors=0
> > ?
> 
> Apparently that would not work.  Probably either:
> 	if $_have_ignore_errors
> 		set ignore-errors yes
> 		add-auto-load-safe-path ~/src
> 		set ignore-errors no
> 	end
> or:
> 	if $_have_ignore_errors
> 		set $_ignore-errors=1
> 		add-auto-load-safe-path ~/src
> 		set $_ignore-errors=0
> 	end
> or:
> 	if $_have_ignore_errors
> 		ignore-errors
> 			add-auto-load-safe-path ~/src
> 		end
> 	end

I fail to see how this is better than using $_version.

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Jan Kratochvil]

On Fri, 24 Aug 2012 20:20:48 +0200, Eli Zaretskii wrote:
> I fail to see how this is better than using $_version.

GNU gdb (GDB) Fedora (7.3.50.20110722-16.fc16)
[...]
(gdb) show auto-load safe-path 
List of directories from which it is safe to auto-load files is [...].

And this is even not yet GDB 7.4.  And for RHELs the security backport will be
for even older GDBs.


Regards,
Jan

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Eli Zaretskii]

> Date: Fri, 24 Aug 2012 20:25:24 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: tromey@redhat.com, gdb-patches@sourceware.org
> 
> On Fri, 24 Aug 2012 20:20:48 +0200, Eli Zaretskii wrote:
> > I fail to see how this is better than using $_version.
> 
> GNU gdb (GDB) Fedora (7.3.50.20110722-16.fc16)
> [...]
> (gdb) show auto-load safe-path 
> List of directories from which it is safe to auto-load files is [...].
> 
> And this is even not yet GDB 7.4.  And for RHELs the security backport will be
> for even older GDBs.

But all this stuff we are discussing is for the user, not for the
distributor.  A user always knows which version(s) she uses, so she
can set the minimum versions accordingly.

And if worse comes to worst, one can always be conservative.  It's not
a disaster not to invoke a command that was back-ported.

I find the alternatives more complex and not much more useful.

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Jan Kratochvil]

On Fri, 24 Aug 2012 20:49:47 +0200, Eli Zaretskii wrote:
> But all this stuff we are discussing is for the user, not for the
> distributor.  A user always knows which version(s) she uses, so she
> can set the minimum versions accordingly.

Users AFAIK use distro GDB, but they use also FSF GDB HEAD.  So your request
for compatible ~/.gdbinit should apply for both FSF and distro GDBs.


> And if worse comes to worst, one can always be conservative.  It's not
> a disaster not to invoke a command that was back-ported.
> 
> I find the alternatives more complex and not much more useful.

TBH I do not find this whole request too useful and I may be one of the people
using various GDB variants most interchange-ably.

I am also fine with dropping this feature completely.

Both the "ignore-errors" or the "version" path have various difficulties where
one may find easier to deal with the problem by hand, at least I do.


Jan

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Eli Zaretskii]

> Date: Fri, 24 Aug 2012 20:59:24 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: tromey@redhat.com, gdb-patches@sourceware.org
> 
> Both the "ignore-errors" or the "version" path have various difficulties where
> one may find easier to deal with the problem by hand, at least I do.

Then maybe we should have both.

Re: [patch] Implement $_version; for auto-load commands in ~/.gdbinit

[Joel Brobecker]

> TBH I do not find this whole request too useful and I may be one of
> the people using various GDB variants most interchange-ably.

Same here. I "solved" the problem by simply having the newer commands
at the end of my .gdbinit. It generates an error when I use GDB 7.4,
which is not elegant, but I don't mind.

-- 
Joel