public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFC][gdb/cli] Add convenience vars _wp_old_val and _wp_val
@ 2022-09-30  9:16 Tom de Vries
  2022-09-30 15:37 ` Pedro Alves
  0 siblings, 1 reply; 5+ messages in thread
From: Tom de Vries @ 2022-09-30  9:16 UTC (permalink / raw)
  To: gdb-patches

Hi,

Add convenience variables _wp_old_val and _wp_val, that match the reported
values of a stopped watchpoint:
...
Hardware watchpoint 2: v

Old value = 3
New value = 2
main () at watchpoint-convenience-vars.c:12
12        return 0;
(gdb) print $_wp_old_val
$1 = 3
(gdb) print $_wp_val
$2 = 2
...

These can be used in a watchpoint condition, for instance to only stop when
changing from one value to another value:
...
(gdb) cond 2 $_wp_old_val == 3 && $_wp_val == 2
...

RFC for now, so no docs yet.

What about naming?  Maybe _wp_old_value and _wp_new_value are more intuitive
because they match the "Old value = _/ New value = _" message?

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29480

Any comments?

Thanks,
- Tom

[gdb/cli] Add convenience vars _wp_old_val and _wp_val

---
 gdb/breakpoint.c                                   |  4 +++
 .../gdb.base/watchpoint-convenience-vars.c         | 30 ++++++++++++++++
 .../gdb.base/watchpoint-convenience-vars.exp       | 40 ++++++++++++++++++++++
 3 files changed, 74 insertions(+)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 002f4a935b1..15b5356cb98 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5060,7 +5060,11 @@ watchpoint_check (bpstat *bs)
 						       new_val)))
 	{
 	  bs->old_val = b->val;
+	  if (bs->old_val.get () != nullptr)
+	    set_internalvar (lookup_internalvar ("_wp_old_val"),
+			     bs->old_val.get ());
 	  b->val = release_value (new_val);
+	  set_internalvar (lookup_internalvar ("_wp_val"), b->val.get ());
 	  b->val_valid = true;
 	  if (new_val != NULL)
 	    value_free_to_mark (mark);
diff --git a/gdb/testsuite/gdb.base/watchpoint-convenience-vars.c b/gdb/testsuite/gdb.base/watchpoint-convenience-vars.c
new file mode 100644
index 00000000000..8681fedb1d9
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-convenience-vars.c
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022 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 <http://www.gnu.org/licenses/>.  */
+
+int v = 0;
+
+int
+main (void)
+{
+  v = 1;
+  v = 2;
+
+  v = 3;
+  v = 2;
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/watchpoint-convenience-vars.exp b/gdb/testsuite/gdb.base/watchpoint-convenience-vars.exp
new file mode 100644
index 00000000000..b5d1ed619f8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-convenience-vars.exp
@@ -0,0 +1,40 @@
+# Copyright 2022 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 <http://www.gnu.org/licenses/>.
+
+# Check the watchpoint convenience variables _wp_old_val and _wp_val.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# Set a watchpoint.
+gdb_test "watch v" "\[Ww\]atchpoint 2: v"
+
+# Make the watchpoint conditional on a specific value transition, using
+# watchpoint convenience variables.
+gdb_test_no_output "cond 2 \$_wp_old_val == 3 && \$_wp_val == 2"
+
+# Verify that the watchpoint triggers on the value transition.
+gdb_test "continue" "Old value = 3\r\nNew value = 2\r\n.*"
+
+# Verify the value of the convenience variables.
+gdb_test "print \$_wp_old_val" " = 3"
+gdb_test "print \$_wp_val" " = 2"

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC][gdb/cli] Add convenience vars _wp_old_val and _wp_val
  2022-09-30  9:16 [RFC][gdb/cli] Add convenience vars _wp_old_val and _wp_val Tom de Vries
@ 2022-09-30 15:37 ` Pedro Alves
  2022-10-07 19:41   ` Tom Tromey
  0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2022-09-30 15:37 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

On 2022-09-30 10:16 a.m., Tom de Vries via Gdb-patches wrote:
> Hi,
> 
> Add convenience variables _wp_old_val and _wp_val, that match the reported
> values of a stopped watchpoint:
> ...
> Hardware watchpoint 2: v
> 
> Old value = 3
> New value = 2
> main () at watchpoint-convenience-vars.c:12
> 12        return 0;
> (gdb) print $_wp_old_val
> $1 = 3
> (gdb) print $_wp_val
> $2 = 2
> ...
> 
> These can be used in a watchpoint condition, for instance to only stop when
> changing from one value to another value:
> ...
> (gdb) cond 2 $_wp_old_val == 3 && $_wp_val == 2
> ...

Cool.  I like it.  Can also be used in the watchpoint's commands.

It can't be safely used in regular CLI after the watchpoint hit is process, though,
given another watchpoint may trigger meanwhile.  I mean, say, in non-stop, you do:

(gdb) c -a &

Hardware watchpoint 2: v

Old value = 3
New value = 2
main () at foo.c:12
12        return 0;

# time passes, you type other gdb commands.

(gdb) print $_wp_old_val # you want to look at wp's 2 value, but just as you're
                         # typing this, another watchpoint triggers:

Hardware watchpoint 3: o

Old value = 4
New value = 5
main () at foo.c:14
14        return 2;

(gdb) print $_wp_old_val
$123 = 4

# whoops


A way to make this safer would be to make it a convenience function instead, that
takes as argument the number of the watchpoint, like:

(gdb) p $_wp_old_val(2)

For the condition example, you'd write:

  (gdb) cond 2 $_wp_old_val($bpnum) == 3 && $_wp_val($bpnum) == 2

I guess that isn't as convenient.  Unless...  I guess the convenient functions
could default to $bpnum if no argument is passed?  That'd reduce it to:

  (gdb) cond 2 $_wp_old_val() == 3 && $_wp_val() == 2

If that's still too much, I guess we could have both separately named convenience variables
and convenience functions.

> RFC for now, so no docs yet.
> 
> What about naming?  Maybe _wp_old_value and _wp_new_value are more intuitive
> because they match the "Old value = _/ New value = _" message?

I'm fine with the shorter names as you proposed.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC][gdb/cli] Add convenience vars _wp_old_val and _wp_val
  2022-09-30 15:37 ` Pedro Alves
@ 2022-10-07 19:41   ` Tom Tromey
  2022-10-08  6:11     ` Tom de Vries
  2022-10-10 13:49     ` Pedro Alves
  0 siblings, 2 replies; 5+ messages in thread
From: Tom Tromey @ 2022-10-07 19:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom de Vries, gdb-patches

Pedro> It can't be safely used in regular CLI after the watchpoint hit is process, though,
Pedro> given another watchpoint may trigger meanwhile.  I mean, say, in non-stop, you do:

FWIW gdb probably already has this problem with some other convenience
variables, like $_exitcode, $_exception, $_probe_*, ...

So, maybe adding one more isn't so bad.  Or maybe now is when we want to
think of a general solution.

I'm not sure this works though:

Pedro> A way to make this safer would be to make it a convenience function instead, that
Pedro> takes as argument the number of the watchpoint, like:

... because it seems to me that the same watchpoint can be hit any
number of times.

Tom

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC][gdb/cli] Add convenience vars _wp_old_val and _wp_val
  2022-10-07 19:41   ` Tom Tromey
@ 2022-10-08  6:11     ` Tom de Vries
  2022-10-10 13:49     ` Pedro Alves
  1 sibling, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2022-10-08  6:11 UTC (permalink / raw)
  To: Tom Tromey, Pedro Alves; +Cc: gdb-patches

On 10/7/22 21:41, Tom Tromey wrote:
> Pedro> It can't be safely used in regular CLI after the watchpoint hit is process, though,
> Pedro> given another watchpoint may trigger meanwhile.  I mean, say, in non-stop, you do:
> 
> FWIW gdb probably already has this problem with some other convenience
> variables, like $_exitcode, $_exception, $_probe_*, ...
> 
> So, maybe adding one more isn't so bad.  Or maybe now is when we want to
> think of a general solution.
> 
> I'm not sure this works though:
> 
> Pedro> A way to make this safer would be to make it a convenience function instead, that
> Pedro> takes as argument the number of the watchpoint, like:
> 
> ... because it seems to me that the same watchpoint can be hit any
> number of times.

I wondered if we can side-step the issue by defining the variable only 
during watchpoint_check, and undefining it immediately afterwards.

Thanks,
- Tom


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC][gdb/cli] Add convenience vars _wp_old_val and _wp_val
  2022-10-07 19:41   ` Tom Tromey
  2022-10-08  6:11     ` Tom de Vries
@ 2022-10-10 13:49     ` Pedro Alves
  1 sibling, 0 replies; 5+ messages in thread
From: Pedro Alves @ 2022-10-10 13:49 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Tom de Vries, gdb-patches

Hi,

On 2022-10-07 8:41 p.m., Tom Tromey wrote:
> Pedro> It can't be safely used in regular CLI after the watchpoint hit is process, though,
> Pedro> given another watchpoint may trigger meanwhile.  I mean, say, in non-stop, you do:
> 
> FWIW gdb probably already has this problem with some other convenience
> variables, like $_exitcode, $_exception, $_probe_*, ...

Hmm, indeed.  Good point.

> 
> So, maybe adding one more isn't so bad.  Or maybe now is when we want to
> think of a general solution.
> 
> I'm not sure this works though:
> 
> Pedro> A way to make this safer would be to make it a convenience function instead, that
> Pedro> takes as argument the number of the watchpoint, like:
> 
> ... because it seems to me that the same watchpoint can be hit any
> number of times.

Also true.

I withdraw my suggestion then.

I guess one way to address this would be to make it so that the
values were put in the value history, with their own unique history
numbers.  Like, e.g.:

  Hardware watchpoint 2: v

  Old value ($123) = 3
  New value ($124) = 2

Not sure about that.  Maybe we can think up something better.
For some other time.

Pedro Alves

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-10-10 13:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-30  9:16 [RFC][gdb/cli] Add convenience vars _wp_old_val and _wp_val Tom de Vries
2022-09-30 15:37 ` Pedro Alves
2022-10-07 19:41   ` Tom Tromey
2022-10-08  6:11     ` Tom de Vries
2022-10-10 13:49     ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).