From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22565 invoked by alias); 2 Oct 2014 18:05:50 -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 22555 invoked by uid 89); 2 Oct 2014 18:05:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 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; Thu, 02 Oct 2014 18:05:49 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s92I5khC028041 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 2 Oct 2014 14:05:46 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s92I5iVv020393; Thu, 2 Oct 2014 14:05:45 -0400 Message-ID: <542D93F8.7050005@redhat.com> Date: Thu, 02 Oct 2014 18:05:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH 1/9] Decide whether we may have removed breakpoints based on step_over_info References: <1411691982-10744-1-git-send-email-palves@redhat.com> <1411691982-10744-2-git-send-email-palves@redhat.com> <87fvfbx65x.fsf@codesourcery.com> In-Reply-To: <87fvfbx65x.fsf@codesourcery.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-10/txt/msg00040.txt.bz2 On 09/28/2014 01:48 PM, Yao Qi wrote: > Pedro Alves writes: > >> gdb/ >> 2014-09-22 Pedro Alves >> >> * infrun.c (step_over_info_valid_p): New function. >> (resume): Use step_over_info_valid_p instead of checking the >> threads's trap_expected flag. Add debug output. > ^^^^^^^^^^^^^^^^ > I don't see any debug output added by the code. Maybe a staled changelog entry? Yeah, I ended up removing that code. Will fix, thanks. > >> +/* Returns true if step-over info is valid. */ >> + >> +static int >> +step_over_info_valid_p (void) >> +{ >> + return (step_over_info.aspace != NULL); >> +} >> + > > How about replace "step_over_info.aspace != NULL" in > stepping_past_instruction_at with step_over_info_valid_p too? See patch 2/9. With that, step_over_info_valid_p will return true if we're stepping over a watchpoint, but aspace will be NULL. > >> /* Advise target which signals may be handled silently. If we have >> - removed breakpoints because we are stepping over one (which can >> - happen only if we are not using displaced stepping), we need to >> + removed breakpoints because we are stepping over one, we need to >> receive all signals to avoid accidentally skipping a breakpoint >> during execution of a signal handler. */ >> - if ((step || singlestep_breakpoints_inserted_p) >> - && tp->control.trap_expected >> - && !use_displaced_stepping (gdbarch)) >> + if (step_over_info_valid_p ()) > > Why do we remove condition (step || singlestep_breakpoints_inserted_p)? > I understand that step_over_info_valid_p is equivalent to > "tp->control.trap_expected && !use_displaced_stepping (gdbarch)", so I > don't know why (step || singlestep_breakpoints_inserted_p) is removed > too. It ends up unnecessary, and it seems to me to be more to the point. I.e., if we have step-over info, then something, somewhere wants a breakpoint lifted out of the target. No matter whether we're stepping or continuing the target at this point, we need to receive all signals so that if the signal handler calls the code that would trigger the breakpoint/watchpoint, we don't miss it. Removing this check now avoids having tweak it when singlestep_breakpoints_inserted_p check global ends up eliminated by a later patch in the series. Does that make sense? > >> target_pass_signals (0, NULL); >> else >> target_pass_signals ((int) GDB_SIGNAL_LAST, signal_pass); Thanks, Pedro Alves