From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27702 invoked by alias); 14 May 2013 18:30:41 -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 27691 invoked by uid 89); 14 May 2013 18:30:40 -0000 X-Spam-SWARE-Status: No, score=-8.0 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 14 May 2013 18:30:40 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4EIUbZE031642 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 14 May 2013 14:30:37 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r4EIUaXw031073; Tue, 14 May 2013 14:30:36 -0400 Message-ID: <519282CB.4030800@redhat.com> Date: Tue, 14 May 2013 18:30:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH 3/7] range stepping: gdbserver on x86/linux References: <1363006291-13334-1-git-send-email-yao@codesourcery.com> <1363006291-13334-4-git-send-email-yao@codesourcery.com> In-Reply-To: <1363006291-13334-4-git-send-email-yao@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg00478.txt.bz2 On 03/11/2013 12:51 PM, Yao Qi wrote: > diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c > index 922a5bf..0b330d1 100644 > --- a/gdb/gdbserver/server.c > +++ b/gdb/gdbserver/server.c > @@ -1871,10 +1871,15 @@ handle_v_cont (char *own_buf) > p = &own_buf[5]; > while (*p) > { > + CORE_ADDR start = 0; > + CORE_ADDR end = 0; > + > p++; > > if (p[0] == 's' || p[0] == 'S') > resume_info[i].kind = resume_step; > + else if (p[0] == 'r') > + resume_info[i].kind = resume_step; > else if (p[0] == 'c' || p[0] == 'C') > resume_info[i].kind = resume_continue; > else if (p[0] == 't') > @@ -1894,6 +1899,21 @@ handle_v_cont (char *own_buf) > goto err; > resume_info[i].sig = gdb_signal_to_host (sig); > } > + else if (p[0] == 'r') > + { > + char *p1; > + > + p = p + 1; > + p1 = strchr (p, ','); > + decode_address (&start, p, p1 - p); > + > + p = p1 + 1; > + p1 = strchr (p, ':'); > + decode_address (&end, p, p1 - p); > + > + resume_info[i].sig = 0; > + p = p1; > + } > else > { > resume_info[i].sig = 0; > @@ -1919,6 +1939,21 @@ handle_v_cont (char *own_buf) > goto err; > > resume_info[i].thread = ptid; > + if (end > 0) > + { > + struct thread_info *tp = find_thread_ptid (ptid); > + > + /* GDB should not send range stepping for all threads of > + a process, like 'vCont;rSTART,END:pPID.-1', TP can't > + be NULL. */ > + gdb_assert (tp != NULL); > + > + tp->start = start; > + tp->end = end; > + > + start = 0; > + end = 0; > + } My main issues with this are: - it assumes GDB will ever only send one r action per vCont. I'd much rather we don't bake in that assumption. - GDBserver should not gdb_assert for unexpected input it can't control. - It seems like this leaves threads with stale step ranges. Because linux-low.c only clears the stepping range of the thread that is reporting the event (here): > @@ -2685,6 +2705,8 @@ Check if we're already there.\n", > fprintf (stderr, "Hit a non-gdbserver trap event.\n"); > } > > + thread_clear_range_stepping (current_inferior); > + > /* Alright, we're going to report a stop. */ > > if (!non_stop && !stabilizing_threads) > @@ -5081,6 +5103,15 @@ linux_supports_agent (void) > return 1; > } That is, e.g.,: - user "step"s thread 1 (vCont;r) - thread 2 hits breakpoint (step range of thread 2 is cleared) - user goes back to thread 1, and does "si". - GDB sends vCont;s, but thread 1 still has the stale step range from the previous vCont;r. We should probably have an explicit test for this, though I haven't added one myself (at least yet). -- Pedro Alves