New testsuite errors with gdbserver (remote)

New testsuite errors with gdbserver (remote)

[Michael Snyder]

Hi,

Actually I don't know how new these are -- certainly newer than the
7.2 release.  Is anybody else running the testsuite with 
native-gdbserver.exp?

I'm seeing hundreds of instances of this assert failure:

/data/home/msnyder/cvs/localhost/src/gdb/thread.c:619: internal-error: 
is_thread_state: Assertion `tp' failed.


Michael

Re: New testsuite errors with gdbserver (remote)

[Michael Snyder]

Michael Snyder wrote:
> Hi,
> 
> Actually I don't know how new these are -- certainly newer than the
> 7.2 release.  Is anybody else running the testsuite with 
> native-gdbserver.exp?
> 
> I'm seeing hundreds of instances of this assert failure:
> 
> /data/home/msnyder/cvs/localhost/src/gdb/thread.c:619: internal-error: 
> is_thread_state: Assertion `tp' failed.


Luckily, this shows up in head but not in the current release branch.
So it need not hold up the 7.2.1 release.

To see an instance of this, run

     make check RUNTESTFLAGS="--target_board=native-gdbserver bang.exp"

Re: New testsuite errors with gdbserver (remote)

[Michael Snyder]

Michael Snyder wrote:
> Michael Snyder wrote:
>> Hi,
>>
>> Actually I don't know how new these are -- certainly newer than the
>> 7.2 release.  Is anybody else running the testsuite with 
>> native-gdbserver.exp?
>>
>> I'm seeing hundreds of instances of this assert failure:
>>
>> /data/home/msnyder/cvs/localhost/src/gdb/thread.c:619: internal-error: 
>> is_thread_state: Assertion `tp' failed.
> 
> 
> Luckily, this shows up in head but not in the current release branch.
> So it need not hold up the 7.2.1 release.
> 
> To see an instance of this, run
> 
>      make check RUNTESTFLAGS="--target_board=native-gdbserver bang.exp"
> 

I have binary-searched it down to a change occurring in the main trunk, 
on February 4.  One of these:


P bfd/version.h
P gdb/ChangeLog
P gdb/Makefile.in
P gdb/NEWS
P gdb/breakpoint.c
P gdb/breakpoint.h
P gdb/dwarf2read.c
P gdb/mips-linux-tdep.c
P gdb/regcache.c
U gdb/version.in
P gdb/data-directory/Makefile.in
P gdb/doc/ChangeLog
P gdb/doc/gdb.texinfo
P gdb/doc/gdbint.texinfo
cvs update: gdb/python/py-bpevent.c is no longer in the repository
P gdb/python/py-breakpoint.c
cvs update: gdb/python/py-continueevent.c is no longer in the repository
cvs update: gdb/python/py-event.c is no longer in the repository
cvs update: gdb/python/py-event.h is no longer in the repository
cvs update: gdb/python/py-events.h is no longer in the repository
cvs update: gdb/python/py-evtregistry.c is no longer in the repository
cvs update: gdb/python/py-evts.c is no longer in the repository
cvs update: gdb/python/py-exitedevent.c is no longer in the repository
P gdb/python/py-inferior.c
cvs update: gdb/python/py-signalevent.c is no longer in the repository
cvs update: gdb/python/py-stopevent.c is no longer in the repository
cvs update: gdb/python/py-stopevent.h is no longer in the repository
cvs update: gdb/python/py-threadevent.c is no longer in the repository
P gdb/python/python-internal.h
P gdb/python/python.c
cvs update: gdb/syscalls/mips-n32-linux.xml is no longer in the repository
cvs update: gdb/syscalls/mips-n64-linux.xml is no longer in the repository
cvs update: gdb/syscalls/mips-o32-linux.xml is no longer in the repository
P gdb/testsuite/ChangeLog
P gdb/testsuite/gdb.base/catch-syscall.exp
cvs update: gdb/testsuite/gdb.python/py-events.c is no longer in the 
repository
cvs update: gdb/testsuite/gdb.python/py-events.exp is no longer in the 
repository
cvs update: gdb/testsuite/gdb.python/py-events.py is no longer in the 
repository
cvs update: gdb/testsuite/gdb.python/py-evthreads.c is no longer in the 
repository
cvs update: gdb/testsuite/gdb.python/py-evthreads.exp is no longer in 
the repository

Re: New testsuite errors with gdbserver (remote)

[Michael Snyder]

Michael Snyder wrote:
> Michael Snyder wrote:
>> Michael Snyder wrote:
>>> Hi,
>>>
>>> Actually I don't know how new these are -- certainly newer than the
>>> 7.2 release.  Is anybody else running the testsuite with 
>>> native-gdbserver.exp?
>>>
>>> I'm seeing hundreds of instances of this assert failure:
>>>
>>> /data/home/msnyder/cvs/localhost/src/gdb/thread.c:619: internal-error: 
>>> is_thread_state: Assertion `tp' failed.
>>
>> Luckily, this shows up in head but not in the current release branch.
>> So it need not hold up the 7.2.1 release.
>>
>> To see an instance of this, run
>>
>>      make check RUNTESTFLAGS="--target_board=native-gdbserver bang.exp"
>>
> 
> I have binary-searched it down to a change occurring in the main trunk, 
> on February 4.  One of these:

Somewhat to my surprise, these new failures narrow down to this change:

2011-02-04  Sami Wagiaalla  <swagiaal@redhat.com>
                 Oguz Kayral <oguzkayral@gmail.com>

         * python/py-inferior.c (python_on_normal_stop): New function.
         (python_on_resume): New function.
         (python_inferior_exit): New function.
         (gdbpy_initialize_inferior): Add normal_stop, target_resumed, and
         inferior_exit observers.
         * python/py-evtregistry.c: New file.
         * python/py-threadevent.c : New file.
         * python/py-event.c: New file.
         * python/py-evts.c: New file.
         * python/py-continueevent.c: New file.
         * python/py-bpevent.c: New file.
         * python/py-signalevent.c: New file.
         * python/py-exetiedevent.c: New file.
         * python/py-breakpoint.c (gdbpy_breakpoint_from_bpstats): New 
function.
         Move struct breakpoint_object from here...
         * python/python-internal.h: ... to here.
         * python/py-event.h: New file.
         * python/py-events.h: New file.
         * Makefile.in (SUBDIR_PYTHON_OBS): Add py-breakpointstopevent.o,
         py-continueevent.o, py-event.o, py-eventregistry.o, py-events.o,
         py-exitedevent.o, py-signalstopevent.o, and py-stopevent.o.
         (SUBDIR_PYTHON_SRCS): Add py-breakpointstopevent.c,
         py-continueevent.c, py-event.c, py-eventregistry.c, py-events.c,
         py-exitedevent.c, py-signalstopevent.c, and py-stopevent.c.
         Add build rules for all the above.

Re: New testsuite errors with gdbserver (remote)

[Keith Seitz]

On 02/17/2011 10:48 AM, Michael Snyder wrote:
> Somewhat to my surprise, these new failures narrow down to this change:

I've given this a peek, and here's what I've found out...

When the exit notification is sent, and python picks it up, it is 
setting up the python environment (architecture and language). The 
python exit callback does this by calling get_current_arch.

The problem is that (long before this) wait_for_inferior sets the 
inferior with TID = 0. It is this inferior object that is then passed 
around for the event notification. Alas, unlike natives, where the 
inferior is popped, the inferior is still around.

As a result, target_has_stack goes to check the thread status and cannot 
find the appropriate thread (since the one in the list hasn't been 
cleared). This leads to the internal error you're seeing.

I'm not sure how valid it is to call get_current_arch from an exit 
event. Maybe it is. Maybe it isn't. Someone who knows a lot more about 
this is going to have to investigate this or guide me.

Nonetheless, if you pass target_gdbarch instead to ensure_python_env in 
python_inferior_exit (in python/py_inferior.c), things will behave much 
better. Once again, I don't know if this is strictly correct, but it 
should at least get you going with gdbserver again.

I hope this leads someone to a better solution.

Keith

Index: python/py-inferior.c
===================================================================
RCS file: /cvs/src/src/gdb/python/py-inferior.c,v
retrieving revision 1.5
diff -u -p -r1.5 py-inferior.c
--- python/py-inferior.c	4 Feb 2011 21:54:16 -0000	1.5
+++ python/py-inferior.c	17 Feb 2011 20:46:08 -0000
@@ -116,7 +116,7 @@ python_inferior_exit (struct inferior *i
    ptid_t ptidp;
    struct target_waitstatus status;

-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  cleanup = ensure_python_env (target_gdbarch, current_language);

    get_last_target_status (&ptidp, &status);

[RFA] Fix for testsuite errors with gdbserver (remote)

[Pierre Muller]

> Nonetheless, if you pass target_gdbarch instead to ensure_python_env in
> python_inferior_exit (in python/py_inferior.c), things will behave much
> better. Once again, I don't know if this is strictly correct, but it
> should at least get you going with gdbserver again.
> 
> I hope this leads someone to a better solution.

  I investigated a little further and it seems to me
that the problem is that instead of a null_ptid,
the TARGET_WAITKIND_EXITED event has a (pid,0,0) form,
which is also not a valid thread, but doesn't get
filtered out yet by has_stack_frames  before
calling is_exited.


  I first wanted to add a test "lwp == 0 && tid == 0"
inside has_stack_frames, but as I was not sure that
other target could not have valid thread with a zero identifier,
I thought it would be safer to look for a fix inside
remote specific code.

  Inverting two lines in remote_close fixes hopefully this issue.
At least using --target_board=native_gdbsderver.exp
works again!

Is this patch OK?

 2011-02-18  Pierre Muller  <muller@ics.u-strasbg.fr>

	* remote.c (remote_close): Reset INFERIOR_PTID to NULL_PTID
	before calling discard_all_inferiors.

Index: src/gdb/remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.434
diff -u -p -r1.434 remote.c
--- src/gdb/remote.c	14 Feb 2011 11:22:29 -0000	1.434
+++ src/gdb/remote.c	18 Feb 2011 16:41:42 -0000
@@ -2908,9 +2908,11 @@ remote_close (int quitting)
   remote_desc = NULL;
 
   /* We don't have a connection to the remote stub anymore.  Get rid
-     of all the inferiors and their threads we were controlling.  */
-  discard_all_inferiors ();
+     of all the inferiors and their threads we were controlling.
+     Reset inferior_ptid to null_ptid first, as otherwise has_stack_frames
+     will be unable to find the thread corresponding to (pid, 0, 0).  */
   inferior_ptid = null_ptid;
+  discard_all_inferiors ();
 
   /* We're no longer interested in any of these events.  */
   discard_pending_stop_replies (-1);

Re: New testsuite errors with gdbserver (remote)

[Pedro Alves]

On Thursday 17 February 2011 20:47:48, Keith Seitz wrote:
> On 02/17/2011 10:48 AM, Michael Snyder wrote:
> > Somewhat to my surprise, these new failures narrow down to this change:
> 
> I've given this a peek, and here's what I've found out...
> 
> When the exit notification is sent, and python picks it up, it is 
> setting up the python environment (architecture and language). The 
> python exit callback does this by calling get_current_arch.
> 
> The problem is that (long before this) wait_for_inferior sets the 
> inferior with TID = 0. It is this inferior object that is then passed 
> around for the event notification. Alas, unlike natives, where the 
> inferior is popped, the inferior is still around.
> 
> As a result, target_has_stack goes to check the thread status and cannot 
> find the appropriate thread (since the one in the list hasn't been 
> cleared). This leads to the internal error you're seeing.
> 

> I'm not sure how valid it is to call get_current_arch from an exit 
> event. Maybe it is. Maybe it isn't. Someone who knows a lot more about 
> this is going to have to investigate this or guide me.

/* Return "current" architecture.  If the target is running, this is
   the architecture of the selected frame.  Otherwise, the "current"
   architecture defaults to the target architecture.

   This function should normally be called solely by the command
   interpreter routines to determine the architecture to execute a
   command in.  */
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
struct gdbarch *
get_current_arch (void)

The exited observer are called at other times, not when
preparing to execute a command in.  It is quite bogus to assume
the current architecture, frame and language have anything to do
with the inferior that has been passed is as argument to the
observer.

> @@ -116,7 +116,7 @@ python_inferior_exit (struct inferior *i
>     ptid_t ptidp;
>     struct target_waitstatus status;
> 
> -  cleanup = ensure_python_env (get_current_arch (), current_language);
> +  cleanup = ensure_python_env (target_gdbarch, current_language);
> 
>     get_last_target_status (&ptidp, &status);

I can't say that I'm very familiar with most of the python layer
code, but, why does this observer need to call ensure_python_env
in the first place?  Or if it does, why doesn't it call it with
python_gdbarch like other places in py-inferior that are internal
helpers, not implementing a user command?

-- 
Pedro Alves