From patchwork Tue Jul 5 08:14:39 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 13631 Received: (qmail 127314 invoked by alias); 5 Jul 2016 08:15:04 -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 127257 invoked by uid 89); 5 Jul 2016 08:15:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=lp, sk:remove_, HX-Received:Tue, arrives X-HELO: mail-pf0-f195.google.com Received: from mail-pf0-f195.google.com (HELO mail-pf0-f195.google.com) (209.85.192.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 05 Jul 2016 08:14:52 +0000 Received: by mail-pf0-f195.google.com with SMTP id i123so18415154pfg.3 for ; Tue, 05 Jul 2016 01:14:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=qQjSJRNhje64S4ehMkcaH2gBb5O4OHwwoRY7xXpZy9A=; b=nE8cKsFkXKVfFcbDaEYeKWZ+c0Kxvk3wB6eErzqVDRPcmxtGvYZ4olRBMA7wzRRycN S4jvVnO1+Io81siLlXioeJunX7ODXN5Ifbl/8jfwfsKYmE2x11ivPCjvlXrZJxHUvxf9 ATuYwbWU1B9urgGpygcwOCqEYcoC/6Tn9wl2t7UsrqrHF9hnQII9hJ3j3rSYKDXIH5yF HMLpIACQ86XJRtcPCwaneBjOzWaxUOysmZbLtyknm2KglePtxTw95zLMAWoM+KPTp7Vy ZJcRKXcp4zZo0VyyMsj6557g1GTQy+tgfUDdYpp6RFxw09UrX4SzZBoa3swf01B746am v11Q== X-Gm-Message-State: ALyK8tJArpoRakujf5bRJDdGsc/R11ukRBFNso4D5LeE1jRD/8yAopIjpeM0pqmIw/fJuA== X-Received: by 10.98.18.131 with SMTP id 3mr29972508pfs.102.1467706490258; Tue, 05 Jul 2016 01:14:50 -0700 (PDT) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id i3sm2741178pfk.30.2016.07.05.01.14.46 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Tue, 05 Jul 2016 01:14:49 -0700 (PDT) From: Yao Qi To: Pedro Alves Cc: Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH 8/9] Use reinsert_breakpoint for vCont;s References: <1467295765-3457-1-git-send-email-yao.qi@linaro.org> <1467295765-3457-9-git-send-email-yao.qi@linaro.org> Date: Tue, 05 Jul 2016 09:14:39 +0100 In-Reply-To: (Pedro Alves's message of "Fri, 1 Jul 2016 16:07:26 +0100") Message-ID: <861t38zekw.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes 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 unrelated > 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. 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 == TARGET_WAITKIND_IGNORE) { - int step = thread->last_resume_kind == resume_step; + int step = 0; + + if (thread->last_resume_kind == resume_step) + step = maybe_hw_step (thread); if (debug_threads) debug_printf ("RSRL: resuming stopped-resumed LWP %s at %s: step=%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 = 0; + + if (non_stop) + { + remove_reinsert_breakpoints_p + = 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 = (struct thread_info *) inf; + + if (has_reinsert_breakpoints (thread)) + { + remove_reinsert_breakpoints_p = 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 = (struct thread_info *) inf; + + 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 = 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 == 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 = 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 (thread)) + install_software_single_step_breakpoints (lwp); + + step = maybe_hw_step (thread); } else if (lwp->bp_reinsert != 0) {