From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6322 invoked by alias); 28 Apr 2016 10:10:36 -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 6189 invoked by uid 89); 28 Apr 2016 10:10:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:2514, super 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, 28 Apr 2016 10:10:24 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 70A9B7F09B; Thu, 28 Apr 2016 10:10:23 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3SAAMBJ003095; Thu, 28 Apr 2016 06:10:22 -0400 Subject: Re: [RFC] Remove need_step_over from struct lwp_info To: Yao Qi , gdb-patches@sourceware.org References: <1461657497-12076-1-git-send-email-yao.qi@linaro.org> From: Pedro Alves Message-ID: <89e60caa-7324-5bca-2cfa-ece25b46e7c3@redhat.com> Date: Thu, 28 Apr 2016 10:10:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <1461657497-12076-1-git-send-email-yao.qi@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-04/txt/msg00631.txt.bz2 On 04/26/2016 08:58 AM, Yao Qi wrote: > Hi, > I happen to see that field need_step_over in struct lwp_info is only > used to print a debug info. need_step_over is set in linux_wait_1 > when breakpoint_here is true, however, we check breakpoint_here too in > need_step_over_p and do the step over. I think we don't need field > need_step_over, and check breakpoint_here directly in need_step_over_p. > > This field was added in this patch > https://sourceware.org/ml/gdb-patches/2010-03/msg00605.html and the code > wasn't changed much since then. > > This patch is to remove it. > The intention was for this: if (!lwp->need_step_over) { if (debug_threads) debug_printf ("Need step over [LWP %ld]? No\n", lwpid_of (thread)); } to not just be a debug print, but also a return. Looks like that might have been only on an earlier prototype, or I messed it up and removed the return by accident before posting or some such. This is why we have comments like: if (bp_explains_trap) { /* If we stepped or ran into an internal breakpoint, we've already handled it. So next time we resume (from this PC), we should step over it. */ if (debug_threads) debug_printf ("Hit a gdbserver breakpoint.\n"); if (breakpoint_here (event_child->stop_pc)) event_child->need_step_over = 1; The idea was that if the thread happens to be stopped at a breakpoint, but the breakpoint wasn't actually hit yet, you'd want to let it be hit, rather than step over it. E.g., say you have 2 threads, and thread 1 stops for a breakpoint at PC1. Since we're in all-stop mode, gdbserver pauses thread 2 as well. Thread 2 pauses at PC2. Now the user sets a breakpoint at PC2. User continues. gdbserver at that time would step over the breakpoint at PC2, which is not what GDB expects. Likewise, if the user starts a trace session with a tracepoint at PC2, while thread 2 is stopped at PC2, gdbserver would skip the tracepoint rather than collect it immediately. However, since: [x86-linux Z0 support, and support multiple breakpoints at the same address] https://sourceware.org/ml/gdb-patches/2010-03/msg00917.html need_step_over_p checks gdb_breakpoint_here, so unless the target doesn't support Z0 breakpoints, the gdb breakpoint case ends up handled that way. And the tracepoint case is probably not a big deal, and we can live with it. In any case, looks like this never worked upstream, so I'm super fine with removing the field and cleaning this all up. Thanks, Pedro Alves