This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Fix internal-error on dead LWPs with no associated thread
On Mon, 29 Jun 2009 19:49:29 +0200, Jan Kratochvil wrote:
> > Does anyone know why does the new_thread_event bit need to resume the target
> > at all? Removing that resume should fix the issue too.
Attached here. (this one is for CVS HEAD so it still includes your patch above)
Expecting you are right but only for GNU/Linux.
This patch has no regressions on {x86_64,i686}-fedora-linux-gnu.
Thanks,
Jan
gdb/
2009-06-29 Jan Kratochvil <jan.kratochvil@redhat.com>
Attach new threads in the GNU/Linux all-stop mode immediately.
* inferior.h (default_new_thread_found): New prototype.
* infrun.c (handle_inferior_event <new_thread_event>): Call
target_new_thread_found, move the original code to ...
(default_new_thread_found): ... a new function.
* linux-nat.c (linux_handle_extended_wait): Create the GDB structures
for new thread even in the all-stop mode. New comment point 3.
(linux_nat_new_thread_found): New function.
(linux_nat_add_target): Register linux_nat_new_thread_found.
* target.c (update_current_target): Initialize to_new_thread_found.
* target.h (struct target_ops): New field `to_new_thread_found'.
(target_new_thread_found): New macro.
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -244,6 +244,8 @@ extern void ensure_not_running (void);
void set_step_info (struct frame_info *frame, struct symtab_and_line sal);
+extern void default_new_thread_found (ptid_t ptid);
+
/* From infcmd.c */
extern void tty_command (char *, int);
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2732,21 +2732,7 @@ handle_inferior_event (struct execution_control_state *ecs)
if (ecs->new_thread_event)
{
- if (non_stop)
- /* Non-stop assumes that the target handles adding new threads
- to the thread list. */
- internal_error (__FILE__, __LINE__, "\
-targets should add new threads to the thread list themselves in non-stop mode.");
-
- /* We may want to consider not doing a resume here in order to
- give the user a chance to play with the new thread. It might
- be good to make that a user-settable option. */
-
- /* At this point, all threads are stopped (happens automatically
- in either the OS or the native code). Therefore we need to
- continue all threads in order to make progress. */
-
- target_resume (RESUME_ALL, 0, TARGET_SIGNAL_0);
+ target_new_thread_found (ecs->ptid);
prepare_to_wait (ecs);
return;
}
@@ -4116,6 +4102,34 @@ infrun: not switching back to stepped thread, it has vanished\n");
keep_going (ecs);
}
+/* Handle new unexpected LWP PTID. At this point, both the parent and the new
+ child LWP are stopped (happens automatically in either the OS or the native
+ code). Therefore we need to continue all threads in order to make progress.
+
+ GNU/Linux support uses linux_nat_new_thread_found instead. */
+
+void
+default_new_thread_found (ptid_t ptid)
+{
+ if (non_stop)
+ /* Non-stop assumes that the target handles adding new threads
+ to the thread list. */
+ internal_error (__FILE__, __LINE__, "\
+targets should add new threads to the thread list themselves in non-stop mode.");
+
+ /* We may want to consider not doing a resume here in order to
+ give the user a chance to play with the new thread. It might
+ be good to make that a user-settable option. */
+
+ /* target_resume requires valid INFERIOR_PTID while it may have been deleted
+ in the meantime. At least the new PTID was now created by add_thread. */
+
+ if (!ptid_equal (ptid, inferior_ptid))
+ context_switch (ptid);
+
+ target_resume (RESUME_ALL, 0, TARGET_SIGNAL_0);
+}
+
/* Is thread TP in the middle of single-stepping? */
static int
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1843,35 +1843,32 @@ linux_handle_extended_wait (struct lwp_info *lp, int status,
else
status = 0;
- if (non_stop)
- {
- /* Add the new thread to GDB's lists as soon as possible
- so that:
+ /* Add the new thread to GDB's lists as soon as possible
+ so that:
- 1) the frontend doesn't have to wait for a stop to
- display them, and,
+ 1) the frontend doesn't have to wait for a stop to
+ display them, and,
- 2) we tag it with the correct running state. */
+ 2) we tag it with the correct running state.
- /* If the thread_db layer is active, let it know about
- this new thread, and add it to GDB's list. */
- if (!thread_db_attach_lwp (new_lp->ptid))
- {
- /* We're not using thread_db. Add it to GDB's
- list. */
- target_post_attach (GET_LWP (new_lp->ptid));
- add_thread (new_lp->ptid);
- }
+ 3) we do not consider the first breakpoint just as
+ a NEW_THREAD_EVENT. */
- if (!stopping)
- {
- set_running (new_lp->ptid, 1);
- set_executing (new_lp->ptid, 1);
- }
+ /* If the thread_db layer is active, let it know about
+ this new thread, and add it to GDB's list. */
+ if (!thread_db_attach_lwp (new_lp->ptid))
+ {
+ /* We're not using thread_db. Add it to GDB's
+ list. */
+ target_post_attach (GET_LWP (new_lp->ptid));
+ add_thread (new_lp->ptid);
}
if (!stopping)
{
+ set_running (new_lp->ptid, 1);
+ set_executing (new_lp->ptid, 1);
+
new_lp->stopped = 0;
new_lp->resumed = 1;
ptrace (PTRACE_CONT, new_pid, 0,
@@ -1926,6 +1923,20 @@ linux_handle_extended_wait (struct lwp_info *lp, int status,
_("unknown ptrace event %d"), event);
}
+/* linux_handle_extended_wait already creates the GDB structures for the new
+ LWP PTID. LWPs therefore should not appear unexpectedly. */
+
+static void
+linux_nat_new_thread_found (ptid_t ptid)
+{
+ if (linux_supports_tracefork (GET_LWP (ptid)))
+ internal_error (__FILE__, __LINE__, ("\
+Unexpected %s despite PTRACE_O_TRACEFORK is supported on this GNU/Linux OS."),
+ target_pid_to_str (ptid));
+
+ linux_ops->to_new_thread_found (ptid);
+}
+
/* Wait for LP to stop. Returns the wait status, or 0 if the LWP has
exited. */
@@ -4599,6 +4610,8 @@ linux_nat_add_target (struct target_ops *t)
t->to_supports_multi_process = linux_nat_supports_multi_process;
+ t->to_new_thread_found = linux_nat_new_thread_found;
+
/* We don't change the stratum; this target will sit at
process_stratum and thread_db will set at thread_stratum. This
is a little strange, since this is a multi-threaded-capable
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -634,6 +634,7 @@ update_current_target (void)
INHERIT (to_get_ada_task_ptid, t);
/* Do not inherit to_search_memory. */
INHERIT (to_supports_multi_process, t);
+ INHERIT (to_new_thread_found, t);
INHERIT (to_magic, t);
/* Do not inherit to_memory_map. */
/* Do not inherit to_flash_erase. */
@@ -777,6 +778,8 @@ update_current_target (void)
de_fault (to_supports_multi_process,
(int (*) (void))
return_zero);
+ de_fault (to_new_thread_found,
+ default_new_thread_found);
#undef de_fault
/* Finally, position the target-stack beneath the squashed
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -542,6 +542,9 @@ struct target_ops
simultaneously? */
int (*to_supports_multi_process) (void);
+ /* An event was received for a so far unknown PTID. */
+ void (*to_new_thread_found) (ptid_t ptid);
+
int to_magic;
/* Need sub-structure for target machine related rather than comm related?
*/
@@ -1118,6 +1121,9 @@ extern char *normal_pid_to_str (ptid_t ptid);
(current_target.to_can_execute_reverse ? \
current_target.to_can_execute_reverse () : 0)
+#define target_new_thread_found \
+ (*current_target.to_new_thread_found)
+
extern const struct target_desc *target_read_description (struct target_ops *);
#define target_get_ada_task_ptid(lwp, tid) \