[PING^2] [PATCH v4] [PR python/29603] Disable out-of-scope watchpoints

[Johnson Sun <j3.soon777@gmail.com>]

Ping for: 
<https://sourceware.org/pipermail/gdb-patches/2023-March/198253.html>

Johnson

On 4/10/2023 4:49 AM, JohnsonSun wrote:
> I forgot to add the `[PING]` prefix in the last email. Corrected in 
> this email. Thanks.
>
> Johnson
>
> On 4/10/2023 4:47 AM, JohnsonSun wrote:
>> Ping for: 
>> <https://sourceware.org/pipermail/gdb-patches/2023-March/198253.html>
>>
>> Johnson
>>
>> On 3/24/2023 2:31 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.
>>>
>>> Calling `breakpoint_auto_delete' on all kinds of stops (in
>>> `fetch_inferior_event') seem to solve this issue, but is in fact
>>> inappropriate, since `breakpoint_auto_delete' goes over all breakpoints
>>> instead of just going through the bpstat chain (which only contains the
>>> breakpoints that were hit right now).
>>>
>>> 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 | 54 
>>> ++++++++++++++++++++++
>>>   gdb/testsuite/gdb.python/py-watchpoint.py  | 30 ++++++++++++
>>>   4 files changed, 112 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 bff3bac7d1..47dcf1e127 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 0000000000..0c58b31647
>>> --- /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 = -1;
>>> +  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 0000000000..5504736629
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.python/py-watchpoint.exp
>>> @@ -0,0 +1,54 @@
>>> +# 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 "break 24" ".*" "set breakpoint before loop"
>>> +gdb_test "continue" ".*" "run until reaching loop"
>>> +gdb_test "clear" ".*" "delete the breakpoint before loop"
>>> +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: 1"
>>> +    "Watchpoint Hit: 2"
>>> +    "Watchpoint Hit: 3"
>>> +    "Watchpoint Hit: 4"
>>> +    "Watchpoint 3 deleted because the program has left the block in"
>>> +    "which its expression is valid\."
>>> +    "Watchpoint Hit: 5"
>>> +    "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 0000000000..c5fae00e93
>>> --- /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.n = 0
>>> +
>>> +    def stop(self):
>>> +        self.n += 1
>>> +        print("Watchpoint Hit:", self.n, flush=True)
>>> +        return False
>>> +
>>> +
>>> +MyBreakpoint("i", gdb.BP_WATCHPOINT)
>>> +
>>> +print("Python script imported")
>>
>