From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 35226 invoked by alias); 21 Jul 2016 08:38:38 -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 35207 invoked by uid 89); 21 Jul 2016 08:38:37 -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,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=GDBserver X-HELO: mail-oi0-f68.google.com Received: from mail-oi0-f68.google.com (HELO mail-oi0-f68.google.com) (209.85.218.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 21 Jul 2016 08:38:27 +0000 Received: by mail-oi0-f68.google.com with SMTP id c199so7106596oig.1 for ; Thu, 21 Jul 2016 01:38:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=/ff8LERuzr+jWM3xfSRCloaED8VG20jQGQjoMPOIEwk=; b=M/Hq72i5yikMAO/6CwVLszCoyWyG/LLTkxvTPbW5RVdxadWYGx3WJSTBBOGCGlddoM vbfwxGn4o6IoLAqEDG4U6epwIeQMwn7dBvo0x6L5HoVPH1fxgWkWDbbcn0D+XPPSMuBr EZxICtX5rtjoux/37F2Js5Q11Go3oBz6KIEzxd+NgrgEadd58yOt55GRJjDuDNtBpavG oHd1osrEU3UYGc8J69o8JjV3EVCgMKm/VXl4j90It9YqMURuHPsrkga2Zs0sU/0g3HDc l2pB9xVqVlJdlt5ooz1w4wLOJZuJm/ig0puj3SWljRpOwiIxGvGp+3agzEElkliiwZUX xiDQ== X-Gm-Message-State: ALyK8tKlAqjnDXJG0G3I2Zgg9BFRuE9AhW6kGhvgSmGFdMg0n1JKcJn5g7gZ0T1MRQ58UJ9ZHiv/xJJ3YyBEOQ== X-Received: by 10.202.75.142 with SMTP id y136mr24308827oia.103.1469090305262; Thu, 21 Jul 2016 01:38:25 -0700 (PDT) MIME-Version: 1.0 Received: by 10.202.105.138 with HTTP; Thu, 21 Jul 2016 01:38:24 -0700 (PDT) In-Reply-To: <861t38zekw.fsf@gmail.com> References: <1467295765-3457-1-git-send-email-yao.qi@linaro.org> <1467295765-3457-9-git-send-email-yao.qi@linaro.org> <861t38zekw.fsf@gmail.com> From: Yao Qi Date: Thu, 21 Jul 2016 08:38:00 -0000 Message-ID: Subject: Re: [PATCH 8/9] Use reinsert_breakpoint for vCont;s To: Pedro Alves Cc: Yao Qi , "gdb-patches@sourceware.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-07/txt/msg00237.txt.bz2 Ping? I want this series go in before the release. On Tue, Jul 5, 2016 at 9:14 AM, Yao Qi wrote: > Pedro Alves writes: > >>> if thread 1 doesn't hit the reinsert breakpoint, we don't have to >>> remove them, because GDB will send vCont;s:1 next time, and GDBserver >> >> There's no guarantee GDB will send vCont;s:1 next time. >> The user may do "continue" instead of "step". >> >>> can only install reinsert breakpoints if they are not installed yet. >> >> The user may even do "return + continue" or "jump", or an infcall, >> all of which resume the thread at a different address from the address >> the thread last stopped. So there's no guarantee that the >> reinsert breakpoint address makes any sense for the next step request, >> or even that the next resume request is a step in the first place. >> >> Basically the previous step request must be completely forgotten after >> gdb has seen the thread stop. In all-stop, gdb "sees "all threads >> stopped on each and every event reported to gdb, for any thread. >> A stop reply cancels any and all previous resume requests. > > I add some code to delete all reinsert breakpoints for all threads in > all-stop. See the patch below, > >> >>> if thread hits the reinsert breakpoint, but the event is not reported. >>> It becomes pending, and GDBserver will delete the reinsert breakpoints >>> next time when this pending event is reported back to GDB. >> >> I don't follow. I'm talking about the case where the thread does _not_ >> hit the reinsert breakpoint. Instead some other thread hits some unrela= ted >> event. > > In your review to V2, your words are about other threads hit a breakpoint= , but > doesn't mention "thread 1" (requested for resume_step) hits reinsert > breakpoint or not. I just list two possible results (not hit vs > hit). You didn't talk about the latter, so we don't have to follow it. > > -- > Yao (=E9=BD=90=E5=B0=A7) > > From f4aa0593190e7e78831532c03177d8264b7dd1c1 Mon Sep 17 00:00:00 2001 > From: Yao Qi > Date: Thu, 19 May 2016 17:07:49 +0100 > Subject: [PATCH] Use reinsert_breakpoint for vCont;s > > V4: remove all reinsert breakpoints before sending event back to GDB > in all-stop mode, > > V3: install breakpoints in proceed_one_lwp, > no longer stop all threads when installing breakpoints, > delete reinsert breakpoints when GDBserver wants to report event, > > V2: fix spaces in changelog entry, > use maybe_hw_step, > cancel step-over if signal arrives (!maybe_internal_trap), > > This patch is to teach GDBserver using software single step to handle > vCont;s. Simply speaking, if the thread's resume request is resume_step, > install reinsert breakpoint at the next pcs when GDBserver is about to > resume threads. These reinsert breakpoints of a thread are removed, > when GDBserver gets an event from that thread and reports it back to > GDB. > > gdb/gdbserver: > > 2016-07-05 Yao Qi > > * linux-low.c (resume_stopped_resumed_lwps): If resume request > is resume_step, call maybe_hw_step. > (linux_wait_1): Stop all threads, remove reinsert breakpoints, > and unstop them. > (linux_resume_one_lwp_throw): Don't assert the thread has reinsert > breakpoints or not. > (proceed_one_lwp): If resume request is resume_step, install > reinsert breakpoints and call maybe_hw_step. > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index abaf288..b579b4d 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -2563,7 +2563,10 @@ resume_stopped_resumed_lwps (struct inferior_list_= entry *entry) > && !lp->status_pending_p > && thread->last_status.kind =3D=3D TARGET_WAITKIND_IGNORE) > { > - int step =3D thread->last_resume_kind =3D=3D resume_step; > + int step =3D 0; > + > + if (thread->last_resume_kind =3D=3D resume_step) > + step =3D maybe_hw_step (thread); > > if (debug_threads) > debug_printf ("RSRL: resuming stopped-resumed LWP %s at %s: step= =3D%d\n", > @@ -3622,6 +3625,66 @@ linux_wait_1 (ptid_t ptid, > > /* Alright, we're going to report a stop. */ > > + /* Remove reinsert breakpoints. */ > + if (can_software_single_step ()) > + { > + /* Remove reinsert breakpoints or not. It it is true, stop all > + lwps, so that other threads won't hit the breakpoint in the > + staled memory. */ > + int remove_reinsert_breakpoints_p =3D 0; > + > + if (non_stop) > + { > + remove_reinsert_breakpoints_p > + =3D has_reinsert_breakpoints (current_thread); > + } > + else > + { > + /* In all-stop, a stop reply cancels all previous resume > + requests. Delete all reinsert breakpoints. */ > + struct inferior_list_entry *inf, *tmp; > + > + ALL_INFERIORS (&all_threads, inf, tmp) > + { > + struct thread_info *thread =3D (struct thread_info *) inf; > + > + if (has_reinsert_breakpoints (thread)) > + { > + remove_reinsert_breakpoints_p =3D 1; > + break; > + } > + } > + } > + > + if (remove_reinsert_breakpoints_p) > + { > + /* If we remove reinsert breakpoints from memory, stop all lwps, > + so that other threads won't hit the breakpoint in the staled > + memory. */ > + stop_all_lwps (0, event_child); > + > + if (non_stop) > + { > + gdb_assert (has_reinsert_breakpoints (current_thread)); > + delete_reinsert_breakpoints (current_thread); > + } > + else > + { > + struct inferior_list_entry *inf, *tmp; > + > + ALL_INFERIORS (&all_threads, inf, tmp) > + { > + struct thread_info *thread =3D (struct thread_info *) i= nf; > + > + if (has_reinsert_breakpoints (thread)) > + delete_reinsert_breakpoints (thread); > + } > + } > + > + unstop_all_lwps (0, event_child); > + } > + } > + > if (!stabilizing_threads) > { > /* In all-stop, stop all threads. */ > @@ -4275,12 +4338,6 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, > > step =3D maybe_hw_step (thread); > } > - else > - { > - /* If the thread isn't doing step-over, there shouldn't be any > - reinsert breakpoints. */ > - gdb_assert (!has_reinsert_breakpoints (thread)); > - } > > if (fast_tp_collecting =3D=3D 1) > { > @@ -5088,7 +5145,14 @@ proceed_one_lwp (struct inferior_list_entry *entry= , void *except) > if (debug_threads) > debug_printf (" stepping LWP %ld, client wants it stepping\n", > lwpid_of (thread)); > - step =3D 1; > + > + /* If resume_step is requested by GDB, install reinsert > + breakpoints when the thread is about to be actually resumed if > + the reinsert breakpoints weren't removed. */ > + if (can_software_single_step () && !has_reinsert_breakpoints (thre= ad)) > + install_software_single_step_breakpoints (lwp); > + > + step =3D maybe_hw_step (thread); > } > else if (lwp->bp_reinsert !=3D 0) > { --=20 Yao (=E9=BD=90=E5=B0=A7)