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

[Hui Zhu <teawater at gmail dot com>]

Thanks Pedro,  it's OK now.

Hui

On Tue, Oct 20, 2009 at 19:12, Pedro Alves <pedro@codesourcery.com> wrote:
> [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?
> ? ? ?*/
>