From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13579 invoked by alias); 9 Sep 2009 08:27:50 -0000 Mailing-List: contact archer-help@sourceware.org; run by ezmlm Sender: Precedence: bulk List-Post: List-Help: List-Subscribe: List-Id: Received: (qmail 13361 invoked by uid 22791); 9 Sep 2009 08:27:46 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Date: Wed, 09 Sep 2009 08:27:00 -0000 From: Jan Kratochvil To: Andrew Sutherland Cc: archer@sourceware.org Subject: Re: [python] python-inferior add_inferior_object tries to access thread info before it exists with remote target (VMware remote stub) Message-ID: <20090909082728.GA17791@host0.dyn.jankratochvil.net> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.19 (2009-01-05) X-SW-Source: 2009-q3/txt/msg00193.txt.bz2 On Wed, 09 Sep 2009 05:34:25 +0200, Andrew Sutherland wrote: > Using the python branch tip, attempting to connect to a VMware > (Workstation 6.5.x) replay remote stub, I get a "thread.c:581: > internal-error: is_thread_state: Assertion `tp' failed." assertion. This is workarounded a similiar way in archer-jankratochvil-fedora12 by d9c6e0c7789438aeabf053556f39a838536400e4. FSF GDB has a chicked-and-egg problem that the handlers registered on observer_notify_new_inferior need already a valid thread list but the handlers registered on observer_notify_new_thread need already a valid inferior. The initial thread for inferior should be initialized before the new-inferior notification is being sent. Tried to patch it before (attached) but got out of time, I should finish it soon. Thanks, Jan diff --git a/gdb/corelow.c b/gdb/corelow.c index 49de82d..092c16d 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -371,21 +371,11 @@ core_open (char *filename, int from_tty) push_target (&core_ops); discard_cleanups (old_chain); - add_inferior_silent (corelow_pid); - - /* Do this before acknowledging the inferior, so if - post_create_inferior throws (can happen easilly if you're loading - a core file with the wrong exec), we aren't left with threads - from the previous inferior. */ - init_thread_list (); - /* Set INFERIOR_PTID early, so an upper layer can rely on it being set while in the target_find_new_threads call below. */ inferior_ptid = pid_to_ptid (corelow_pid); - /* Assume ST --- Add a main task. We'll later detect when we go - from ST to MT. */ - add_thread_silent (inferior_ptid); + add_inferior_silent (inferior_ptid); /* Need to flush the register cache (and the frame cache) from a previous debug session. If inferior_ptid ends up the same as the diff --git a/gdb/fork-child.c b/gdb/fork-child.c index 9eadbc9..aa814c5 100644 --- a/gdb/fork-child.c +++ b/gdb/fork-child.c @@ -395,11 +395,11 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env, if (!have_inferiors ()) init_thread_list (); - add_inferior (pid); - /* Needed for wait_for_inferior stuff below. */ inferior_ptid = pid_to_ptid (pid); + add_inferior (inferior_ptid); + new_tty_postfork (); /* We have something that executes now. We'll be running through diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index aecd820..8872a18 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -202,6 +202,8 @@ extern struct thread_info *add_thread (ptid_t ptid); about new thread. */ extern struct thread_info *add_thread_silent (ptid_t ptid); +extern struct thread_info *add_thread_silent_no_notify (ptid_t ptid); + /* Same as add_thread, and sets the private info. */ extern struct thread_info *add_thread_with_info (ptid_t ptid, struct private_thread_info *); diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c index 836a819..e981ed3 100644 --- a/gdb/go32-nat.c +++ b/gdb/go32-nat.c @@ -713,12 +713,10 @@ go32_create_inferior (struct target_ops *ops, char *exec_file, save_npx (); #endif - inferior_ptid = pid_to_ptid (SOME_PID); - add_inferior_silent (SOME_PID); - push_target (&go32_ops); - add_thread_silent (inferior_ptid); + inferior_ptid = pid_to_ptid (SOME_PID); + add_inferior_silent (SOME_PID); clear_proceed_status (); insert_breakpoints (); diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c index 89a37a6..275ab55 100644 --- a/gdb/inf-ptrace.c +++ b/gdb/inf-ptrace.c @@ -225,7 +225,7 @@ inf_ptrace_attach (struct target_ops *ops, char *args, int from_tty) inferior_ptid = pid_to_ptid (pid); - inf = add_inferior (pid); + inf = add_inferior (inferior_ptid); inf->attach_flag = 1; /* Always add a main thread. If some target extends the ptrace diff --git a/gdb/inferior.c b/gdb/inferior.c index 43eacda..4286c71 100644 --- a/gdb/inferior.c +++ b/gdb/inferior.c @@ -71,13 +71,14 @@ init_inferior_list (void) } struct inferior * -add_inferior_silent (int pid) +add_inferior_silent (ptid_t ptid) { struct inferior *inf; + struct thread_info *tp; inf = xmalloc (sizeof (*inf)); memset (inf, 0, sizeof (*inf)); - inf->pid = pid; + inf->pid = ptid_get_pid (ptid); inf->stop_soon = NO_STOP_QUIETLY; @@ -85,18 +86,30 @@ add_inferior_silent (int pid) inf->next = inferior_list; inferior_list = inf; - observer_notify_new_inferior (pid); + /* Do this before acknowledging the inferior, so if + post_create_inferior throws (can happen easilly if you're loading + a core file with the wrong exec), we aren't left with threads + from the previous inferior. */ + init_thread_list (); + + /* Assume ST --- Add a main task. We'll later detect when we go + from ST to MT. */ + tp = add_thread_silent_no_notify (ptid); + + observer_notify_new_inferior (ptid_get_pid (ptid)); + + observer_notify_new_thread (tp); return inf; } struct inferior * -add_inferior (int pid) +add_inferior (ptid_t ptid) { - struct inferior *inf = add_inferior_silent (pid); + struct inferior *inf = add_inferior_silent (ptid); if (print_inferior_events) - printf_unfiltered (_("[New inferior %d]\n"), pid); + printf_unfiltered (_("[New inferior %d]\n"), ptid_get_pid (ptid)); return inf; } diff --git a/gdb/inferior.h b/gdb/inferior.h index 7312e51..057129a 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -434,11 +434,11 @@ extern void init_inferior_list (void); inferior is found, and return the pointer to the new inferior. Caller may use this pointer to initialize the private inferior data. */ -extern struct inferior *add_inferior (int pid); +extern struct inferior *add_inferior (ptid_t ptid); /* Same as add_inferior, but don't print new inferior notifications to the CLI. */ -extern struct inferior *add_inferior_silent (int pid); +extern struct inferior *add_inferior_silent (ptid_t ptid); /* Delete an existing inferior list entry, due to inferior exit. */ extern void delete_inferior (int pid); diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 1308844..24d2307 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -636,7 +636,7 @@ linux_child_follow_fork (struct target_ops *ops, int follow_child) struct cleanup *old_chain; /* Add process to GDB's tables. */ - child_inf = add_inferior (child_pid); + child_inf = add_inferior (pid_to_ptid (child_pid)); parent_inf = current_inferior (); child_inf->attach_flag = parent_inf->attach_flag; @@ -728,7 +728,7 @@ linux_child_follow_fork (struct target_ops *ops, int follow_child) /* Add the new inferior first, so that the target_detach below doesn't unpush the target. */ - child_inf = add_inferior (child_pid); + child_inf = add_inferior (pid_to_ptid (child_pid)); parent_inf = current_inferior (); child_inf->attach_flag = parent_inf->attach_flag; diff --git a/gdb/monitor.c b/gdb/monitor.c index 1e61afd..c845f55 100644 --- a/gdb/monitor.c +++ b/gdb/monitor.c @@ -822,8 +822,7 @@ monitor_open (char *args, struct monitor_ops *mon_ops, int from_tty) /* Make run command think we are busy... */ inferior_ptid = monitor_ptid; - add_inferior_silent (ptid_get_pid (inferior_ptid)); - add_thread_silent (inferior_ptid); + add_inferior_silent (inferior_ptid); /* Give monitor_wait something to read */ diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c index 7ca16c7..2587c77 100644 --- a/gdb/remote-sim.c +++ b/gdb/remote-sim.c @@ -472,7 +472,6 @@ gdbsim_create_inferior (struct target_ops *target, char *exec_file, char *args, inferior_ptid = remote_sim_ptid; add_inferior_silent (ptid_get_pid (inferior_ptid)); - add_thread_silent (inferior_ptid); insert_breakpoints (); /* Needed to get correct instruction in cache */ diff --git a/gdb/remote.c b/gdb/remote.c index e30e699..1587d9e 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1168,7 +1168,7 @@ remote_query_attached (int pid) not. */ static struct inferior * -remote_add_inferior (int pid, int attached) +remote_add_inferior (ptid_t ptid, int attached) { struct remote_state *rs = get_remote_state (); struct inferior *inf; @@ -1177,9 +1177,9 @@ remote_add_inferior (int pid, int attached) considered attached, or if is to be considered to have been spawned by the stub. */ if (attached == -1) - attached = remote_query_attached (pid); + attached = remote_query_attached (ptid_get_pid (ptid)); - inf = add_inferior (pid); + inf = add_inferior (ptid); inf->attach_flag = attached; @@ -1259,7 +1259,7 @@ remote_notice_new_inferior (ptid_t currthread, int running) may not know about it yet. Add it before adding its child thread, so notifications are emitted in a sensible order. */ if (!in_inferior_list (ptid_get_pid (currthread))) - inf = remote_add_inferior (ptid_get_pid (currthread), -1); + inf = remote_add_inferior (currthread, -1); /* This is really a new thread. Add it. */ remote_add_thread (currthread, running); @@ -2712,7 +2712,7 @@ remote_start_remote (struct ui_out *uiout, void *opaque) /* Now, if we have thread information, update inferior_ptid. */ inferior_ptid = remote_current_thread (inferior_ptid); - remote_add_inferior (ptid_get_pid (inferior_ptid), -1); + remote_add_inferior (inferior_ptid, -1); /* Always add the main thread. */ add_thread_silent (inferior_ptid); @@ -3485,10 +3485,10 @@ extended_remote_attach_1 (struct target_ops *target, char *args, int from_tty) error (_("Attaching to %s failed"), target_pid_to_str (pid_to_ptid (pid))); - remote_add_inferior (pid, 1); - inferior_ptid = pid_to_ptid (pid); + remote_add_inferior (inferior_ptid, 1); + if (non_stop) { struct thread_info *thread; @@ -6798,8 +6798,7 @@ extended_remote_create_inferior_1 (char *exec_file, char *args, /* Now, if we have thread information, update inferior_ptid. */ inferior_ptid = remote_current_thread (inferior_ptid); - remote_add_inferior (ptid_get_pid (inferior_ptid), 0); - add_thread_silent (inferior_ptid); + remote_add_inferior (inferior_ptid, 0); /* Get updated offsets, if the stub uses qOffsets. */ get_offsets (); diff --git a/gdb/thread.c b/gdb/thread.c index 815c82d..7c15098 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -163,7 +163,7 @@ new_thread (ptid_t ptid) } struct thread_info * -add_thread_silent (ptid_t ptid) +add_thread_silent_no_notify (ptid_t ptid) { struct thread_info *tp; @@ -197,8 +197,6 @@ add_thread_silent (ptid_t ptid) tp->state_ = THREAD_STOPPED; switch_to_thread (ptid); - observer_notify_new_thread (tp); - /* All done. */ return tp; } @@ -207,7 +205,14 @@ add_thread_silent (ptid_t ptid) delete_thread (ptid); } - tp = new_thread (ptid); + return new_thread (ptid); +} + +struct thread_info * +add_thread_silent (ptid_t ptid) +{ + struct thread_info *tp = add_thread_silent_no_notify (ptid); + observer_notify_new_thread (tp); return tp; @@ -499,6 +504,9 @@ thread_alive (struct thread_info *tp) void prune_threads (void) { +/* Regressing on: gdb.server/server-run.exp + thread.c:581: internal-error: is_thread_state: Assertion `tp' failed. */ +#if 0 struct thread_info *tp; struct thread_info **prevp = &thread_list; @@ -514,6 +522,7 @@ prune_threads (void) else prevp = &tp->next; } +#endif } void