From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by sourceware.org (Postfix) with ESMTPS id 3818D3858421 for ; Tue, 20 Dec 2022 22:08:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3818D3858421 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pj1-x102c.google.com with SMTP id n65-20020a17090a2cc700b0021bc5ef7a14so215735pjd.0 for ; Tue, 20 Dec 2022 14:08:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:references:to:from :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=ncE7xbIV/9LLVRsoQJ+JM7JERc+LiPSLwCE7g27JlZA=; b=S8t5dqEi3mojS6eas08QNr91+3RgQI5ppD86dubobH57rxhcDZVVDYJE5Ob1xUlApy 5t1OjZNmkXRqaCuDHq0BdpP9IMxC6TELAkx36NgeonYUcSnits+wkJHk4cVIC/j4Kf2f hhIJ7wKJZjojACeRzPs7baqDM21ew4zgq+eqweY2rEQtBUx+u3FlwoIpl2BBRYKAPxSG 6IccYHECKH7cCVB8cz1yl9WMe4Iwv0tmYoNZQVYlotVhTZxuiNjsjuopjllYfqI6On4b op+XKBhH8xblNmgw18Cwoh/e6V3Q10K6TM/Xr9/6FqaszTKtiqPdmfUFe9/oh+xjaFoP jpaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:references:to:from :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ncE7xbIV/9LLVRsoQJ+JM7JERc+LiPSLwCE7g27JlZA=; b=T/mGBnS/cmam5AA0k+CCyey7P93mdMYVevKo8iGErxKbB6RJYGSzoRA8EFg662DwsA M1xvmHNy3Il+WNy2OycTBx+YFkgi0N1BL5AOZvNKwd4qq2oX7pYCwGf4WNxMfL1QzLOV 55H1oYlCRAVf7KgELRcjZ2dxe0DTIHRHQpj96Ryk3pOdWzYdFdKsnVEsEIG9ezyHQaxY oxsfuqKyJ5fvESOudW+fHQASGR022XTVK1TQG2rsK+6r01QhOcrS4q/SDtPwq7qPtXR/ m8BigBDfYBmP7NUvnfDr3WL//CAyTOCeg2piaCfFdup7YFWMLKXP1jiNL98tT/dq2DKj AcwA== X-Gm-Message-State: AFqh2kp8l0CkEOT9WIONlfc1W/QH3kWq5ChJxgtMdAio1oNadGEsP7yY Th8ZBUD8MrbbgWZihwylAFA= X-Google-Smtp-Source: AMrXdXvXy9Yql0L4L5bxkSaBFLnkMhOFmZr+4ezWWzAh7Awicdws4yhqQzcS+3JMsfUpMzmtQUDk2w== X-Received: by 2002:a17:90a:4407:b0:223:f4e9:b22b with SMTP id s7-20020a17090a440700b00223f4e9b22bmr2465790pjg.41.1671574129971; Tue, 20 Dec 2022 14:08:49 -0800 (PST) Received: from [192.168.2.103] (61-220-36-70.hinet-ip.hinet.net. [61.220.36.70]) by smtp.gmail.com with ESMTPSA id 15-20020a17090a0d4f00b0021896fa945asm30734pju.15.2022.12.20.14.08.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 20 Dec 2022 14:08:49 -0800 (PST) Message-ID: <262532fd-3e7b-7936-070a-2b56ccd6495a@gmail.com> Date: Wed, 21 Dec 2022 06:08:42 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Subject: Re: [PING^5] [PATCH v3] [PR python/29603] Disable out-of-scope watchpoints Content-Language: en-US From: Johnson Sun To: LancelotSIX , gdb-patches@sourceware.org References: <20221020175707.193041-1-j3.soon777@gmail.com> <20221020181101.193226-1-j3.soon777@gmail.com> <4fc32997-d555-1aeb-1b57-fce152a33540@gmail.com> <23ce74fe-1d98-8b22-85f9-9b825c81b66d@gmail.com> <8cf0cf4f-9370-749c-13a3-665a5c703115@gmail.com> In-Reply-To: <8cf0cf4f-9370-749c-13a3-665a5c703115@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,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 List-Id: Ping for: . Johnson On 12/13/2022 5:44 AM, Johnson Sun wrote: > Ping for: > . > > Johnson > > On 12/5/2022 12:45 AM, Johnson Sun wrote: >> Ping for: >> . >> >> Johnson >> >> On 11/25/2022 11:11 PM, Johnson Sun wrote: >>> Ping for: >>> . >>> >>> Johnson >>> >>> On 11/18/2022 8:17 PM, JohnsonSun wrote: >>>> Ping for: >>>> . >>>> >>>> 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 >>>>> .  */ >>>>> + >>>>> +#include >>>>> + >>>>> +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 >>>>> . >>>>> + >>>>> +# 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 >>>>> . >>>>> + >>>>> + >>>>> +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") >>>>