Re: [BUG/discussion] set scheduler-locking on get internal-error(maybe about multi-inferior)

[Pedro Alves <pedro at codesourcery dot com>]

[moved to gdb-patches@]

On Tuesday 20 October 2009 11:02:14, Pedro Alves wrote:
> On Tuesday 20 October 2009 08:56:08, Hui Zhu wrote:
> > Hi guys,
> > 
> > I got some error with multi-thread and low arch-linux-nat.  I
> > reproduced it in i386-linux, I am not sure it affect other arch or
> > not.
> 
> Thanks.
> 
> > (gdb) set scheduler-locking on
> > (gdb) si
> > ../../src/gdb/target.c:2567: internal-error: Can't determine the
> > current address space of thread process 16277
> 
> > The bug issue is:
> > In linux-nat.c:linux_nat_resume
> >   /* Convert to something the lower layer understands.  */
> >   ptid = pid_to_ptid (GET_LWP (lp->ptid));
> > 
> > 
> > In i386-linux-nat.c:i386_linux_resume
> > int pid = PIDGET (ptid);
> > struct regcache *regcache = get_thread_regcache (pid_to_ptid (pid));
> > 
> > The pid in i386_linux_resume is lwp, get_thread_regcache will not get
> > the right ptid.
> 
> Right.
> 
> > 
> > I don't have any good idea with this bug.  There is too much "int pid
> > = PIDGET (ptid);" In arch-linux-nat function.   I am not sure it can
> > really work well with multi-inferior.  This level code looks don't
> > have good way if the ptid is get from " ptid = pid_to_ptid (GET_LWP
> > (lp->ptid));".
> 
> I'm testing a patch.
> 

I've applied the patch below.  Thanks again!

-- 
Pedro Alves

2009-10-20  Pedro Alves  <pedro@codesourcery.com>

	* linux-nat.c (linux_nat_thread_address_space): New.
	(linux_nat_add_target): Install it.
	* progspace.c (address_space_num): New.
	* progspace.h (address_space_num): Declare.
	* target.c (target_thread_address_space): Really query the target.
	* target.h (struct target_ops) <to_thread_address_space>: New
	field.

---
 gdb/linux-nat.c |   34 ++++++++++++++++++++++++++++++++++
 gdb/progspace.c |    6 ++++++
 gdb/progspace.h |    3 +++
 gdb/target.c    |   19 +++++++++++++++++--
 gdb/target.h    |    7 +++++++
 5 files changed, 67 insertions(+), 2 deletions(-)

Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c	2009-10-20 11:42:05.000000000 +0100
+++ src/gdb/linux-nat.c	2009-10-20 12:07:21.000000000 +0100
@@ -5311,6 +5311,39 @@ linux_nat_close (int quitting)
     linux_ops->to_close (quitting);
 }
 
+/* When requests are passed down from the linux-nat layer to the
+   single threaded inf-ptrace layer, ptids of (lwpid,0,0) form are
+   used.  The address space pointer is stored in the inferior object,
+   but the common code that is passed such ptid can't tell whether
+   lwpid is a "main" process id or not (it assumes so).  We reverse
+   look up the "main" process id from the lwp here.  */
+
+struct address_space *
+linux_nat_thread_address_space (struct target_ops *t, ptid_t ptid)
+{
+  struct lwp_info *lwp;
+  struct inferior *inf;
+  int pid;
+
+  pid = GET_LWP (ptid);
+  if (GET_LWP (ptid) == 0)
+    {
+      /* An (lwpid,0,0) ptid.  Look up the lwp object to get at the
+	 tgid.  */
+      lwp = find_lwp_pid (ptid);
+      pid = GET_PID (lwp->ptid);
+    }
+  else
+    {
+      /* A (pid,lwpid,0) ptid.  */
+      pid = GET_PID (ptid);
+    }
+
+  inf = find_inferior_pid (pid);
+  gdb_assert (inf != NULL);
+  return inf->aspace;
+}
+
 void
 linux_nat_add_target (struct target_ops *t)
 {
@@ -5333,6 +5366,7 @@ linux_nat_add_target (struct target_ops 
   t->to_thread_alive = linux_nat_thread_alive;
   t->to_pid_to_str = linux_nat_pid_to_str;
   t->to_has_thread_control = tc_schedlock;
+  t->to_thread_address_space = linux_nat_thread_address_space;
 
   t->to_can_async_p = linux_nat_can_async_p;
   t->to_is_async_p = linux_nat_is_async_p;
Index: src/gdb/progspace.c
===================================================================
--- src.orig/gdb/progspace.c	2009-10-20 11:42:05.000000000 +0100
+++ src/gdb/progspace.c	2009-10-20 11:54:34.000000000 +0100
@@ -89,6 +89,12 @@ free_address_space (struct address_space
   xfree (aspace);
 }
 
+int
+address_space_num (struct address_space *aspace)
+{
+  return aspace->num;
+}
+
 /* Start counting over from scratch.  */
 
 static void
Index: src/gdb/progspace.h
===================================================================
--- src.orig/gdb/progspace.h	2009-10-20 11:42:05.000000000 +0100
+++ src/gdb/progspace.h	2009-10-20 11:54:34.000000000 +0100
@@ -254,6 +254,9 @@ extern struct address_space *new_address
    share an address space.  */
 extern struct address_space *maybe_new_address_space (void);
 
+/* Returns the integer address space id of ASPACE.  */
+extern int address_space_num (struct address_space *aspace);
+
 /* Update all program spaces matching to address spaces.  The user may
    have created several program spaces, and loaded executables into
    them before connecting to the target interface that will create the
Index: src/gdb/target.c
===================================================================
--- src.orig/gdb/target.c	2009-10-20 11:42:05.000000000 +0100
+++ src/gdb/target.c	2009-10-20 11:54:34.000000000 +0100
@@ -2555,10 +2555,25 @@ target_get_osdata (const char *type)
 struct address_space *
 target_thread_address_space (ptid_t ptid)
 {
+  struct address_space *aspace;
   struct inferior *inf;
+  struct target_ops *t;
+
+  for (t = current_target.beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_thread_address_space != NULL)
+	{
+	  aspace = t->to_thread_address_space (t, ptid);
+	  gdb_assert (aspace);
 
-  /* For now, assume frame chains and inferiors only see one address
-     space.  */
+	  if (targetdebug)
+	    fprintf_unfiltered (gdb_stdlog,
+				"target_thread_address_space (%s) = %d\n",
+				target_pid_to_str (ptid),
+				address_space_num (aspace));
+	  return aspace;
+	}
+    }
 
   /* Fall-back to the "main" address space of the inferior.  */
   inf = find_inferior_pid (ptid_get_pid (ptid));
Index: src/gdb/target.h
===================================================================
--- src.orig/gdb/target.h	2009-10-20 11:42:05.000000000 +0100
+++ src/gdb/target.h	2009-10-20 11:56:10.000000000 +0100
@@ -585,6 +585,13 @@ struct target_ops
        The default implementation always returns target_gdbarch.  */
     struct gdbarch *(*to_thread_architecture) (struct target_ops *, ptid_t);
 
+    /* Determine current address space of thread PTID.
+
+       The default implementation always returns the inferior's
+       address space.  */
+    struct address_space *(*to_thread_address_space) (struct target_ops *,
+						      ptid_t);
+
     int to_magic;
     /* Need sub-structure for target machine related rather than comm related?
      */