public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [PR python/29603] Fix deletion of Watchpoints
@ 2022-09-25  5:33 Johnson Sun
  2022-09-25 18:10 ` Lancelot SIX
  2022-10-01  5:20 ` Johnson Sun
  0 siblings, 2 replies; 9+ messages in thread
From: Johnson Sun @ 2022-09-25  5:33 UTC (permalink / raw)
  To: gdb-patches; +Cc: j3.soon777

Currently, during normal stop (i.e., stop and waiting for the next command),
GDB automatically deletes local Watchpoints that are out of scope. However,
local Watchpoints are not deleted if a Python script decides not to normal
stop upon hit, causing them to be hit many more times than they should, as
reported in PR python/29603. This was happening because the watchpoint is not
disabled when going out of scope.

This commit fixes this issue by disabling the watchpoint when going out of
scope. It also adds a test to ensure this feature isn't regressed in the
future.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29603
---
 gdb/breakpoint.c                           |  1 +
 gdb/testsuite/gdb.python/py-watchpoint.c   | 27 +++++++++++++
 gdb/testsuite/gdb.python/py-watchpoint.exp | 44 ++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-watchpoint.py  | 31 +++++++++++++++
 4 files changed, 103 insertions(+)
 create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.c
 create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.exp
 create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.py

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index bff3bac7d1a..b78ae9c4993 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1832,6 +1832,7 @@ watchpoint_del_at_next_stop (struct watchpoint *w)
       w->related_breakpoint = w;
     }
   w->disposition = disp_del_at_next_stop;
+  disable_breakpoint(w);
 }
 
 /* Extract a bitfield value from value VAL using the bit parameters contained in
diff --git a/gdb/testsuite/gdb.python/py-watchpoint.c b/gdb/testsuite/gdb.python/py-watchpoint.c
new file mode 100644
index 00000000000..6d02e87e571
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-watchpoint.c
@@ -0,0 +1,27 @@
+/* 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/>.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  int i;
+  for (i = 0; i < 3; i++)
+    printf ("%d", i);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-watchpoint.exp b/gdb/testsuite/gdb.python/py-watchpoint.exp
new file mode 100644
index 00000000000..863f3eff66a
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-watchpoint.exp
@@ -0,0 +1,44 @@
+# Copyright (C) 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/>.
+
+# This file is part of the GDB testsuite.  It checks Watchpoints
+# are deleted after used.
+
+load_lib gdb-python.exp
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if ![runto_main] then {
+    return 0
+}
+
+#
+# Check Watchpoints are deleted after used
+#
+
+gdb_test "set can-use-hw-watchpoints 0" ".*" "Don't use hardware watchpoints"
+gdb_test "python print (len(gdb.breakpoints()))" "1" "check default BP count"
+gdb_test "source ${srcdir}/${subdir}/${testfile}.py" ".*Python script imported.*" \
+	 "import python scripts"
+gdb_test "python print (len(gdb.breakpoints()))" "2" "check modified BP count"
+gdb_test "continue" ".*Watchpoint Hit: 4.*" "run until program stops"
+gdb_test "python print (len(gdb.breakpoints()))" "1" "check BP count"
diff --git a/gdb/testsuite/gdb.python/py-watchpoint.py b/gdb/testsuite/gdb.python/py-watchpoint.py
new file mode 100644
index 00000000000..855c820b245
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-watchpoint.py
@@ -0,0 +1,31 @@
+# Copyright (C) 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/>.
+
+# This file is part of the GDB testsuite.  It checks Watchpoints
+# are deleted after used.
+
+class MyBreakpoint(gdb.Breakpoint):
+    def __init__(self, *args, **kwargs):
+        gdb.Breakpoint.__init__(self, *args, **kwargs)
+        self.i = 0
+    def stop(self):
+        self.i += 1
+        print("Watchpoint Hit:", self.i)
+        gdb.execute('print i')
+        return False
+
+MyBreakpoint('i', gdb.BP_WATCHPOINT)
+
+print("Python script imported")
-- 
2.34.1

base-commit: c99b2113a478a075e8d8f7c3848a92f1ce73f847

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

* Re: [PATCH] [PR python/29603] Fix deletion of Watchpoints
  2022-09-25  5:33 [PATCH] [PR python/29603] Fix deletion of Watchpoints Johnson Sun
@ 2022-09-25 18:10 ` Lancelot SIX
  2022-10-01  5:20 ` Johnson Sun
  1 sibling, 0 replies; 9+ messages in thread
From: Lancelot SIX @ 2022-09-25 18:10 UTC (permalink / raw)
  To: Johnson Sun; +Cc: gdb-patches

Hi,

Thanks for the patch!

First, I have not really explored why, but running your testcase with
current master GDB gives me:

    $ make check-gdb TESTS=gdb.python/py-watchpoint.exp
    Running .../gdb/testsuite/gdb.python/py-watchpoint.exp ...

                === gdb Summary ===

    # of expected passes            6

which is strange.  I was expecting FAILs there.  Is it only on my system
(ubuntu 22.04 on x86_64)?

I can however reproduce the problem by running the ticket’s testcase
manually and observe the issue you describe.

On Sun, Sep 25, 2022 at 01:33:50PM +0800, Johnson Sun via Gdb-patches wrote:
> Currently, during normal stop (i.e., stop and waiting for the next command),
> GDB automatically deletes local Watchpoints that are out of scope. However,
> local Watchpoints are not deleted if a Python script decides not to normal
> stop upon hit, causing them to be hit many more times than they should, as
> reported in PR python/29603. This was happening because the watchpoint is not
> disabled when going out of scope.
> 
> This commit fixes this issue by disabling the watchpoint when going out of
> scope. It also adds a test to ensure this feature isn't regressed in the
> future.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29603
> ---
>  gdb/breakpoint.c                           |  1 +
>  gdb/testsuite/gdb.python/py-watchpoint.c   | 27 +++++++++++++
>  gdb/testsuite/gdb.python/py-watchpoint.exp | 44 ++++++++++++++++++++++
>  gdb/testsuite/gdb.python/py-watchpoint.py  | 31 +++++++++++++++
>  4 files changed, 103 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.c
>  create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.exp
>  create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.py
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index bff3bac7d1a..b78ae9c4993 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -1832,6 +1832,7 @@ watchpoint_del_at_next_stop (struct watchpoint *w)
>        w->related_breakpoint = w;
>      }
>    w->disposition = disp_del_at_next_stop;
> +  disable_breakpoint(w);

From what I understand, the problem you are trying to solve is linked to
extension language (python in this case), so it seems odd to me that the
fix is in a non-extension related code path.

If my understanding is correct, the python extension you provide should
work similarly to typing say "watch i if 0".  So my approach would be to
have something similar between checking the watchpoint condition and
evaluating the `stop` method from you python class.

In the condition case, `disable_breakpoint` is not used before the
breakpoint is deleted.  Instead, the code checks "b->disposition !=
disp_del_at_next_stop" (in bpstat_check_breakpoint_conditions) before
trying to evaluate the condition.  I guess that this is what you also
want to do in your case.

In the same function, you can have the following change:

    diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
    index 002f4a935b1..e8d3fd0d50a 100644
    --- a/gdb/breakpoint.c
    +++ b/gdb/breakpoint.c
    @@ -5339,7 +5339,8 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
     
       /* Evaluate extension language breakpoints that have a "stop" method
          implemented.  */
    -  bs->stop = breakpoint_ext_lang_cond_says_stop (b);
    +  if (b->disposition != disp_del_at_next_stop)
    +    bs->stop = breakpoint_ext_lang_cond_says_stop (b);
     
       if (is_watchpoint (b))
         {

I believe this is what you are looking for.  That being said, this is my
approach to solve your problem, a maintainer might have a better way to
address this.

>  }
>  
>  /* Extract a bitfield value from value VAL using the bit parameters contained in
> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.c b/gdb/testsuite/gdb.python/py-watchpoint.c
> new file mode 100644
> index 00000000000..6d02e87e571
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-watchpoint.c
> @@ -0,0 +1,27 @@
> +/* 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/>.  */
> +
> +#include <stdio.h>
> +
> +int
> +main ()
> +{
> +  int i;
> +  for (i = 0; i < 3; i++)
> +    printf ("%d", i);
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.exp b/gdb/testsuite/gdb.python/py-watchpoint.exp
> new file mode 100644
> index 00000000000..863f3eff66a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-watchpoint.exp
> @@ -0,0 +1,44 @@
> +# Copyright (C) 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/>.
> +
> +# This file is part of the GDB testsuite.  It checks Watchpoints
> +# are deleted after used.
> +
> +load_lib gdb-python.exp
> +
> +standard_testfile
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> +    return -1
> +}
> +
> +# Skip all tests if Python scripting is not enabled.
> +if { [skip_python_tests] } { continue }
> +
> +if ![runto_main] then {
> +    return 0
> +}
> +
> +#
> +# Check Watchpoints are deleted after used
> +#
> +
> +gdb_test "set can-use-hw-watchpoints 0" ".*" "Don't use hardware watchpoints"

In this case, you could probably use gdb_test_no_output

> +gdb_test "python print (len(gdb.breakpoints()))" "1" "check default BP count"

It looks odd to have a space after print and not before the other
parenthesis.  To follow python coding convention, I would remove the
space between "print" and the "(".  Same remark holds for the two other
occurrences of this pattern below.

Best,
Lancelot.

> +gdb_test "source ${srcdir}/${subdir}/${testfile}.py" ".*Python script imported.*" \
> +	 "import python scripts"
> +gdb_test "python print (len(gdb.breakpoints()))" "2" "check modified BP count"
> +gdb_test "continue" ".*Watchpoint Hit: 4.*" "run until program stops"
> +gdb_test "python print (len(gdb.breakpoints()))" "1" "check BP count"
> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.py b/gdb/testsuite/gdb.python/py-watchpoint.py
> new file mode 100644
> index 00000000000..855c820b245
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-watchpoint.py
> @@ -0,0 +1,31 @@
> +# Copyright (C) 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/>.
> +
> +# This file is part of the GDB testsuite.  It checks Watchpoints
> +# are deleted after used.
> +
> +class MyBreakpoint(gdb.Breakpoint):
> +    def __init__(self, *args, **kwargs):
> +        gdb.Breakpoint.__init__(self, *args, **kwargs)
> +        self.i = 0
> +    def stop(self):
> +        self.i += 1
> +        print("Watchpoint Hit:", self.i)
> +        gdb.execute('print i')
> +        return False
> +
> +MyBreakpoint('i', gdb.BP_WATCHPOINT)
> +
> +print("Python script imported")
> -- 
> 2.34.1
> 
> base-commit: c99b2113a478a075e8d8f7c3848a92f1ce73f847

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

* Re: [PATCH] [PR python/29603] Fix deletion of Watchpoints
  2022-09-25  5:33 [PATCH] [PR python/29603] Fix deletion of Watchpoints Johnson Sun
  2022-09-25 18:10 ` Lancelot SIX
@ 2022-10-01  5:20 ` Johnson Sun
  2022-10-01  5:27   ` [PATCH v2] [PR python/29603] Disable out-of-scope watchpoints Johnson Sun
  1 sibling, 1 reply; 9+ messages in thread
From: Johnson Sun @ 2022-10-01  5:20 UTC (permalink / raw)
  To: Lancelot SIX, gdb-patches; +Cc: j3.soon777

Hi Lancelot,

Thanks for reviewing this!  I revised the patch and 

will send version 2 shortly.

Cheers,
Johnson


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

* [PATCH v2] [PR python/29603] Disable out-of-scope watchpoints
  2022-10-01  5:20 ` Johnson Sun
@ 2022-10-01  5:27   ` Johnson Sun
  2022-10-20 17:57     ` Johnson Sun
  0 siblings, 1 reply; 9+ messages in thread
From: Johnson Sun @ 2022-10-01  5:27 UTC (permalink / raw)
  To: Lancelot SIX, gdb-patches; +Cc: j3.soon777

Currently, when a local software watchpoint goes out of scope, GDB sets the
watchpoint's disposition to `delete at next stop' and then normal stops
(i.e., stop and wait for the next GDB command). When GDB normal stops, it
automatically deletes the breakpoints with disposition set to
`delete at next stop'.

Suppose a Python script decides not to normal stop when a local software
watchpoint goes out of scope, the watchpoint will not be automatically
deleted even when its disposition is set to `delete at next stop'.

Since GDB single-steps the program and tests the watched expression after each
instruction, not deleting the watchpoint causes the watchpoint to be hit many
more times than it should, as reported in PR python/29603.

This was happening because the watchpoint is not disabled when going out of
scope.

This commit fixes this issue by disabling the watchpoint when going out of
scope. It also adds a test to ensure this feature isn't regressed in the
future.

There are two other solutions that seem to solve this issue but don't:
1. Automatically delete breakpoints on all kinds of stops
   (in `fetch_inferior_event'). This solution is very slow since the deletion
   requires O(N) time for N breakpoints.
2. Disable the watchpoint when the watchpoint's disposition is set to
   `delete at next stop' (in `watchpoint_del_at_next_stop'). This solution
   will not trigger a breakpoint stop callback when the watchpoint goes out of
   scope.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29603
---
 gdb/breakpoint.c                           |  2 +
 gdb/testsuite/gdb.python/py-watchpoint.c   | 27 ++++++++++++
 gdb/testsuite/gdb.python/py-watchpoint.exp | 50 ++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-watchpoint.py  | 30 +++++++++++++
 4 files changed, 109 insertions(+)
 create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.c
 create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.exp
 create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.py

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index bff3bac7d1a..15f4ae2131c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5340,6 +5340,8 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
   /* Evaluate extension language breakpoints that have a "stop" method
      implemented.  */
   bs->stop = breakpoint_ext_lang_cond_says_stop (b);
+  if (b->disposition == disp_del_at_next_stop)
+    disable_breakpoint(b);
 
   if (is_watchpoint (b))
     {
diff --git a/gdb/testsuite/gdb.python/py-watchpoint.c b/gdb/testsuite/gdb.python/py-watchpoint.c
new file mode 100644
index 00000000000..6d02e87e571
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-watchpoint.c
@@ -0,0 +1,27 @@
+/* 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/>.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  int i;
+  for (i = 0; i < 3; i++)
+    printf ("%d", i);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-watchpoint.exp b/gdb/testsuite/gdb.python/py-watchpoint.exp
new file mode 100644
index 00000000000..a14ab139a3f
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-watchpoint.exp
@@ -0,0 +1,50 @@
+# Copyright (C) 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/>.
+
+# This file is part of the GDB testsuite.  It checks Watchpoints
+# are deleted after used.
+
+load_lib gdb-python.exp
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if ![runto_main] then {
+    return 0
+}
+
+#
+# Check Watchpoints are deleted after used
+#
+
+gdb_test_no_output "set can-use-hw-watchpoints 0" "Don't use hardware watchpoints"
+gdb_test "python print(len(gdb.breakpoints()))" "1" "check default BP count"
+gdb_test "source ${srcdir}/${subdir}/${testfile}.py" ".*Python script imported.*" \
+	 "import python scripts"
+gdb_test "python print(len(gdb.breakpoints()))" "2" "check modified BP count"
+gdb_test_sequence "continue" "run until program stops" {
+    "Watchpoint Hit: ."
+    "\[\r\n\]+Watchpoint . deleted because the program has left the block in"
+    "\[\r\n\]+which its expression is valid\."
+    "Watchpoint Hit: ."
+    "\[\r\n\]+012\\[Inferior 1 \\(process .*\\) exited normally\\]"
+}
+gdb_test "python print(len(gdb.breakpoints()))" "1" "check BP count"
diff --git a/gdb/testsuite/gdb.python/py-watchpoint.py b/gdb/testsuite/gdb.python/py-watchpoint.py
new file mode 100644
index 00000000000..2c668aeca77
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-watchpoint.py
@@ -0,0 +1,30 @@
+# Copyright (C) 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/>.
+
+# This file is part of the GDB testsuite.  It checks Watchpoints
+# are deleted after used.
+
+class MyBreakpoint(gdb.Breakpoint):
+    def __init__(self, *args, **kwargs):
+        gdb.Breakpoint.__init__(self, *args, **kwargs)
+        self.i = 0
+    def stop(self):
+        self.i += 1
+        print("Watchpoint Hit:", self.i)
+        return False
+
+MyBreakpoint('i', gdb.BP_WATCHPOINT)
+
+print("Python script imported")
-- 
2.34.1

base-commit: c99b2113a478a075e8d8f7c3848a92f1ce73f847

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

* Re: [PATCH v2] [PR python/29603] Disable out-of-scope watchpoints
  2022-10-01  5:27   ` [PATCH v2] [PR python/29603] Disable out-of-scope watchpoints Johnson Sun
@ 2022-10-20 17:57     ` Johnson Sun
  2022-10-20 18:11       ` [PATCH v3] " Johnson Sun
  0 siblings, 1 reply; 9+ messages in thread
From: Johnson Sun @ 2022-10-20 17:57 UTC (permalink / raw)
  To: Lancelot SIX, gdb-patches; +Cc: j3.soon777

Hi,

I revised the patch after further investigation and 

will send version 3 shortly.

Cheers,
Johnson


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

* [PATCH v3] [PR python/29603] Disable out-of-scope watchpoints
  2022-10-20 17:57     ` Johnson Sun
@ 2022-10-20 18:11       ` Johnson Sun
  2022-11-18 12:17         ` [PING] " Johnson Sun
  0 siblings, 1 reply; 9+ messages in thread
From: Johnson Sun @ 2022-10-20 18:11 UTC (permalink / raw)
  To: Lancelot SIX, gdb-patches; +Cc: j3.soon777

Currently, when a local software watchpoint goes out of scope, GDB sets the
watchpoint's disposition to `delete at next stop' and then normal stops
(i.e., stop and wait for the next GDB command). When GDB normal stops, it
automatically deletes the breakpoints with their disposition set to
`delete at next stop'.

Suppose a Python script decides not to normal stop when a local software
watchpoint goes out of scope, the watchpoint will not be automatically
deleted even when its disposition is set to `delete at next stop'.

Since GDB single-steps the program and tests the watched expression after each
instruction, not deleting the watchpoint causes the watchpoint to be hit many
more times than it should, as reported in PR python/29603.

This was happening because the watchpoint is not deleted or disabled when
going out of scope.

This commit fixes this issue by disabling the watchpoint when going out of
scope. It also adds a test to ensure this feature isn't regressed in the
future.

Two other solutions seem to solve this issue, but are in fact inappropriate:
1. Automatically delete breakpoints on all kinds of stops
   (in `fetch_inferior_event'). This solution is very slow since the deletion
   requires O(N) time for N breakpoints.
2. Disable the watchpoint after the watchpoint's disposition is set to
   `delete at next stop' (in `watchpoint_del_at_next_stop'). This solution
   modifies a non-extension-related code path, and isn't preferred since this
   issue cannot occur without extension languages. (gdb scripts always normal
   stop before continuing execution)

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29603
---
 gdb/breakpoint.c                           |  2 +
 gdb/testsuite/gdb.python/py-watchpoint.c   | 27 ++++++++++++
 gdb/testsuite/gdb.python/py-watchpoint.exp | 48 ++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-watchpoint.py  | 30 ++++++++++++++
 4 files changed, 107 insertions(+)
 create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.c
 create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.exp
 create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.py

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index bff3bac7d1a..15f4ae2131c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5340,6 +5340,8 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
   /* Evaluate extension language breakpoints that have a "stop" method
      implemented.  */
   bs->stop = breakpoint_ext_lang_cond_says_stop (b);
+  if (b->disposition == disp_del_at_next_stop)
+    disable_breakpoint(b);
 
   if (is_watchpoint (b))
     {
diff --git a/gdb/testsuite/gdb.python/py-watchpoint.c b/gdb/testsuite/gdb.python/py-watchpoint.c
new file mode 100644
index 00000000000..4e1760bb05b
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-watchpoint.c
@@ -0,0 +1,27 @@
+/* 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/>.  */
+
+#include <stdio.h>
+
+int
+main (void)
+{
+  int i;
+  for (i = 0; i < 3; i++)
+    printf ("%d", i);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-watchpoint.exp b/gdb/testsuite/gdb.python/py-watchpoint.exp
new file mode 100644
index 00000000000..ac58d75c523
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-watchpoint.exp
@@ -0,0 +1,48 @@
+# Copyright (C) 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 that Watchpoints are deleted after use.
+
+load_lib gdb-python.exp
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if ![runto_main] then {
+    return 0
+}
+
+# For remote host testing
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
+
+gdb_test_no_output "set can-use-hw-watchpoints 0" "Don't use hardware watchpoints"
+gdb_test "python print(len(gdb.breakpoints()))" "1" "check default BP count"
+gdb_test "source $pyfile" ".*Python script imported.*" \
+    "import python scripts"
+gdb_test "python print(len(gdb.breakpoints()))" "2" "check modified BP count"
+gdb_test_sequence "continue" "run until program stops" {
+    "Watchpoint Hit: ."
+    "\[\r\n\]+Watchpoint . deleted because the program has left the block in"
+    "\[\r\n\]+which its expression is valid\."
+    "Watchpoint Hit: ."
+    "\[\r\n\]+012\\[Inferior 1 \\(process .*\\) exited normally\\]"
+}
+gdb_test "python print(len(gdb.breakpoints()))" "1" "check BP count"
diff --git a/gdb/testsuite/gdb.python/py-watchpoint.py b/gdb/testsuite/gdb.python/py-watchpoint.py
new file mode 100644
index 00000000000..ce5dee118ad
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-watchpoint.py
@@ -0,0 +1,30 @@
+# Copyright (C) 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/>.
+
+
+class MyBreakpoint(gdb.Breakpoint):
+    def __init__(self, *args, **kwargs):
+        gdb.Breakpoint.__init__(self, *args, **kwargs)
+        self.i = 0
+
+    def stop(self):
+        self.i += 1
+        print("Watchpoint Hit:", self.i)
+        return False
+
+
+MyBreakpoint("i", gdb.BP_WATCHPOINT)
+
+print("Python script imported")
-- 
2.34.1

base-commit: c99b2113a478a075e8d8f7c3848a92f1ce73f847

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

* [PING] [PATCH v3] [PR python/29603] Disable out-of-scope watchpoints
  2022-10-20 18:11       ` [PATCH v3] " Johnson Sun
@ 2022-11-18 12:17         ` Johnson Sun
  2022-11-25 15:11           ` [PING^2] " Johnson Sun
  0 siblings, 1 reply; 9+ messages in thread
From: Johnson Sun @ 2022-11-18 12:17 UTC (permalink / raw)
  To: Lancelot SIX, gdb-patches

Ping for: 
<https://sourceware.org/pipermail/gdb-patches/2022-October/192909.html>.

Johnson

On 10/21/2022 2:11 AM, Johnson Sun wrote:
> Currently, when a local software watchpoint goes out of scope, GDB sets the
> watchpoint's disposition to `delete at next stop' and then normal stops
> (i.e., stop and wait for the next GDB command). When GDB normal stops, it
> automatically deletes the breakpoints with their disposition set to
> `delete at next stop'.
>
> Suppose a Python script decides not to normal stop when a local software
> watchpoint goes out of scope, the watchpoint will not be automatically
> deleted even when its disposition is set to `delete at next stop'.
>
> Since GDB single-steps the program and tests the watched expression after each
> instruction, not deleting the watchpoint causes the watchpoint to be hit many
> more times than it should, as reported in PR python/29603.
>
> This was happening because the watchpoint is not deleted or disabled when
> going out of scope.
>
> This commit fixes this issue by disabling the watchpoint when going out of
> scope. It also adds a test to ensure this feature isn't regressed in the
> future.
>
> Two other solutions seem to solve this issue, but are in fact inappropriate:
> 1. Automatically delete breakpoints on all kinds of stops
>     (in `fetch_inferior_event'). This solution is very slow since the deletion
>     requires O(N) time for N breakpoints.
> 2. Disable the watchpoint after the watchpoint's disposition is set to
>     `delete at next stop' (in `watchpoint_del_at_next_stop'). This solution
>     modifies a non-extension-related code path, and isn't preferred since this
>     issue cannot occur without extension languages. (gdb scripts always normal
>     stop before continuing execution)
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29603
> ---
>   gdb/breakpoint.c                           |  2 +
>   gdb/testsuite/gdb.python/py-watchpoint.c   | 27 ++++++++++++
>   gdb/testsuite/gdb.python/py-watchpoint.exp | 48 ++++++++++++++++++++++
>   gdb/testsuite/gdb.python/py-watchpoint.py  | 30 ++++++++++++++
>   4 files changed, 107 insertions(+)
>   create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.c
>   create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.exp
>   create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.py
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index bff3bac7d1a..15f4ae2131c 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -5340,6 +5340,8 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
>     /* Evaluate extension language breakpoints that have a "stop" method
>        implemented.  */
>     bs->stop = breakpoint_ext_lang_cond_says_stop (b);
> +  if (b->disposition == disp_del_at_next_stop)
> +    disable_breakpoint(b);
>   
>     if (is_watchpoint (b))
>       {
> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.c b/gdb/testsuite/gdb.python/py-watchpoint.c
> new file mode 100644
> index 00000000000..4e1760bb05b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-watchpoint.c
> @@ -0,0 +1,27 @@
> +/* 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/>.  */
> +
> +#include <stdio.h>
> +
> +int
> +main (void)
> +{
> +  int i;
> +  for (i = 0; i < 3; i++)
> +    printf ("%d", i);
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.exp b/gdb/testsuite/gdb.python/py-watchpoint.exp
> new file mode 100644
> index 00000000000..ac58d75c523
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-watchpoint.exp
> @@ -0,0 +1,48 @@
> +# Copyright (C) 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 that Watchpoints are deleted after use.
> +
> +load_lib gdb-python.exp
> +
> +standard_testfile
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> +    return -1
> +}
> +
> +# Skip all tests if Python scripting is not enabled.
> +if { [skip_python_tests] } { continue }
> +
> +if ![runto_main] then {
> +    return 0
> +}
> +
> +# For remote host testing
> +set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
> +
> +gdb_test_no_output "set can-use-hw-watchpoints 0" "Don't use hardware watchpoints"
> +gdb_test "python print(len(gdb.breakpoints()))" "1" "check default BP count"
> +gdb_test "source $pyfile" ".*Python script imported.*" \
> +    "import python scripts"
> +gdb_test "python print(len(gdb.breakpoints()))" "2" "check modified BP count"
> +gdb_test_sequence "continue" "run until program stops" {
> +    "Watchpoint Hit: ."
> +    "\[\r\n\]+Watchpoint . deleted because the program has left the block in"
> +    "\[\r\n\]+which its expression is valid\."
> +    "Watchpoint Hit: ."
> +    "\[\r\n\]+012\\[Inferior 1 \\(process .*\\) exited normally\\]"
> +}
> +gdb_test "python print(len(gdb.breakpoints()))" "1" "check BP count"
> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.py b/gdb/testsuite/gdb.python/py-watchpoint.py
> new file mode 100644
> index 00000000000..ce5dee118ad
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-watchpoint.py
> @@ -0,0 +1,30 @@
> +# Copyright (C) 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/>.
> +
> +
> +class MyBreakpoint(gdb.Breakpoint):
> +    def __init__(self, *args, **kwargs):
> +        gdb.Breakpoint.__init__(self, *args, **kwargs)
> +        self.i = 0
> +
> +    def stop(self):
> +        self.i += 1
> +        print("Watchpoint Hit:", self.i)
> +        return False
> +
> +
> +MyBreakpoint("i", gdb.BP_WATCHPOINT)
> +
> +print("Python script imported")

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

* [PING^2] [PATCH v3] [PR python/29603] Disable out-of-scope watchpoints
  2022-11-18 12:17         ` [PING] " Johnson Sun
@ 2022-11-25 15:11           ` Johnson Sun
  2022-12-04 16:45             ` [PING^3] " Johnson Sun
  0 siblings, 1 reply; 9+ messages in thread
From: Johnson Sun @ 2022-11-25 15:11 UTC (permalink / raw)
  To: LancelotSIX, gdb-patches

Ping for: 
<https://sourceware.org/pipermail/gdb-patches/2022-October/192909.html>.

Johnson

On 11/18/2022 8:17 PM, JohnsonSun wrote:
> Ping for: 
> <https://sourceware.org/pipermail/gdb-patches/2022-October/192909.html>.
>
> Johnson
>
> On 10/21/2022 2:11 AM, Johnson Sun wrote:
>> Currently, when a local software watchpoint goes out of scope, GDB 
>> sets the
>> watchpoint's disposition to `delete at next stop' and then normal stops
>> (i.e., stop and wait for the next GDB command). When GDB normal 
>> stops, it
>> automatically deletes the breakpoints with their disposition set to
>> `delete at next stop'.
>>
>> Suppose a Python script decides not to normal stop when a local software
>> watchpoint goes out of scope, the watchpoint will not be automatically
>> deleted even when its disposition is set to `delete at next stop'.
>>
>> Since GDB single-steps the program and tests the watched expression 
>> after each
>> instruction, not deleting the watchpoint causes the watchpoint to be 
>> hit many
>> more times than it should, as reported in PR python/29603.
>>
>> This was happening because the watchpoint is not deleted or disabled 
>> when
>> going out of scope.
>>
>> This commit fixes this issue by disabling the watchpoint when going 
>> out of
>> scope. It also adds a test to ensure this feature isn't regressed in the
>> future.
>>
>> Two other solutions seem to solve this issue, but are in fact 
>> inappropriate:
>> 1. Automatically delete breakpoints on all kinds of stops
>>     (in `fetch_inferior_event'). This solution is very slow since the 
>> deletion
>>     requires O(N) time for N breakpoints.
>> 2. Disable the watchpoint after the watchpoint's disposition is set to
>>     `delete at next stop' (in `watchpoint_del_at_next_stop'). This 
>> solution
>>     modifies a non-extension-related code path, and isn't preferred 
>> since this
>>     issue cannot occur without extension languages. (gdb scripts 
>> always normal
>>     stop before continuing execution)
>>
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29603
>> ---
>>   gdb/breakpoint.c                           |  2 +
>>   gdb/testsuite/gdb.python/py-watchpoint.c   | 27 ++++++++++++
>>   gdb/testsuite/gdb.python/py-watchpoint.exp | 48 ++++++++++++++++++++++
>>   gdb/testsuite/gdb.python/py-watchpoint.py  | 30 ++++++++++++++
>>   4 files changed, 107 insertions(+)
>>   create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.c
>>   create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.exp
>>   create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.py
>>
>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>> index bff3bac7d1a..15f4ae2131c 100644
>> --- a/gdb/breakpoint.c
>> +++ b/gdb/breakpoint.c
>> @@ -5340,6 +5340,8 @@ bpstat_check_breakpoint_conditions (bpstat *bs, 
>> thread_info *thread)
>>     /* Evaluate extension language breakpoints that have a "stop" method
>>        implemented.  */
>>     bs->stop = breakpoint_ext_lang_cond_says_stop (b);
>> +  if (b->disposition == disp_del_at_next_stop)
>> +    disable_breakpoint(b);
>>       if (is_watchpoint (b))
>>       {
>> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.c 
>> b/gdb/testsuite/gdb.python/py-watchpoint.c
>> new file mode 100644
>> index 00000000000..4e1760bb05b
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.python/py-watchpoint.c
>> @@ -0,0 +1,27 @@
>> +/* 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/>.  */
>> +
>> +#include <stdio.h>
>> +
>> +int
>> +main (void)
>> +{
>> +  int i;
>> +  for (i = 0; i < 3; i++)
>> +    printf ("%d", i);
>> +  return 0;
>> +}
>> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.exp 
>> b/gdb/testsuite/gdb.python/py-watchpoint.exp
>> new file mode 100644
>> index 00000000000..ac58d75c523
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.python/py-watchpoint.exp
>> @@ -0,0 +1,48 @@
>> +# Copyright (C) 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 that Watchpoints are deleted after use.
>> +
>> +load_lib gdb-python.exp
>> +
>> +standard_testfile
>> +
>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
>> +    return -1
>> +}
>> +
>> +# Skip all tests if Python scripting is not enabled.
>> +if { [skip_python_tests] } { continue }
>> +
>> +if ![runto_main] then {
>> +    return 0
>> +}
>> +
>> +# For remote host testing
>> +set pyfile [gdb_remote_download host 
>> ${srcdir}/${subdir}/${testfile}.py]
>> +
>> +gdb_test_no_output "set can-use-hw-watchpoints 0" "Don't use 
>> hardware watchpoints"
>> +gdb_test "python print(len(gdb.breakpoints()))" "1" "check default 
>> BP count"
>> +gdb_test "source $pyfile" ".*Python script imported.*" \
>> +    "import python scripts"
>> +gdb_test "python print(len(gdb.breakpoints()))" "2" "check modified 
>> BP count"
>> +gdb_test_sequence "continue" "run until program stops" {
>> +    "Watchpoint Hit: ."
>> +    "\[\r\n\]+Watchpoint . deleted because the program has left the 
>> block in"
>> +    "\[\r\n\]+which its expression is valid\."
>> +    "Watchpoint Hit: ."
>> +    "\[\r\n\]+012\\[Inferior 1 \\(process .*\\) exited normally\\]"
>> +}
>> +gdb_test "python print(len(gdb.breakpoints()))" "1" "check BP count"
>> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.py 
>> b/gdb/testsuite/gdb.python/py-watchpoint.py
>> new file mode 100644
>> index 00000000000..ce5dee118ad
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.python/py-watchpoint.py
>> @@ -0,0 +1,30 @@
>> +# Copyright (C) 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/>.
>> +
>> +
>> +class MyBreakpoint(gdb.Breakpoint):
>> +    def __init__(self, *args, **kwargs):
>> +        gdb.Breakpoint.__init__(self, *args, **kwargs)
>> +        self.i = 0
>> +
>> +    def stop(self):
>> +        self.i += 1
>> +        print("Watchpoint Hit:", self.i)
>> +        return False
>> +
>> +
>> +MyBreakpoint("i", gdb.BP_WATCHPOINT)
>> +
>> +print("Python script imported")
>

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

* Re: [PING^3] [PATCH v3] [PR python/29603] Disable out-of-scope watchpoints
  2022-11-25 15:11           ` [PING^2] " Johnson Sun
@ 2022-12-04 16:45             ` Johnson Sun
  0 siblings, 0 replies; 9+ messages in thread
From: Johnson Sun @ 2022-12-04 16:45 UTC (permalink / raw)
  To: LancelotSIX, gdb-patches

Ping for: 
<https://sourceware.org/pipermail/gdb-patches/2022-October/192909.html>.

Johnson

On 11/25/2022 11:11 PM, Johnson Sun wrote:
> Ping for: 
> <https://sourceware.org/pipermail/gdb-patches/2022-October/192909.html>.
>
> Johnson
>
> On 11/18/2022 8:17 PM, JohnsonSun wrote:
>> Ping for: 
>> <https://sourceware.org/pipermail/gdb-patches/2022-October/192909.html>.
>>
>> Johnson
>>
>> On 10/21/2022 2:11 AM, Johnson Sun wrote:
>>> Currently, when a local software watchpoint goes out of scope, GDB 
>>> sets the
>>> watchpoint's disposition to `delete at next stop' and then normal stops
>>> (i.e., stop and wait for the next GDB command). When GDB normal 
>>> stops, it
>>> automatically deletes the breakpoints with their disposition set to
>>> `delete at next stop'.
>>>
>>> Suppose a Python script decides not to normal stop when a local 
>>> software
>>> watchpoint goes out of scope, the watchpoint will not be automatically
>>> deleted even when its disposition is set to `delete at next stop'.
>>>
>>> Since GDB single-steps the program and tests the watched expression 
>>> after each
>>> instruction, not deleting the watchpoint causes the watchpoint to be 
>>> hit many
>>> more times than it should, as reported in PR python/29603.
>>>
>>> This was happening because the watchpoint is not deleted or disabled 
>>> when
>>> going out of scope.
>>>
>>> This commit fixes this issue by disabling the watchpoint when going 
>>> out of
>>> scope. It also adds a test to ensure this feature isn't regressed in 
>>> the
>>> future.
>>>
>>> Two other solutions seem to solve this issue, but are in fact 
>>> inappropriate:
>>> 1. Automatically delete breakpoints on all kinds of stops
>>>     (in `fetch_inferior_event'). This solution is very slow since 
>>> the deletion
>>>     requires O(N) time for N breakpoints.
>>> 2. Disable the watchpoint after the watchpoint's disposition is set to
>>>     `delete at next stop' (in `watchpoint_del_at_next_stop'). This 
>>> solution
>>>     modifies a non-extension-related code path, and isn't preferred 
>>> since this
>>>     issue cannot occur without extension languages. (gdb scripts 
>>> always normal
>>>     stop before continuing execution)
>>>
>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29603
>>> ---
>>>   gdb/breakpoint.c                           |  2 +
>>>   gdb/testsuite/gdb.python/py-watchpoint.c   | 27 ++++++++++++
>>>   gdb/testsuite/gdb.python/py-watchpoint.exp | 48 
>>> ++++++++++++++++++++++
>>>   gdb/testsuite/gdb.python/py-watchpoint.py  | 30 ++++++++++++++
>>>   4 files changed, 107 insertions(+)
>>>   create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.c
>>>   create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.exp
>>>   create mode 100644 gdb/testsuite/gdb.python/py-watchpoint.py
>>>
>>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>>> index bff3bac7d1a..15f4ae2131c 100644
>>> --- a/gdb/breakpoint.c
>>> +++ b/gdb/breakpoint.c
>>> @@ -5340,6 +5340,8 @@ bpstat_check_breakpoint_conditions (bpstat 
>>> *bs, thread_info *thread)
>>>     /* Evaluate extension language breakpoints that have a "stop" 
>>> method
>>>        implemented.  */
>>>     bs->stop = breakpoint_ext_lang_cond_says_stop (b);
>>> +  if (b->disposition == disp_del_at_next_stop)
>>> +    disable_breakpoint(b);
>>>       if (is_watchpoint (b))
>>>       {
>>> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.c 
>>> b/gdb/testsuite/gdb.python/py-watchpoint.c
>>> new file mode 100644
>>> index 00000000000..4e1760bb05b
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.python/py-watchpoint.c
>>> @@ -0,0 +1,27 @@
>>> +/* 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/>.  */
>>> +
>>> +#include <stdio.h>
>>> +
>>> +int
>>> +main (void)
>>> +{
>>> +  int i;
>>> +  for (i = 0; i < 3; i++)
>>> +    printf ("%d", i);
>>> +  return 0;
>>> +}
>>> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.exp 
>>> b/gdb/testsuite/gdb.python/py-watchpoint.exp
>>> new file mode 100644
>>> index 00000000000..ac58d75c523
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.python/py-watchpoint.exp
>>> @@ -0,0 +1,48 @@
>>> +# Copyright (C) 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 that Watchpoints are deleted after use.
>>> +
>>> +load_lib gdb-python.exp
>>> +
>>> +standard_testfile
>>> +
>>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
>>> +    return -1
>>> +}
>>> +
>>> +# Skip all tests if Python scripting is not enabled.
>>> +if { [skip_python_tests] } { continue }
>>> +
>>> +if ![runto_main] then {
>>> +    return 0
>>> +}
>>> +
>>> +# For remote host testing
>>> +set pyfile [gdb_remote_download host 
>>> ${srcdir}/${subdir}/${testfile}.py]
>>> +
>>> +gdb_test_no_output "set can-use-hw-watchpoints 0" "Don't use 
>>> hardware watchpoints"
>>> +gdb_test "python print(len(gdb.breakpoints()))" "1" "check default 
>>> BP count"
>>> +gdb_test "source $pyfile" ".*Python script imported.*" \
>>> +    "import python scripts"
>>> +gdb_test "python print(len(gdb.breakpoints()))" "2" "check modified 
>>> BP count"
>>> +gdb_test_sequence "continue" "run until program stops" {
>>> +    "Watchpoint Hit: ."
>>> +    "\[\r\n\]+Watchpoint . deleted because the program has left the 
>>> block in"
>>> +    "\[\r\n\]+which its expression is valid\."
>>> +    "Watchpoint Hit: ."
>>> +    "\[\r\n\]+012\\[Inferior 1 \\(process .*\\) exited normally\\]"
>>> +}
>>> +gdb_test "python print(len(gdb.breakpoints()))" "1" "check BP count"
>>> diff --git a/gdb/testsuite/gdb.python/py-watchpoint.py 
>>> b/gdb/testsuite/gdb.python/py-watchpoint.py
>>> new file mode 100644
>>> index 00000000000..ce5dee118ad
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.python/py-watchpoint.py
>>> @@ -0,0 +1,30 @@
>>> +# Copyright (C) 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/>.
>>> +
>>> +
>>> +class MyBreakpoint(gdb.Breakpoint):
>>> +    def __init__(self, *args, **kwargs):
>>> +        gdb.Breakpoint.__init__(self, *args, **kwargs)
>>> +        self.i = 0
>>> +
>>> +    def stop(self):
>>> +        self.i += 1
>>> +        print("Watchpoint Hit:", self.i)
>>> +        return False
>>> +
>>> +
>>> +MyBreakpoint("i", gdb.BP_WATCHPOINT)
>>> +
>>> +print("Python script imported")
>>

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

end of thread, other threads:[~2022-12-04 16:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-25  5:33 [PATCH] [PR python/29603] Fix deletion of Watchpoints Johnson Sun
2022-09-25 18:10 ` Lancelot SIX
2022-10-01  5:20 ` Johnson Sun
2022-10-01  5:27   ` [PATCH v2] [PR python/29603] Disable out-of-scope watchpoints Johnson Sun
2022-10-20 17:57     ` Johnson Sun
2022-10-20 18:11       ` [PATCH v3] " Johnson Sun
2022-11-18 12:17         ` [PING] " Johnson Sun
2022-11-25 15:11           ` [PING^2] " Johnson Sun
2022-12-04 16:45             ` [PING^3] " Johnson Sun

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