From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27948 invoked by alias); 13 Apr 2015 10:47:29 -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 27933 invoked by uid 89); 13 Apr 2015 10:47:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD 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; Mon, 13 Apr 2015 10:47:27 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id C02EFB6A2A; Mon, 13 Apr 2015 10:47:26 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3DAlPh2000547; Mon, 13 Apr 2015 06:47:26 -0400 Message-ID: <552B9EBC.8000709@redhat.com> Date: Mon, 13 Apr 2015 10:47:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH v2 06/23] Make thread_still_needs_step_over consider stepping_over_watchpoint too References: <1428410990-28560-1-git-send-email-palves@redhat.com> <1428410990-28560-7-git-send-email-palves@redhat.com> <86vbh7vuja.fsf@gmail.com> In-Reply-To: <86vbh7vuja.fsf@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-04/txt/msg00453.txt.bz2 On 04/08/2015 10:28 AM, Yao Qi wrote: > Pedro Alves writes: > >> +static int >> +thread_still_needs_step_over (struct thread_info *tp) >> +{ >> + struct inferior *inf = find_inferior_ptid (tp->ptid); > > 'inf' isn't used. > Whoops. It used to in an older version. Dropped. >> @@ -6327,6 +6357,8 @@ keep_going (struct execution_control_state *ecs) >> else >> clear_step_over_info (); >> >> + ecs->event_thread->control.trap_expected = (remove_bp || remove_wps); >> + >> /* Stop stepping if inserting breakpoints fails. */ >> TRY >> { >> @@ -6341,8 +6373,6 @@ keep_going (struct execution_control_state *ecs) >> } >> END_CATCH >> >> - ecs->event_thread->control.trap_expected = (remove_bp || remove_wps); >> - > > Why do we hoist this line in front of the TRY/CATCH block? because we > want to set control.trap_expected before insert_breakpoints which may > throw exception? We need a ChangeLog entry for it. Short version: there's really no reason for that. I've undone that last week, and all the testing I've been doing so far never tripped on any problem. The longer version is that in a earlier version of this series (never posted anywhere), I had a version of keep_going and resume that only prepared the resume, but did not insert breakpoints or call 'target_resume', so that we could aggregate all the target_resume calls into a single call. That trap_expected set back then was still done by keep_going. That caused a lot of pain and so in the end, I undid all that. When I put back the insert_breakpoints etc code I probably put it after the trap_expected line instead of before, probably just because it's easier to read that way, as there's a connection between that trap_expected set and the code that comes before the breakpoints insertion. And then after staring at these patches for so long, my brain just learned to ignore this diff's hunk as not important. :-) Thanks! -- Pedro Alves