From: Daniel Jacobowitz <drow@false.org>
To: Jie Zhang <jzhang918@gmail.com>
Cc: gdb@sources.redhat.com
Subject: Re: [RFC] Reinsert the single stepping breakpoint after being hopped over
Date: Tue, 02 Aug 2005 15:45:00 -0000 [thread overview]
Message-ID: <20050802154456.GA5342@nevyn.them.org> (raw)
In-Reply-To: <6f48278f05080208114fb35462@mail.gmail.com>
On Tue, Aug 02, 2005 at 11:11:45PM +0800, Jie Zhang wrote:
> Hi,
>
> I'm working on a new port of GDB for Blackfin. It's usually used for
> remote debugging using gdbserver and software single stepping. It has
> many FAILs for schedlock.exp. I traced these FAILs and, I think, found
> a bug in GDB.
>
> Assume there are two threads in the program and we are stepping on
> one. (I use GDB for the whole of GDB and gdbserver.)
>
> 1. GDB inserts a breakpoint for stepping and resumes both threads.
> 2. Both threads hit this breakpoint.
> 3. The first stopped thread GDB looks for (for some reason) is not the
> thread we are stepping. GDB make it hop over the stepping breakpoint.
> 4. GDB resume both threads by
>
> resume (1, TARGET_SIGNAL_0);
>
> So the thread we are stepping will do a new step and the previous stop
> of step will never be noticed by GDB. (It's caught by gdbserver and
> saved as a pending status. However, when it stops again for the new
> step, the previous stepping breakpoint has been removed, so
> check_removed_breakpoint () will clear status_pending_p and the
> previous stop is lost.) GDB will never get a chance to check if the
> stepping is out of the range. The step command will not return.
Yes... I fixed this once already, but I failed to consider the case
where both threads have hit the singlestep breakpoint, rather than
another thread hitting the breakpoint before the original thread has
had a chance to step.
I've fixed the version you encountered before, but it appears I never
submitted the patch. I'm not sure if it still applies, but could you
give this a try?
> To fix it, GDB should put the previous stepping breakpoint back, not
> do a new stepping when resuming both threads. The following patch is
> trying to fix this. It adds a new field "singlestep_breakpoint_addr"
> in "struct thread_info" to remember the last single stepping
> breakpoint address. Passing 2 as the second argument to
> SOFTWARE_SINGLE_STEP () to tell the target software single stepping
> function that we just reinsert the previous single stepping
> breakpoint.
Interesting approach. I don't like the implementation - I'd rather not
extend context_switch - but the concept may be better than mine. Let
me think about this for a little.
--
Daniel Jacobowitz
CodeSourcery, LLC
Index: infrun.c
===================================================================
RCS file: /big/fsf/rsync/src-cvs/src/gdb/infrun.c,v
retrieving revision 1.137
diff -u -p -r1.137 infrun.c
--- infrun.c 16 Feb 2004 20:49:51 -0000 1.137
+++ infrun.c 6 Mar 2004 04:23:06 -0000
@@ -479,6 +479,9 @@ static int singlestep_breakpoints_insert
/* The thread we inserted single-step breakpoints for. */
static ptid_t singlestep_ptid;
+/* PC when we started this single-step. */
+static CORE_ADDR singlestep_pc;
+
/* If another thread hit the singlestep breakpoint, we save the original
thread here so that we can resume single-stepping it later. */
static ptid_t saved_singlestep_ptid;
@@ -570,6 +573,7 @@ resume (int step, enum target_signal sig
`wait_for_inferior' */
singlestep_breakpoints_inserted_p = 1;
singlestep_ptid = inferior_ptid;
+ singlestep_pc = read_pc ();
}
/* Handle any optimized stores to the inferior NOW... */
@@ -1201,6 +1205,7 @@ context_switch (struct execution_control
&ecs->current_line, &ecs->current_symtab, &step_sp);
}
inferior_ptid = ecs->ptid;
+ flush_cached_frames ();
}
/* Wrapper for PC_IN_SIGTRAMP that takes care of the need to find the
@@ -1803,7 +1808,10 @@ handle_inferior_event (struct execution_
}
else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
{
+ gdb_assert (ptid_equal (inferior_ptid, singlestep_ptid));
+
ecs->random_signal = 0;
+
/* The call to in_thread_list is necessary because PTIDs sometimes
change when we go from single-threaded to multi-threaded. If
the singlestep_ptid is still in the list, assume that it is
@@ -1811,9 +1819,32 @@ handle_inferior_event (struct execution_
if (!ptid_equal (singlestep_ptid, ecs->ptid)
&& in_thread_list (singlestep_ptid))
{
- thread_hop_needed = 1;
- stepping_past_singlestep_breakpoint = 1;
- saved_singlestep_ptid = singlestep_ptid;
+ /* If the PC of the thread we were trying to single-step
+ has changed, discard this event (which we were going
+ to ignore anyway), and pretend we saw that thread
+ trap. This runs a risk of losing signal information
+ for singlestep_ptid, but prevents us continuously
+ moving the single-step breakpoint. This situation
+ means that the thread has trapped or been signalled,
+ but the event has not been reported to GDB yet.
+ Really we should arrange to report all events, or to
+ re-poll the remote looking for this particular
+ thread (i.e. temporarily enable schedlock). */
+ if (read_pc_pid (singlestep_ptid) != singlestep_pc)
+ {
+ /* The current context still belongs to
+ singlestep_ptid. Don't swap here, since that's
+ the context we want to use. Just fudge our
+ state and continue. */
+ ecs->ptid = singlestep_ptid;
+ stop_pc = read_pc_pid (ecs->ptid);
+ }
+ else
+ {
+ thread_hop_needed = 1;
+ stepping_past_singlestep_breakpoint = 1;
+ saved_singlestep_ptid = singlestep_ptid;
+ }
}
}
@@ -1942,8 +1973,6 @@ handle_inferior_event (struct execution_
if (context_hook)
context_hook (pid_to_thread_id (ecs->ptid));
-
- flush_cached_frames ();
}
if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
next prev parent reply other threads:[~2005-08-02 15:45 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-08-02 15:11 Jie Zhang
2005-08-02 15:45 ` Daniel Jacobowitz [this message]
2005-08-09 13:45 ` Jie Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20050802154456.GA5342@nevyn.them.org \
--to=drow@false.org \
--cc=gdb@sources.redhat.com \
--cc=jzhang918@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).