From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29850 invoked by alias); 17 Sep 2014 13:49:28 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 29841 invoked by uid 89); 17 Sep 2014 13:49:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 17 Sep 2014 13:49:26 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8HDnMcm022250 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 17 Sep 2014 09:49:23 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8HDnKK2027971; Wed, 17 Sep 2014 09:49:21 -0400 Message-ID: <54199160.5040500@redhat.com> Date: Wed, 17 Sep 2014 13:49:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.0 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH/DOC RFA] Fix "breakpoint always-inserted off"; remove "breakpoint always-inserted auto" References: <1410913576-4928-1-git-send-email-palves@redhat.com> <8738bq1jcz.fsf@codesourcery.com> In-Reply-To: <8738bq1jcz.fsf@codesourcery.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-09/txt/msg00586.txt.bz2 On 09/17/2014 02:12 PM, Yao Qi wrote: > Pedro Alves writes: > >> In hindsight, this "auto" setting was unnecessary, and not the ideal >> solution. Non-stop mode does depends on breakpoints always-inserted >> mode, but only as long as any thread is running. If no thread is >> running, no breakpoint can be missed. The same is true for all-stop >> too. E.g., if, in all-stop mode, and the user does: >> >> (gdb) c& >> (gdb) b foo >> >> That breakpoint at "foo" should be inserted immediately, but it >> currently isn't -- currently it'll end up inserted only if the target >> happens to trip on some event, and is re-resumed, e.g., an internal >> breakpoint triggers that doesn't cause a user-visible stop, and so we >> end up in keep_going calling insert_breakpoints. >> >> IOW, no matter whether in non-stop or all-stop, if the target fully >> stops, we can remove breakpoints. And no matter whether in all-stop >> or non-stop, if any thread is running in the target, then we need >> breakpoints to be immediately inserted. And then, if the target has > > Could we add some test cases for this? for example, breakpoint should be > inserted immediately if any thread is running. I'll try writing one. May need to skip all-stop/remote, as we can't talk to the target if anything is running (blocked waiting for the vCont reply). >> +/* update_global_location_list's modes of operation wrt to whether to >> + insert locations now. */ >> +enum ugll_insert_mode >> +{ >> + /* Don't insert any breakpoint locations into the inferior, only >> + remove already-inserted locations that no longer should be >> + inserted. Functions that delete a breakpoint or breakpoints >> + should specify this mode, so that deleting a breakpoint doesn't >> + have the side effect of inserting the locations of other >> + breakpoints that are marked not-inserted, but should_be_inserted >> + returns true on them. >> + >> + This behavior is useful is situations close to tear-down -- e.g., >> + after an exec, while the target still has execution, but >> + breakpoint shadows of the previous executable image should *NOT* >> + be restored to the new image; or before detaching, where the >> + target still has execution and wants to delete breakpoints from >> + GDB's lists, and all breakpoints had already been removed from >> + the inferior. */ >> + UGLL_DONT_INSERT, >> + >> + /* May insert breakpoints iff breakpoints_should_be_inserted_now >> + claims breakpoints should be inserted now. */ >> + UGLL_MAY_INSERT, >> + >> + /* Insert locations now, even if breakpoints_should_be_inserted_now >> + would return false. For example, no thread is resumed yet, but >> + we're just about to resume the target. */ >> + UGLL_INSERT, >> +}; >> + > > What does the last sentence in comments mean? "no thread is resumed > yet, but we're just about to resume the target." E.g., say everything all threads are stopped right now, and the user does "continue". We need to insert breakpoints _before_ resuming the target, but breakpoints_should_be_inserted_now returns false at that point, as no thread is running. See where it's used: @@ -2928,13 +2968,7 @@ insert_breakpoints (void) update_watchpoint (w, 0 /* don't reparse. */); } - update_global_location_list (1); - - /* update_global_location_list does not insert breakpoints when - always_inserted_mode is not enabled. Explicitly insert them - now. */ - if (!breakpoints_always_inserted_mode ()) - insert_breakpoint_locations (); + update_global_location_list (UGLL_INSERT); } I'll try to clarify the comment of UGLL_INSERT, and I'll preserve this comment in insert_breakpoints somehow instead of removing it completely. > The patch looks good to me. However, IWBN to split it to two, patch 1 > is about refactor update_global_location_list and add enum ugll_insert_mode > with only UGLL_DONT_INSERT and UGLL_MAY_INSERT. patch 2 does the rest. I'll do that. Thanks, Pedro Alves