From patchwork Wed Apr 22 20:05:05 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 6391 Received: (qmail 112739 invoked by alias); 22 Apr 2015 20:05:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 112723 invoked by uid 89); 22 Apr 2015 20:05:09 -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; Wed, 22 Apr 2015 20:05:07 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id B9FF62CD7F0; Wed, 22 Apr 2015 20:05:06 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3MK55bH021611; Wed, 22 Apr 2015 16:05:05 -0400 Message-ID: <5537FEF1.7000100@redhat.com> Date: Wed, 22 Apr 2015 21:05:05 +0100 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: Doug Evans CC: gdb-patches@sourceware.org Subject: Re: [PATCH v3 07/17] Misc switch_back_to_stepped_thread cleanups References: <1429267521-21047-1-git-send-email-palves@redhat.com> <1429267521-21047-8-git-send-email-palves@redhat.com> <21815.12255.713123.668054@ruffy2.mtv.corp.google.com> In-Reply-To: <21815.12255.713123.668054@ruffy2.mtv.corp.google.com> On 04/22/2015 06:21 AM, Doug Evans wrote: > Pedro Alves writes: > > [...] it isn't ever correct to pass step=1 to target_resume > > on software single-step targets [...] > > Sounds like a good thing to document in a comment or assert. Yeah, we were discussing adding an assertion on the gdbserver side here: https://sourceware.org/ml/gdb-patches/2015-04/msg00232.html An assertion on the gdb side is complicated, and I'd rather that be done separately. gdbarch_software_single_step_p() returning true does not mean that we can't use hardware step. E.g., ppc installs that hook for stepping past atomic regions. We can't assert based on software single-step breakpoints inserted, as infrun.c uses those even on targets that don't implement gdbarch_software_single_step_p. We could add a new target_can_hardware_single_step method, which ideally for remote targets would be based on the reply to the "vCont?" probe packet, but gdbserver always includes "s" actions in the reply even on targets that can't hardware step, and we can't just make it not do that, since remote.c:remote_vcont_probe has: /* If s, S, c, and C are not all supported, we can't use vCont. Clearing BUF will make packet_ok disable the packet. */ if (!support_s || !support_S || !support_c || !support_C) buf[0] = 0; so even if we fixed mainline, newer gdbserver against older gdb would be broken... So we need something else or in addition (probably based on qSupported). For comments, how about this? Thanks, Pedro Alves diff --git i/gdb/infrun.c w/gdb/infrun.c index 0bf1274..fbe12ab 100644 --- i/gdb/infrun.c +++ w/gdb/infrun.c @@ -5839,7 +5839,9 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) return 0; } -/* Is thread TP in the middle of single-stepping? */ +/* Is thread TP in the middle of (software or hardware) + single-stepping? (Note the result of this function must never be + passed directly as target_resume's STEP parameter.) */ static int currently_stepping (struct thread_info *tp) diff --git i/gdb/target.h w/gdb/target.h index 66bf91e..283f56f 100644 --- i/gdb/target.h +++ w/gdb/target.h @@ -1254,8 +1254,8 @@ extern void target_detach (const char *, int); extern void target_disconnect (const char *, int); /* Resume execution of the target process PTID (or a group of - threads). STEP says whether to single-step or to run free; SIGGNAL - is the signal to be given to the target, or GDB_SIGNAL_0 for no + threads). STEP says whether to hardware single-step or to run free; + SIGGNAL is the signal to be given to the target, or GDB_SIGNAL_0 for no signal. The caller may not pass GDB_SIGNAL_DEFAULT. A specific PTID means `step/resume only this process id'. A wildcard PTID (all threads, or all threads of process) means `step/resume