Message ID | 20230323183118.3172-1-j3.soon777@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D70023870894 for <patchwork@sourceware.org>; Thu, 23 Mar 2023 18:33:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D70023870894 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1679596399; bh=qqPmGolC1V2ziHZ4KZKrtFj7ffMe4NlrfDSgt/lcOdM=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=fZHqyJ4Me33+/ANs2wX1Te1Hkb6zx8qcPwhvARxS6FNgnxiIQ76cqS9VKpbIn+wiz ZUU2+2mPnji/d2LB4uS+9L5dF5/C7uk3G8SMQe39KwXZFQwGlZW5QI9kq4ZiaUcF3h odf21n9BADUY4OEzHSXUxUC7uPISm5Yuf9NeQgLo= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-pj1-x1029.google.com (mail-pj1-x1029.google.com [IPv6:2607:f8b0:4864:20::1029]) by sourceware.org (Postfix) with ESMTPS id 4BF8F38493DE for <gdb-patches@sourceware.org>; Thu, 23 Mar 2023 18:32:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4BF8F38493DE Received: by mail-pj1-x1029.google.com with SMTP id om3-20020a17090b3a8300b0023efab0e3bfso2807788pjb.3 for <gdb-patches@sourceware.org>; Thu, 23 Mar 2023 11:32:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679596374; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=qqPmGolC1V2ziHZ4KZKrtFj7ffMe4NlrfDSgt/lcOdM=; b=zlIHrpgkrflViDPAhdzqrWPcO7fIjiFAf0B+XaXwCevI0CnoAop0qzrvrNby8PpsiB 4IdHUF9g7f0DqdLpyQzjasyBH3YeiRf7acDK9yGoS16sIJOw4dn6ViHGSfAFNwpR8PQ+ UhOOU4FTCZWg6WUFjvaN2CT/4ewfAX8V+G4vJXbOqZERT31MrsVp8oYIl6po790ILBXO ylhel09NIL6cM4j5O5nifPX/8KbhS8NPlQMeShmTHFFqh7wkAJBNd9cAQkwfhZxvLu2x jZcoNo/49b7zNdSpVlfpgVW3rVVxv7AitbButwV5UHLcWW0IJM8wew3u+peJfY7DoxCE jhPA== X-Gm-Message-State: AAQBX9exeP5BqLT1hVmYJ7ufQMpLwil3HPz5IjUA2z6XqcKhCix58oXI WLlKCGADLKH/mOa7WjWRFtQouyH84XJ2Tg== X-Google-Smtp-Source: AKy350Yw/i+Y2C12SOsjerqx+mvzqZBEDJdcL6rcbqwohIgWVq5lGzcLg1GTlloOAru4qoCclWMqbA== X-Received: by 2002:a17:90b:4a46:b0:22b:b832:d32 with SMTP id lb6-20020a17090b4a4600b0022bb8320d32mr21810pjb.9.1679596374360; Thu, 23 Mar 2023 11:32:54 -0700 (PDT) Received: from ubuntu.. (intel10.cs.nthu.edu.tw. [140.114.89.60]) by smtp.gmail.com with ESMTPSA id mv21-20020a17090b199500b0023d16f05dd8sm1608951pjb.36.2023.03.23.11.32.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Mar 2023 11:32:53 -0700 (PDT) To: SimonMarchi <simark@simark.ca>, Lancelot SIX <lsix@lancelotsix.com>, gdb-patches@sourceware.org Cc: j3.soon777@gmail.com Subject: [PATCH v4] [PR python/29603] Disable out-of-scope watchpoints Date: Fri, 24 Mar 2023 02:31:21 +0800 Message-Id: <20230323183118.3172-1-j3.soon777@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <0e614d96-52dc-6e14-ea23-8aca19c5414c@gmail.com> References: <0e614d96-52dc-6e14-ea23-8aca19c5414c@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Johnson Sun via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Johnson Sun <j3.soon777@gmail.com> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
[v4,PR,python/29603] Disable out-of-scope watchpoints
|
|
Commit Message
Johnson Sun
March 23, 2023, 6:31 p.m. UTC
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
Comments
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")
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") >
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") >> >
> 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 } This needs to be updated to: require allow_python_tests > + > +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" Avoid using hardcoded line numbers. Either break on a symbol (function) name, or put a comment in the source code and use gdb_get_line_number. > 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 I think it would be a bit more straightforward to read if you passed the arguments to the base class directly: def __init__(self): super().__init__("i", gdb.BP_WATCHPOINT) self.n = 0 I think we can use the super() notation, now that we only support Python 3? > + > + def stop(self): > + self.n += 1 > + print("Watchpoint Hit:", self.n, flush=True) > + return False > + > + > +MyBreakpoint("i", gdb.BP_WATCHPOINT) > + > +print("Python script imported") I tried your test with the native-gdbserver and native-extended-gdbserver boards, and for some reason it doesn't work. You can try with: $ make check TESTS="gdb.python/py-watchpoint.exp" RUNTESTFLAGS="--target_board=native-extended-gdbserver" In gdb.log: ... Python script imported (gdb) PASS: gdb.python/py-watchpoint.exp: import python scripts python print(len(gdb.breakpoints())) 2 (gdb) PASS: gdb.python/py-watchpoint.exp: check modified BP count gdb_expect_list pattern: /Watchpoint Hit: 1/ continue Continuing. Watchpoint Hit: 1 gdb_expect_list pattern: /Watchpoint Hit: 2/ FAIL: gdb.python/py-watchpoint.exp: run until program stops (pattern 2) (timeout) I didn't dig to understand why, but we would need to fix that before merging. Simon
Hi Simon, Thanks for reviewing this! I revised the patch and will send version 5 shortly. The new version has been tested on the native-gdbserver and native-extended-gdbserver boards. Cheers, Johnson
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")