From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3285 invoked by alias); 22 Apr 2015 05:09:08 -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 3140 invoked by uid 89); 22 Apr 2015 05:08:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 X-HELO: mail-qc0-f202.google.com Received: from mail-qc0-f202.google.com (HELO mail-qc0-f202.google.com) (209.85.216.202) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 22 Apr 2015 05:08:39 +0000 Received: by qcrw7 with SMTP id w7so4874227qcr.1 for ; Tue, 21 Apr 2015 22:08:37 -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:mime-version:content-type :content-transfer-encoding:message-id:date:to:cc:subject:in-reply-to :references; bh=IMpq9Ihf82x4tiB7rn/XAt6SQDaS0qWdgfvMteSTeSE=; b=KFa/yLlRqESynnd1W+HYF41EY/g4w+Oi333B40fKnW/882jL591B+v7IlBvaxu5yyg utYCqBK+YxReihXGH2FM9ZxNZpgoeUIa/I8tnBOwAP/VUU3sdClU8mDgPiwWcwyiveB7 lMo0zwyDBcBbbcJHlUP7RHVOWtjmh3yKhxk07Vw7R/wb6w0nS2+Eod3B5EBDjDGJW8Pd RlBS9VtWXzKyTh5EatQyKnAccd0+PgCb3oClLc2uume8rhlzBkqPnPz51N/mNrMnhesE 2SrtipIGvcPvd7FLGFTHxAtCTLg8sN13N9cS9wz5qlG28TN5fvs1AGGnthYvaT5hjK4v 2Ddw== X-Gm-Message-State: ALoCoQlKk4lHJ81JjTWmQbQJbac3/PzsAEElwC4ulL8vtb0qr8vq/iCYwZk77vinigGsVr/0HPb1eezcWDT/DhfWCqp+DFuiXvTCRjFJxJDsiIgtcBudPl2g9ddfSBVO7WaKeePJQOJU8E06YpxrOapFfkRukmREmK/6iwjeTW1atF77cEe0MOU= X-Received: by 10.236.36.37 with SMTP id v25mr34489065yha.23.1429679317444; Tue, 21 Apr 2015 22:08:37 -0700 (PDT) Received: from corpmail-nozzle1-1.hot.corp.google.com ([100.108.1.104]) by gmr-mx.google.com with ESMTPS id l36si356643yhb.1.2015.04.21.22.08.36 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 21 Apr 2015 22:08:37 -0700 (PDT) Received: from ruffy2.mtv.corp.google.com ([172.17.128.107]) by corpmail-nozzle1-1.hot.corp.google.com with ESMTP id FSAvYkvg.1; Tue, 21 Apr 2015 22:08:37 -0700 From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <21815.11476.327167.153546@ruffy2.mtv.corp.google.com> Date: Wed, 22 Apr 2015 05:09:00 -0000 To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3 06/17] Use keep_going in proceed and start_step_over too In-Reply-To: <1429267521-21047-7-git-send-email-palves@redhat.com> References: <1429267521-21047-1-git-send-email-palves@redhat.com> <1429267521-21047-7-git-send-email-palves@redhat.com> X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg00811.txt.bz2 Pedro Alves writes: > The main motivation of this patch is sharing more code between the > proceed (starting the inferior for the first time) and keep_going > (restarting the inferior after handling an event) paths and using the > step_over_chain queue now embedded in the thread_info object for > pending in-line step-overs too (instead of just for displaced > stepping). Hi. A couple of nits inline. grep for ==== > > So this commit: > > - splits out a new keep_going_pass function out of keep_going that is > just like keep_going except for the bits that clear the signal to > pass if the signal is set to "handle nopass". > > - makes proceed use keep_going too. > > - Makes start_step_over use keep_going_pass instead of lower level > displaced stepping things. > > One user visible change: if inserting breakpoints while trying to > proceed fails, we now get: > > (gdb) si > Warning: > Could not insert hardware watchpoint 7. > Could not insert hardware breakpoints: > You may have requested too many hardware breakpoints/watchpoints. > > Command aborted. > (gdb) > > while before we only saw warnings with no indication that the command > was cancelled: > > (gdb) si > Warning: > Could not insert hardware watchpoint 7. > Could not insert hardware breakpoints: > You may have requested too many hardware breakpoints/watchpoints. > > (gdb) > > Tested on x86_64-linux-gnu, ppc64-linux-gnu and s390-linux-gnu. > > gdb/ChangeLog: > 2015-04-17 Pedro Alves > > * gdbthread.h (struct thread_info) : Extend comment. > * infrun.c (struct execution_control_state): Move higher up in the > file. > (reset_ecs): New function. > (start_step_over): Now returns int. Rewrite to use > keep_going_pass instead of manually starting a displaced step. > (resume): Don't call set_running here. If displaced stepping > can't start now, clear trap_expected. > (find_thread_needs_step_over): Delete function. > (proceed): Set up finish_thread_state_cleanup. Call set_running. > If the current thread needs a step over, push it in the step-over > chain. Don't set insert breakpoints nor call resume directly > here. Instead rewrite to use start_step_over and keep_going_pass. > (finish_step_over): New function. > (handle_signal_stop): Call finish_step_over instead of > start_step_over. > (switch_back_to_stepped_thread): If the event thread needs another > step-over do that first. Use start_step_over. > (keep_going_pass): New function, factored out from ... > (keep_going): ... here. > (_initialize_infrun): Comment moved here. > * thread.c (set_running_thread): New function. > (set_running, finish_thread_state): Use set_running_thread. > > v3: > > - Adjusted to step_over_chain changes. Fixed gdb internal error > caught by signal-sigtrap.exp on ARM (software single-step target). > > v2: > > - Testing on x86_64 with software single-step revealed v1 missed > setting prev_pc on proceed, for switch_back_to_stepped_thread. > Rewrite tail end of proceed to fix that. This ends up simplifying > later patches in the series. >... > > diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h > index 2c871a2..7052ee1 100644 > --- a/gdb/gdbthread.h > +++ b/gdb/gdbthread.h > @@ -208,8 +208,10 @@ struct thread_info > > /* Internal stepping state. */ > > - /* Record the pc of the thread the last time it stopped. This is > - maintained by proceed and keep_going, and used in > + /* Record the pc of the thread the last time it was resumed. (It > + can't be done on stop as the PC may change since the last stop, > + e.g., "return" command, or "p $pc = 0xf000"). This is maintained > + by proceed and keep_going, and among other things, it's used in > adjust_pc_after_break to distinguish a hardware single-step > SIGTRAP from a breakpoint SIGTRAP. */ > CORE_ADDR prev_pc; > diff --git a/gdb/infrun.c b/gdb/infrun.c > index f325a53..beb9ea2 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -1844,30 +1844,61 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal) > displaced->step_ptid = null_ptid; > } > > -/* Are there any pending step-over requests? If so, run one now. */ > +/* Data to be passed around while handling an event. This data is > + discarded between events. */ > +struct execution_control_state > +{ > + ptid_t ptid; > + /* The thread that got the event, if this was a thread event; NULL > + otherwise. */ > + struct thread_info *event_thread; > + > + struct target_waitstatus ws; > + int stop_func_filled_in; > + CORE_ADDR stop_func_start; > + CORE_ADDR stop_func_end; > + const char *stop_func_name; > + int wait_some_more; > + > + /* True if the event thread hit the single-step breakpoint of > + another thread. Thus the event doesn't cause a stop, the thread > + needs to be single-stepped past the single-step breakpoint before > + we can switch back to the original stepping thread. */ > + int hit_singlestep_breakpoint; > +}; > + > +/* Clear ECS and set it to point at TP. */ > > static void > +reset_ecs (struct execution_control_state *ecs, struct thread_info *tp) > +{ > + memset (ecs, 0, sizeof (*ecs)); > + ecs->event_thread = tp; > + ecs->ptid = tp->ptid; > +} > + > +static void keep_going_pass (struct execution_control_state *ecs); > +static void prepare_to_wait (struct execution_control_state *ecs); > +static int thread_still_needs_step_over (struct thread_info *tp); > + > +/* Are there any pending step-over requests? If so, run one now and > + return true. Otherwise, return false. */ > + > +static int > start_step_over (void) > { > struct thread_info *tp, *next; > > for (tp = step_over_queue_head; tp != NULL; tp = next) > { > - ptid_t ptid; > - struct displaced_step_inferior_state *displaced; > - struct regcache *regcache; > - struct gdbarch *gdbarch; > - CORE_ADDR actual_pc; > - struct address_space *aspace; > - struct inferior *inf = find_inferior_ptid (tp->ptid); > + struct execution_control_state ecss; > + struct execution_control_state *ecs = &ecss; > > next = thread_step_over_chain_next (tp); > > - displaced = get_displaced_stepping_state (inf->pid); > - > /* If this inferior already has a displaced step in process, > don't start a new one. */ > - if (!ptid_equal (displaced->step_ptid, null_ptid)) > + if (displaced_step_in_progress (ptid_get_pid (tp->ptid))) > continue; > > thread_step_over_chain_remove (tp); > @@ -1879,74 +1910,57 @@ start_step_over (void) > "infrun: step-over queue now empty\n"); > } > > - ptid = tp->ptid; > - context_switch (ptid); > - > - regcache = get_thread_regcache (ptid); > - actual_pc = regcache_read_pc (regcache); > - aspace = get_regcache_aspace (regcache); > - gdbarch = get_regcache_arch (regcache); > - > - if (breakpoint_here_p (aspace, actual_pc)) > + if (tp->control.trap_expected || tp->executing) > { > - if (debug_displaced) > - fprintf_unfiltered (gdb_stdlog, > - "displaced: stepping queued %s now\n", > - target_pid_to_str (ptid)); > - > - displaced_step_prepare (ptid); > - > - if (debug_displaced) > - { > - CORE_ADDR actual_pc = regcache_read_pc (regcache); > - gdb_byte buf[4]; > - > - fprintf_unfiltered (gdb_stdlog, "displaced: run %s: ", > - paddress (gdbarch, actual_pc)); > - read_memory (actual_pc, buf, sizeof (buf)); > - displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf)); > - } > - > - if (gdbarch_displaced_step_hw_singlestep (gdbarch, > - displaced->step_closure)) > - target_resume (ptid, 1, GDB_SIGNAL_0); > - else > - target_resume (ptid, 0, GDB_SIGNAL_0); > - > - /* Done, we're stepping a thread. */ > - break; > + internal_error (__FILE__, __LINE__, > + "[%s] has inconsistent state: " > + "trap_expected=%d, executing=%d\n", > + target_pid_to_str (tp->ptid), > + tp->control.trap_expected, > + tp->executing); > } > - else > - { > - int step; > - struct thread_info *tp = inferior_thread (); > - > - /* The breakpoint we were sitting under has since been > - removed. */ > - tp->control.trap_expected = 0; > > - /* Go back to what we were trying to do. */ > - step = currently_stepping (tp); > - > - if (step) > - step = maybe_software_singlestep (gdbarch, actual_pc); > + if (debug_infrun) > + fprintf_unfiltered (gdb_stdlog, > + "infrun: resuming [%s] for step-over\n", > + target_pid_to_str (tp->ptid)); > + > + /* keep_going_pass skips the step-over if the breakpoint is no > + longer inserted. In all-stop, we want to keep looking for a ==== missing word? "... keep looking for a thread" ? > + that needs a step-over instead of resuming TP, because we > + wouldn't be able to resume anything else until the target > + stops again. In non-stop, the resume always resumes only TP, > + so it's OK to let the thread resume freely. */ > + if (!non_stop && !thread_still_needs_step_over (tp)) > + continue; > > - if (debug_displaced) > - fprintf_unfiltered (gdb_stdlog, > - "displaced: breakpoint is gone: %s, step(%d)\n", > - target_pid_to_str (tp->ptid), step); > + switch_to_thread (tp->ptid); > + reset_ecs (ecs, tp); > + keep_going_pass (ecs); > > - target_resume (ptid, step, GDB_SIGNAL_0); > - tp->suspend.stop_signal = GDB_SIGNAL_0; > + if (!ecs->wait_some_more) > + error (_("Command aborted.")); > > - /* This request was discarded. See if there's any other > - thread waiting for its turn. */ > + if (!non_stop) > + { > + /* On all-stop, shouldn't have resumed unless we needed a > + step over. */ > + gdb_assert (tp->control.trap_expected > + || tp->step_after_step_resume_breakpoint); > + > + /* With remote targets (at least), in all-stop, we can't > + issue any further remote commands until the program stops > + again. */ > + return 1; > } > > - /* A new displaced stepping sequence started. Maybe we can > - start a displaced step on a thread of other process. > - Continue looking. */ > + /* Either the thread no longer needed a step-over, or a new > + displaced stepping sequence started. Even in the latter > + case, continue looking. Maybe we can also start another > + displaced step on a thread of other process. */ > } > + > + return 0; > } > > /* Update global variables holding ptids to hold NEW_PTID if they were > @@ -2283,16 +2297,11 @@ resume (enum gdb_signal sig) > > if (!displaced_step_prepare (inferior_ptid)) > { > - /* Got placed in displaced stepping queue. Will be resumed > - later when all the currently queued displaced stepping > - requests finish. The thread is not executing at this > - point, and the call to set_executing will be made later. > - But we need to call set_running here, since from the > - user/frontend's point of view, threads were set running. > - Unless we're calling an inferior function, as in that > - case we pretend the inferior doesn't run at all. */ > - if (!tp->control.in_infcall) > - set_running (user_visible_resume_ptid (user_step), 1); > + if (debug_infrun) > + fprintf_unfiltered (gdb_stdlog, > + "Got placed in displaced stepping queue\n"); ==== Nit: Suggest rewording "displaced stepping queue". IIUC there is no displaced-stepping specific queue anymore. > + > + tp->control.trap_expected = 0; > discard_cleanups (old_cleanups); > return; > } > @@ -2367,14 +2376,6 @@ resume (enum gdb_signal sig) > by applying increasingly restricting conditions. */ > resume_ptid = user_visible_resume_ptid (user_step); > > - /* Even if RESUME_PTID is a wildcard, and we end up resuming less > - (e.g., we might need to step over a breakpoint), from the > - user/frontend's point of view, all threads in RESUME_PTID are now > - running. Unless we're calling an inferior function, as in that > - case pretend we inferior doesn't run at all. */ > - if (!tp->control.in_infcall) > - set_running (resume_ptid, 1); > - > /* Maybe resume a single thread after all. */ > if ((step || thread_has_single_step_breakpoints_set (tp)) > && tp->control.trap_expected) > @@ -2589,48 +2590,6 @@ schedlock_applies (struct thread_info *tp) > && tp->control.stepping_command)); > } > > -/* Look a thread other than EXCEPT that has previously reported a > - breakpoint event, and thus needs a step-over in order to make > - progress. Returns NULL is none is found. */ > - > -static struct thread_info * > -find_thread_needs_step_over (struct thread_info *except) > -{ > - struct thread_info *tp, *current; > - > - /* With non-stop mode on, threads are always handled individually. */ > - gdb_assert (! non_stop); > - > - current = inferior_thread (); > - > - /* If scheduler locking applies, we can avoid iterating over all > - threads. */ > - if (schedlock_applies (except)) > - { > - if (except != current > - && thread_still_needs_step_over (current)) > - return current; > - > - return NULL; > - } > - > - ALL_NON_EXITED_THREADS (tp) > - { > - /* Ignore the EXCEPT thread. */ > - if (tp == except) > - continue; > - /* Ignore threads of processes we're not resuming. */ > - if (!sched_multi > - && ptid_get_pid (tp->ptid) != ptid_get_pid (inferior_ptid)) > - continue; > - > - if (thread_still_needs_step_over (tp)) > - return tp; > - } > - > - return NULL; > -} > - > /* Basic routine for continuing the program in various fashions. > > ADDR is the address to resume at, or -1 for resume where stopped. > @@ -2651,6 +2610,11 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) > struct thread_info *tp; > CORE_ADDR pc; > struct address_space *aspace; > + ptid_t resume_ptid; > + struct execution_control_state ecss; > + struct execution_control_state *ecs = &ecss; > + struct cleanup *old_chain; > + int started; > > /* If we're stopped at a fork/vfork, follow the branch set by the > "set follow-fork-mode" command; otherwise, we'll just proceed > @@ -2713,7 +2677,23 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) > (next/step/etc.), we'll want to print stop event output to the MI > console channel (the stepped-to line, etc.), as if the user > entered the execution command on a real GDB console. */ > - inferior_thread ()->control.command_interp = command_interp (); > + tp->control.command_interp = command_interp (); > + > + resume_ptid = user_visible_resume_ptid (tp->control.stepping_command); > + > + /* If an exception is thrown from this point on, make sure to > + propagate GDB's knowledge of the executing state to the > + frontend/user running state. */ > + old_chain = make_cleanup (finish_thread_state_cleanup, &resume_ptid); > + > + /* Even if RESUME_PTID is a wildcard, and we end up resuming fewer > + threads (e.g., we might need to set threads stepping over > + breakpoints first), from the user/frontend's point of view, all > + threads in RESUME_PTID are now running. Unless we're calling an > + inferior function, as in that case we pretend the inferior > + doesn't run at all. */ > + if (!tp->control.in_infcall) > + set_running (resume_ptid, 1); > > if (debug_infrun) > fprintf_unfiltered (gdb_stdlog, > @@ -2721,91 +2701,92 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) > paddress (gdbarch, addr), > gdb_signal_to_symbol_string (siggnal)); > > - if (non_stop) > - /* In non-stop, each thread is handled individually. The context > - must already be set to the right thread here. */ > - ; > - else > + annotate_starting (); > + > + /* Make sure that output from GDB appears before output from the > + inferior. */ > + gdb_flush (gdb_stdout); > + > + /* In a multi-threaded task we may select another thread and > + then continue or step. > + > + But if a thread that we're resuming had stopped at a breakpoint, > + it will immediately cause another breakpoint stop without any > + execution (i.e. it will report a breakpoint hit incorrectly). So > + we must step over it first. > + > + Look for threads other than the current (TP) that reported a > + breakpoint hit and haven't been resumed yet since. */ > + > + /* If scheduler locking applies, we can avoid iterating over all > + threads. */ > + if (!non_stop && !schedlock_applies (tp)) > { > - struct thread_info *step_over; > + struct thread_info *current = tp; > + > + ALL_NON_EXITED_THREADS (tp) > + { > + /* Ignore the current thread here. It's handled > + afterwards. */ > + if (tp == current) > + continue; > > - /* In a multi-threaded task we may select another thread and > - then continue or step. > + /* Ignore threads of processes we're not resuming. */ > + if (!ptid_match (tp->ptid, resume_ptid)) > + continue; > > - But if the old thread was stopped at a breakpoint, it will > - immediately cause another breakpoint stop without any > - execution (i.e. it will report a breakpoint hit incorrectly). > - So we must step over it first. > + if (!thread_still_needs_step_over (tp)) > + continue; > + > + gdb_assert (!thread_is_in_step_over_chain (tp)); > > - Look for a thread other than the current (TP) that reported a > - breakpoint hit and hasn't been resumed yet since. */ > - step_over = find_thread_needs_step_over (tp); > - if (step_over != NULL) > - { > if (debug_infrun) > fprintf_unfiltered (gdb_stdlog, > "infrun: need to step-over [%s] first\n", > - target_pid_to_str (step_over->ptid)); > + target_pid_to_str (tp->ptid)); > > - /* Store the prev_pc for the stepping thread too, needed by > - switch_back_to_stepped_thread. */ > - tp->prev_pc = regcache_read_pc (get_current_regcache ()); > - switch_to_thread (step_over->ptid); > - tp = step_over; > + thread_step_over_chain_enqueue (tp); > } > - } > - > - /* If we need to step over a breakpoint, and we're not using > - displaced stepping to do so, insert all breakpoints (watchpoints, > - etc.) but the one we're stepping over, step one instruction, and > - then re-insert the breakpoint when that step is finished. */ > - if (tp->stepping_over_breakpoint && !use_displaced_stepping (gdbarch)) > - { > - struct regcache *regcache = get_current_regcache (); > > - set_step_over_info (get_regcache_aspace (regcache), > - regcache_read_pc (regcache), 0); > + tp = current; > } > - else > - clear_step_over_info (); > > - insert_breakpoints (); > + /* Enqueue the current thread last, so that we move all other > + threads over their breakpoints first. */ > + if (tp->stepping_over_breakpoint) > + thread_step_over_chain_enqueue (tp); > > - tp->control.trap_expected = tp->stepping_over_breakpoint; > + /* If the thread isn't started, we'll still need to set its prev_pc, > + so that switch_back_to_stepped_thread knows the thread hasn't > + advanced. Must do this before resuming any thread, as in > + all-stop/remote, once we resume we can't send any other packet > + until the target stops again. */ > + tp->prev_pc = regcache_read_pc (regcache); > > - annotate_starting (); > + started = start_step_over (); > > - /* Make sure that output from GDB appears before output from the > - inferior. */ > - gdb_flush (gdb_stdout); > + if (step_over_info_valid_p ()) > + { > + /* Either this thread started a new in-line step over, or some > + other thread was already doing one. In either case, don't > + resume anything else until the step-over is finished. */ > + } > + else if (started && !non_stop) > + { > + /* A new displaced stepping sequence was started. In all-stop, > + we can't talk to the target anymore until it next stops. */ > + } > + else if (!tp->executing && !thread_is_in_step_over_chain (tp)) > + { > + /* The thread wasn't started, and isn't queued, run it now. */ > + reset_ecs (ecs, tp); > + switch_to_thread (tp->ptid); > + keep_going_pass (ecs); > + if (!ecs->wait_some_more) > + error ("Command aborted."); > + } > > - /* Refresh prev_pc value just prior to resuming. This used to be > - done in stop_waiting, however, setting prev_pc there did not handle > - scenarios such as inferior function calls or returning from > - a function via the return command. In those cases, the prev_pc > - value was not set properly for subsequent commands. The prev_pc value > - is used to initialize the starting line number in the ecs. With an > - invalid value, the gdb next command ends up stopping at the position > - represented by the next line table entry past our start position. > - On platforms that generate one line table entry per line, this > - is not a problem. However, on the ia64, the compiler generates > - extraneous line table entries that do not increase the line number. > - When we issue the gdb next command on the ia64 after an inferior call > - or a return command, we often end up a few instructions forward, still > - within the original line we started. > - > - An attempt was made to refresh the prev_pc at the same time the > - execution_control_state is initialized (for instance, just before > - waiting for an inferior event). But this approach did not work > - because of platforms that use ptrace, where the pc register cannot > - be read unless the inferior is stopped. At that point, we are not > - guaranteed the inferior is stopped and so the regcache_read_pc() call > - can fail. Setting the prev_pc value here ensures the value is updated > - correctly when the inferior is stopped. */ > - tp->prev_pc = regcache_read_pc (get_current_regcache ()); > - > - /* Resume inferior. */ > - resume (tp->suspend.stop_signal); > + discard_cleanups (old_chain); > > /* Wait for it to stop (if not standalone) > and in any case decode why it stopped, and act accordingly. */ > @@ -2873,28 +2854,6 @@ init_wait_for_inferior (void) > } > > > -/* Data to be passed around while handling an event. This data is > - discarded between events. */ > -struct execution_control_state > -{ > - ptid_t ptid; > - /* The thread that got the event, if this was a thread event; NULL > - otherwise. */ > - struct thread_info *event_thread; > - > - struct target_waitstatus ws; > - int stop_func_filled_in; > - CORE_ADDR stop_func_start; > - CORE_ADDR stop_func_end; > - const char *stop_func_name; > - int wait_some_more; > - > - /* True if the event thread hit the single-step breakpoint of > - another thread. Thus the event doesn't cause a stop, the thread > - needs to be single-stepped past the single-step breakpoint before > - we can switch back to the original stepping thread. */ > - int hit_singlestep_breakpoint; > -}; > > static void handle_inferior_event (struct execution_control_state *ecs); > > @@ -2908,7 +2867,6 @@ static void check_exception_resume (struct execution_control_state *, > > static void end_stepping_range (struct execution_control_state *ecs); > static void stop_waiting (struct execution_control_state *ecs); > -static void prepare_to_wait (struct execution_control_state *ecs); > static void keep_going (struct execution_control_state *ecs); > static void process_event_stop_test (struct execution_control_state *ecs); > static int switch_back_to_stepped_thread (struct execution_control_state *ecs); > @@ -4261,6 +4219,34 @@ Cannot fill $_exitsignal with the correct signal number.\n")); > } > } > > +/* Called when we get an event that may finish an in-line or > + out-of-line (displaced stepping) step-over started previously. */ > + > +static void > +finish_step_over (struct execution_control_state *ecs) > +{ > + displaced_step_fixup (ecs->ptid, > + ecs->event_thread->suspend.stop_signal); > + > + if (step_over_info_valid_p ()) > + { > + /* If we're stepping over a breakpoint with all threads locked, > + then only the thread that was stepped should be reporting > + back an event. */ > + gdb_assert (ecs->event_thread->control.trap_expected); > + > + if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP) > + clear_step_over_info (); > + } > + > + if (!non_stop) > + return; > + > + /* Start a new step-over in another thread if there's one that > + needs it. */ > + start_step_over (); > +} > + > /* Come here when the program has stopped with a signal. */ > > static void > @@ -4277,9 +4263,7 @@ handle_signal_stop (struct execution_control_state *ecs) > /* Do we need to clean up the state of a thread that has > completed a displaced single-step? (Doing so usually affects > the PC, so do it here, before we set stop_pc.) */ > - displaced_step_fixup (ecs->ptid, > - ecs->event_thread->suspend.stop_signal); > - start_step_over (); > + finish_step_over (ecs); > > /* If we either finished a single-step or hit a breakpoint, but > the user wanted this thread to be stopped, pretend we got a > @@ -5629,7 +5613,6 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) > { > struct thread_info *tp; > struct thread_info *stepping_thread; > - struct thread_info *step_over; > > /* If any thread is blocked on some internal breakpoint, and we > simply need to step over that breakpoint to get it going > @@ -5672,14 +5655,20 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) > return 1; > } > > - /* Otherwise, we no longer expect a trap in the current thread. > - Clear the trap_expected flag before switching back -- this is > - what keep_going does as well, if we call it. */ > - ecs->event_thread->control.trap_expected = 0; > - > - /* Likewise, clear the signal if it should not be passed. */ > - if (!signal_program[ecs->event_thread->suspend.stop_signal]) > - ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0; > + /* If this thread needs yet another step-over (e.g., stepping > + through a delay slot), do it first before moving on to > + another thread. */ > + if (thread_still_needs_step_over (ecs->event_thread)) > + { > + if (debug_infrun) > + { > + fprintf_unfiltered (gdb_stdlog, > + "infrun: thread [%s] still needs step-over\n", > + target_pid_to_str (ecs->event_thread->ptid)); > + } > + keep_going (ecs); > + return 1; > + } > > /* If scheduler locking applies even if not stepping, there's no > need to walk over threads. Above we've checked whether the > @@ -5689,12 +5678,26 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) > if (schedlock_applies (ecs->event_thread)) > return 0; > > - /* Look for the stepping/nexting thread, and check if any other > - thread other than the stepping thread needs to start a > - step-over. Do all step-overs before actually proceeding with > + /* Otherwise, we no longer expect a trap in the current thread. > + Clear the trap_expected flag before switching back -- this is > + what keep_going does as well, if we call it. */ > + ecs->event_thread->control.trap_expected = 0; > + > + /* Likewise, clear the signal if it should not be passed. */ > + if (!signal_program[ecs->event_thread->suspend.stop_signal]) > + ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0; > + > + /* Do all pending step-overs before actually proceeding with > step/next/etc. */ > + if (start_step_over ()) > + { > + prepare_to_wait (ecs); > + return 1; > + } > + > + /* Look for the stepping/nexting thread. */ > stepping_thread = NULL; > - step_over = NULL; > + > ALL_NON_EXITED_THREADS (tp) > { > /* Ignore threads of processes we're not resuming. */ > @@ -5726,37 +5729,6 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) > > stepping_thread = tp; > } > - else if (thread_still_needs_step_over (tp)) > - { > - step_over = tp; > - > - /* At the top we've returned early if the event thread > - is stepping. If some other thread not the event > - thread is stepping, then scheduler locking can't be > - in effect, and we can resume this thread. No need to > - keep looking for the stepping thread then. */ > - break; > - } > - } > - > - if (step_over != NULL) > - { > - tp = step_over; > - if (debug_infrun) > - { > - fprintf_unfiltered (gdb_stdlog, > - "infrun: need to step-over [%s]\n", > - target_pid_to_str (tp->ptid)); > - } > - > - /* Only the stepping thread should have this set. */ > - gdb_assert (tp->control.step_range_end == 0); > - > - ecs->ptid = tp->ptid; > - ecs->event_thread = tp; > - switch_to_thread (ecs->ptid); > - keep_going (ecs); > - return 1; > } > > if (stepping_thread != NULL) > @@ -5855,7 +5827,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) > fprintf_unfiltered (gdb_stdlog, > "infrun: expected thread still " > "hasn't advanced\n"); > - keep_going (ecs); > + keep_going_pass (ecs); > } > > return 1; > @@ -6270,24 +6242,32 @@ stop_waiting (struct execution_control_state *ecs) > ecs->wait_some_more = 0; > } > > -/* Called when we should continue running the inferior, because the > - current event doesn't cause a user visible stop. This does the > - resuming part; waiting for the next event is done elsewhere. */ > +/* Like keep_going, but passes the signal to the inferior, even if the > + signal is set to nopass. */ > > static void > -keep_going (struct execution_control_state *ecs) > +keep_going_pass (struct execution_control_state *ecs) ==== For whatever weird reasons, "keep_going_pass" doesn't read very well to me. "pass" as in "pass/fail"? "pass" as in one of several passes, and this is the "keep going" one? Can you rename this to keep_going_pass_signal? TIA > { > /* Make sure normal_stop is called if we get a QUIT handled before > reaching resume. */ > struct cleanup *old_cleanups = make_cleanup (resume_cleanups, 0); > > + gdb_assert (ptid_equal (ecs->event_thread->ptid, inferior_ptid)); > + > /* Save the pc before execution, to compare with pc after stop. */ > ecs->event_thread->prev_pc > = regcache_read_pc (get_thread_regcache (ecs->ptid)); > > - if (ecs->event_thread->control.trap_expected > - && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP) > + if (ecs->event_thread->control.trap_expected) > { > + struct thread_info *tp = ecs->event_thread; > + > + if (debug_infrun) > + fprintf_unfiltered (gdb_stdlog, > + "infrun: %s has trap_expected set, " > + "resuming to collect trap\n", > + target_pid_to_str (tp->ptid)); > + > /* We haven't yet gotten our trap, and either: intercepted a > non-signal event (e.g., a fork); or took a signal which we > are supposed to pass through to the inferior. Simply > @@ -6358,20 +6338,6 @@ keep_going (struct execution_control_state *ecs) > > ecs->event_thread->control.trap_expected = (remove_bp || remove_wps); > > - /* Do not deliver GDB_SIGNAL_TRAP (except when the user > - explicitly specifies that such a signal should be delivered > - to the target program). Typically, that would occur when a > - user is debugging a target monitor on a simulator: the target > - monitor sets a breakpoint; the simulator encounters this > - breakpoint and halts the simulation handing control to GDB; > - GDB, noting that the stop address doesn't map to any known > - breakpoint, returns control back to the simulator; the > - simulator then delivers the hardware equivalent of a > - GDB_SIGNAL_TRAP to the program being debugged. */ > - if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP > - && !signal_program[ecs->event_thread->suspend.stop_signal]) > - ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0; > - > discard_cleanups (old_cleanups); > resume (ecs->event_thread->suspend.stop_signal); > } > @@ -6379,6 +6345,22 @@ keep_going (struct execution_control_state *ecs) > prepare_to_wait (ecs); > } > > +/* Called when we should continue running the inferior, because the > + current event doesn't cause a user visible stop. This does the > + resuming part; waiting for the next event is done elsewhere. */ > + > +static void > +keep_going (struct execution_control_state *ecs) > +{ > + if (ecs->event_thread->control.trap_expected > + && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP) > + ecs->event_thread->control.trap_expected = 0; > + > + if (!signal_program[ecs->event_thread->suspend.stop_signal]) > + ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0; > + keep_going_pass (ecs); > +} > + > /* This function normally comes after a resume, before > handle_inferior_event exits. It takes care of any last bits of > housekeeping, and sets the all-important wait_some_more flag. */ > @@ -7806,8 +7788,19 @@ leave it stopped or free to run as needed."), > signal_catch[i] = 0; > } > > - /* Signals caused by debugger's own actions > - should not be given to the program afterwards. */ > + /* Signals caused by debugger's own actions should not be given to > + the program afterwards. > + > + Do not deliver GDB_SIGNAL_TRAP by default, except when the user > + explicitly specifies that it should be delivered to the target > + program. Typically, that would occur when a user is debugging a > + target monitor on a simulator: the target monitor sets a > + breakpoint; the simulator encounters this breakpoint and halts > + the simulation handing control to GDB; GDB, noting that the stop > + address doesn't map to any known breakpoint, returns control back > + to the simulator; the simulator then delivers the hardware > + equivalent of a GDB_SIGNAL_TRAP to the program being > + debugged. */ > signal_program[GDB_SIGNAL_TRAP] = 0; > signal_program[GDB_SIGNAL_INT] = 0; > > diff --git a/gdb/thread.c b/gdb/thread.c > index 28e5ef8..3e3f419 100644 > --- a/gdb/thread.c > +++ b/gdb/thread.c > @@ -852,44 +852,62 @@ thread_change_ptid (ptid_t old_ptid, ptid_t new_ptid) > observer_notify_thread_ptid_changed (old_ptid, new_ptid); > } > > +/* Helper for set_running, that marks one thread either running or > + stopped. */ > + > +static int > +set_running_thread (struct thread_info *tp, int running) > +{ > + int started = 0; > + > + if (running && tp->state == THREAD_STOPPED) > + started = 1; > + tp->state = running ? THREAD_RUNNING : THREAD_STOPPED; > + > + if (!running) > + { > + /* If the thread is now marked stopped, remove it from > + the step-over queue, so that we don't try to resume > + it until the user wants it to. */ > + if (tp->step_over_next != NULL) > + thread_step_over_chain_remove (tp); > + } > + > + return started; > +} > + > void > set_running (ptid_t ptid, int running) > { > struct thread_info *tp; > int all = ptid_equal (ptid, minus_one_ptid); > + int any_started = 0; > > /* We try not to notify the observer if no thread has actually changed > the running state -- merely to reduce the number of messages to > frontend. Frontend is supposed to handle multiple *running just fine. */ > if (all || ptid_is_pid (ptid)) > { > - int any_started = 0; > - > for (tp = thread_list; tp; tp = tp->next) > if (all || ptid_get_pid (tp->ptid) == ptid_get_pid (ptid)) > { > if (tp->state == THREAD_EXITED) > continue; > - if (running && tp->state == THREAD_STOPPED) > + > + if (set_running_thread (tp, running)) > any_started = 1; > - tp->state = running ? THREAD_RUNNING : THREAD_STOPPED; > } > - if (any_started) > - observer_notify_target_resumed (ptid); > } > else > { > - int started = 0; > - > tp = find_thread_ptid (ptid); > - gdb_assert (tp); > + gdb_assert (tp != NULL); > gdb_assert (tp->state != THREAD_EXITED); > - if (running && tp->state == THREAD_STOPPED) > - started = 1; > - tp->state = running ? THREAD_RUNNING : THREAD_STOPPED; > - if (started) > - observer_notify_target_resumed (ptid); > + if (set_running_thread (tp, running)) > + any_started = 1; > } > + if (any_started) > + observer_notify_target_resumed (ptid); > } > > static int > @@ -1008,9 +1026,8 @@ finish_thread_state (ptid_t ptid) > continue; > if (all || ptid_get_pid (ptid) == ptid_get_pid (tp->ptid)) > { > - if (tp->executing && tp->state == THREAD_STOPPED) > + if (set_running_thread (tp, tp->executing)) > any_started = 1; > - tp->state = tp->executing ? THREAD_RUNNING : THREAD_STOPPED; > } > } > } > @@ -1020,9 +1037,8 @@ finish_thread_state (ptid_t ptid) > gdb_assert (tp); > if (tp->state != THREAD_EXITED) > { > - if (tp->executing && tp->state == THREAD_STOPPED) > + if (set_running_thread (tp, tp->executing)) > any_started = 1; > - tp->state = tp->executing ? THREAD_RUNNING : THREAD_STOPPED; > } > } > > -- > 1.9.3 >