From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 4070F38582A7 for ; Wed, 21 Sep 2022 14:03:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4070F38582A7 Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-173-AZmeNsAwMKOc8azTDJvhhQ-1; Wed, 21 Sep 2022 10:03:00 -0400 X-MC-Unique: AZmeNsAwMKOc8azTDJvhhQ-1 Received: by mail-ej1-f71.google.com with SMTP id sg44-20020a170907a42c00b007822e506a4fso441690ejc.16 for ; Wed, 21 Sep 2022 07:03:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date; bh=JPCXaRDtSp9+8+6Fb9JM0qKq+aLp7sJYLQmOEhSaKYM=; b=fl+2dbaZrjQYBEE9sCESA8YY3pGjSdikoeo3QjAZt/9VRfq9GtyOnA8gug8UDe6PdC 8nhcaJzLtBTaEgFlOCdVcHb859jxdDUQ9GRaYKCKlo39gCpXCQ/1I3Cl+CGNie7/eiJH vXBex3oZw42g8L11KcK8HpednVa3xhRqvmq+jS7fS31douoSnG5QfCvoDAKlaZZsZJEK eLjfEs522IkOMAle7wmVMzHrK4j6UmE2Cbfm5iOakOXL0foxby/DCSoTO6rF024WgRHQ yOFc4wIDB1BZE/hrc2gyI7cfvSyiHYRsKg7juWXguFTkN0zNaQ21kP1fSPd+vXQUyTHh sssA== X-Gm-Message-State: ACrzQf2oUuM8h2J5nqKSjWUOj2uWI3d0Q1p6qHebn9QF7Nf0iq4q4u1I o1ObtXjhSFWSdYeepUoKG6SGBXB9d9bsdjzFuZIsCMrD2Lo2YayeVWQYh9H21siQor/dq3BQPP8 o/4JCXA3kjzD3waZlcePM0Q== X-Received: by 2002:a17:906:eec9:b0:73d:c369:690f with SMTP id wu9-20020a170906eec900b0073dc369690fmr21380354ejb.767.1663768979667; Wed, 21 Sep 2022 07:02:59 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6+uiKF4aetaa83n3097FiPfLke284YD08bJvnamJwoaqImvXZpS4TijgYyrBm8dMOSTDJi3w== X-Received: by 2002:a17:906:eec9:b0:73d:c369:690f with SMTP id wu9-20020a170906eec900b0073dc369690fmr21380330ejb.767.1663768979403; Wed, 21 Sep 2022 07:02:59 -0700 (PDT) Received: from [192.168.0.45] (ip-213-220-232-121.bb.vodafone.cz. [213.220.232.121]) by smtp.gmail.com with ESMTPSA id b21-20020a17090630d500b0071cef6c53aesm1379725ejb.0.2022.09.21.07.02.58 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 21 Sep 2022 07:02:58 -0700 (PDT) Message-ID: <516bbe6d-6a11-9906-7688-9693040406d4@redhat.com> Date: Wed, 21 Sep 2022 16:02:57 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.1 Subject: Re: [PATCH 2/3] [gdb/python] Fix gdb.python/py-finish-breakpoint-deletion.exp for Bug 18655 To: Johnson Sun , gdb-patches@sourceware.org References: <20220920172426.90659-1-j3.soon777@gmail.com> <20220920172426.90659-3-j3.soon777@gmail.com> From: Bruno Larsen In-Reply-To: <20220920172426.90659-3-j3.soon777@gmail.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-14.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_NUMSUBJECT, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Sep 2022 14:03:08 -0000 Hi Johnson, Thanks for working on this! The code looks fine, but the description can be improved a bit. We try to make commit titles as descriptive as possible on their own. I think describing this as "gdb/python: Fix deletion of FinishBreakpoints" would be easier when referencing this in the future. On 20/09/2022 19:24, Johnson Sun via Gdb-patches wrote: > This removes the KFAIL in the testcase, and fixes the bug by setting the > correct disposition type for deletion. > > The original code fails since it does not correctly delete the temporary > FinishBreakpoints in `gdb/python/py-finishbreakpoint.c': > > /* Can't delete it here, but it will be removed at the next stop. */ > disable_breakpoint (bp_obj->bp); > gdb_assert (bp_obj->bp->disposition == disp_del); > > The code above aims to delete the breakpoint on the next hit by setting the > disposition to `disp_del'. However, the FinishBreakpoint is disabled, so it > will never be hit (thus never be deleted before the program stops). > > The fix only consists of a single line, setting the disposition to > `disp_del_at_next_stop': > > bp_obj->bp->disposition = disp_del_at_next_stop; This may be personal preference, but I would usually describe the fix in less computer-y terms, like: "This commit fixes this by setting the disposition to disp_del_at_next_stop instead, which gets cleaned up in breakpoint_auto_delete". > > The code above allows the breakpoint to be deleted in `breakpoint_auto_delete': > > for (breakpoint *b : all_breakpoints_safe ()) > if (b->disposition == disp_del_at_next_stop) > delete_breakpoint (b); > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=18655 Also, we tend to refer to bugs as PR /, so this would be PR python/18655. This way bugzilla can automatically link things for us. Cheers, Bruno > --- > gdb/python/py-finishbreakpoint.c | 1 + > gdb/testsuite/gdb.python/py-finish-breakpoint-deletion.exp | 1 - > 2 files changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c > index c80096f6810..a219bc82f15 100644 > --- a/gdb/python/py-finishbreakpoint.c > +++ b/gdb/python/py-finishbreakpoint.c > @@ -146,6 +146,7 @@ bpfinishpy_post_stop_hook (struct gdbpy_breakpoint_object *bp_obj) > /* Can't delete it here, but it will be removed at the next stop. */ > disable_breakpoint (bp_obj->bp); > gdb_assert (bp_obj->bp->disposition == disp_del); > + bp_obj->bp->disposition = disp_del_at_next_stop; > } > catch (const gdb_exception &except) > { > diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint-deletion.exp b/gdb/testsuite/gdb.python/py-finish-breakpoint-deletion.exp > index 778b53fbeda..ac5e5d8ac2e 100644 > --- a/gdb/testsuite/gdb.python/py-finish-breakpoint-deletion.exp > +++ b/gdb/testsuite/gdb.python/py-finish-breakpoint-deletion.exp > @@ -43,5 +43,4 @@ gdb_test "source $pyfile" ".*Python script imported.*" \ > "import python scripts" > gdb_test "python print (len(gdb.breakpoints()))" "2" "check modified BP count" > gdb_test "continue" "Breakpoint.*at.*" "run until FinishBreakpoint stops" > -setup_kfail "gdb/18655" "*-*-*" > gdb_test "python print (len(gdb.breakpoints()))" "2" "check BP count"