sim_engine_halt() broken in binutils 2.34

sim_engine_halt() broken in binutils 2.34

[William Tambe]

I am running GDB with target sim.

Calling sim_engine_halt() triggers the following error:
../../binutils-gdb/gdb/inferior.c:286: internal-error: inferior*
find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0'
failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)

Any idea idea what changed ?

Re: sim_engine_halt() broken in binutils 2.34

[Andrew Burgess]

* William Tambe <tambewilliam@gmail.com> [2020-02-02 00:03:40 -0500]:

> I am running GDB with target sim.
> 
> Calling sim_engine_halt() triggers the following error:
> ../../binutils-gdb/gdb/inferior.c:286: internal-error: inferior*
> find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0'
> failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n)
> 
> Any idea idea what changed ?

I suspect that this is a similar issue (but not the exact same fix) as
something I already posted here:

   https://sourceware.org/ml/gdb-patches/2020-01/msg00980.html

You could double check if you like, I believe if you check out the
commit just before 5b6d1e4fa4fc682 then you'll find your issue goes
away.

The problem is caused, I suspect, by a use of `inferior_ptid' at a
place where this used to not be the null_ptid, but now is null_ptid.

I had a little nose around for places related to the simulator where
this bug might be hiding, but couldn't see anything obvious.

If you could let us know which simulator you're using, and what you do
to trigger the call to sim_engine_halt then we might be able to
reproduce the issue.

You could also try to collect a backtrace from GDB at the point the
assertion triggers, I believe GDB offers to dump core on an assertion,
so you can use that to obtain the backtrace.  We might get lucky, and
something in the backtrace could be the offender.

Best of luck!

Andrew

Re: sim_engine_halt() broken in binutils 2.34

[William Tambe]

On Sun, Feb 2, 2020 at 4:47 PM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>
> * William Tambe <tambewilliam@gmail.com> [2020-02-02 00:03:40 -0500]:
>
> > I am running GDB with target sim.
> >
> > Calling sim_engine_halt() triggers the following error:
> > ../../binutils-gdb/gdb/inferior.c:286: internal-error: inferior*
> > find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0'
> > failed.
> > A problem internal to GDB has been detected,
> > further debugging may prove unreliable.
> > Quit this debugging session? (y or n)
> >
> > Any idea idea what changed ?
>
> I suspect that this is a similar issue (but not the exact same fix) as
> something I already posted here:
>
>    https://sourceware.org/ml/gdb-patches/2020-01/msg00980.html

I tried using above fix, but it did not resolve the issue.

>
> You could double check if you like, I believe if you check out the
> commit just before 5b6d1e4fa4fc682 then you'll find your issue goes
> away.

I could not quickly test above as I need to do rebase adjustments for
the custom port that I am using.

>
> The problem is caused, I suspect, by a use of `inferior_ptid' at a
> place where this used to not be the null_ptid, but now is null_ptid.
>
> I had a little nose around for places related to the simulator where
> this bug might be hiding, but couldn't see anything obvious.
>
> If you could let us know which simulator you're using, and what you do
> to trigger the call to sim_engine_halt then we might be able to
> reproduce the issue.

The simulator I am using is a custom port that is not upstream.

sim_engine_halt() is used by simulators to do things like SIM_SIGTRAP,
SIGABRT, SIGILL or SIM_SIGSEGV;
The fact that sim_engine_halt() is broken makes gdb target sim useless
because breakpoints cannot be set.

>
> You could also try to collect a backtrace from GDB at the point the
> assertion triggers, I believe GDB offers to dump core on an assertion,
> so you can use that to obtain the backtrace.  We might get lucky, and
> something in the backtrace could be the offender.

See below backtrace from GDB when assert triggers due to
sim_engine_halt() getting called for a breakpoint using SIM_SIGTRAP:

../../binutils-gdb/gdb/inferior.c:279: internal-error: inferior*
find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0'
failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)
(gdb) bt
#0  0x00007ffff66c9bf9 in __GI___poll (fds=0x555555fd36e0, nfds=4,
timeout=timeout@entry=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
#1  0x00005555556ecf90 in poll (__timeout=-1, __nfds=<optimized out>,
__fds=<optimized out>) at
/usr/include/x86_64-linux-gnu/bits/poll2.h:46
#2  gdb_wait_for_event (block=block@entry=1) at
../../binutils-gdb/gdb/event-loop.c:769
#3  0x00005555556ed152 in gdb_do_one_event () at
../../binutils-gdb/gdb/event-loop.c:346
#4  0x00005555556ed286 in gdb_do_one_event () at
../../binutils-gdb/gdb/event-loop.c:352
#5  0x000055555585d60f in gdb_readline_wrapper (prompt=<optimized
out>) at ../../binutils-gdb/gdb/top.c:1078
#6  0x000055555588db16 in defaulted_query(const char *, char, typedef
__va_list_tag __va_list_tag *) (ctlstr=<optimized out>,
defchar=defchar@entry=0 '\000', args=args@entry=0x7fffffffd740)
    at ../../binutils-gdb/gdb/utils.c:867
#7  0x000055555588de5b in query (ctlstr=<optimized out>) at
../../binutils-gdb/gdb/utils.c:959
#8  0x000055555588e3a6 in internal_vproblem(internal_problem *, const
char *, int, const char *, typedef __va_list_tag __va_list_tag *) (
    problem=problem@entry=0x555555d59880 <internal_error_problem>,
file=<optimized out>, line=279, fmt=<optimized out>,
ap=ap@entry=0x7fffffffd900) at ../../binutils-gdb/gdb/utils.c:372
#9  0x000055555588e4eb in internal_verror (file=<optimized out>,
line=<optimized out>, fmt=<optimized out>, ap=ap@entry=0x7fffffffd900)
at ../../binutils-gdb/gdb/utils.c:438
#10 0x00005555559590df in internal_error
(file=file@entry=0x5555559f2cd0 "../../binutils-gdb/gdb/inferior.c",
line=line@entry=279, fmt=<optimized out>)
    at ../../binutils-gdb/gdbsupport/errors.c:55
#11 0x000055555572e72f in find_inferior_pid (targ=<optimized out>,
pid=<optimized out>) at ../../binutils-gdb/gdb/inferior.c:279
#12 0x0000555555856905 in find_thread_ptid
(targ=targ@entry=0x555555d58f60 <gdbsim_ops>, ptid=...) at
../../binutils-gdb/gdb/thread.c:527
#13 0x000055555573f42d in handle_inferior_event
(ecs=ecs@entry=0x7fffffffdd30) at ../../binutils-gdb/gdb/infrun.c:5062
#14 0x000055555574096f in fetch_inferior_event (client_data=<optimized
out>) at ../../binutils-gdb/gdb/infrun.c:4002
#15 0x00005555556ed19c in check_async_event_handlers () at
../../binutils-gdb/gdb/event-loop.c:1062
#16 gdb_do_one_event () at ../../binutils-gdb/gdb/event-loop.c:325
#17 0x00005555556ed2be in gdb_do_one_event () at
../../binutils-gdb/gdb/event-loop.c:303
#18 start_event_loop () at ../../binutils-gdb/gdb/event-loop.c:370
#19 0x0000555555765018 in captured_command_loop () at
../../binutils-gdb/gdb/main.c:360
#20 0x0000555555766f75 in captured_main (data=<optimized out>) at
../../binutils-gdb/gdb/main.c:1203
#21 gdb_main (args=<optimized out>) at ../../binutils-gdb/gdb/main.c:1218
#22 0x00005555555bff1b in main (argc=<optimized out>, argv=<optimized
out>) at ../../binutils-gdb/gdb/gdb.c:32

>
> Best of luck!
>
> Andrew
>

Re: sim_engine_halt() broken in binutils 2.34

[Andrew Burgess]

* William Tambe <tambewilliam@gmail.com> [2020-02-02 21:12:00 -0500]:

> On Sun, Feb 2, 2020 at 4:47 PM Andrew Burgess
> <andrew.burgess@embecosm.com> wrote:
> >
> > * William Tambe <tambewilliam@gmail.com> [2020-02-02 00:03:40 -0500]:
> >
> > > I am running GDB with target sim.
> > >
> > > Calling sim_engine_halt() triggers the following error:
> > > ../../binutils-gdb/gdb/inferior.c:286: internal-error: inferior*
> > > find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0'
> > > failed.
> > > A problem internal to GDB has been detected,
> > > further debugging may prove unreliable.
> > > Quit this debugging session? (y or n)
> > >
> > > Any idea idea what changed ?
> >
> > I suspect that this is a similar issue (but not the exact same fix) as
> > something I already posted here:
> >
> >    https://sourceware.org/ml/gdb-patches/2020-01/msg00980.html
> 
> I tried using above fix, but it did not resolve the issue.
> 
> >
> > You could double check if you like, I believe if you check out the
> > commit just before 5b6d1e4fa4fc682 then you'll find your issue goes
> > away.
> 
> I could not quickly test above as I need to do rebase adjustments for
> the custom port that I am using.
> 
> >
> > The problem is caused, I suspect, by a use of `inferior_ptid' at a
> > place where this used to not be the null_ptid, but now is null_ptid.
> >
> > I had a little nose around for places related to the simulator where
> > this bug might be hiding, but couldn't see anything obvious.
> >
> > If you could let us know which simulator you're using, and what you do
> > to trigger the call to sim_engine_halt then we might be able to
> > reproduce the issue.
> 
> The simulator I am using is a custom port that is not upstream.
> 
> sim_engine_halt() is used by simulators to do things like SIM_SIGTRAP,
> SIGABRT, SIGILL or SIM_SIGSEGV;
> The fact that sim_engine_halt() is broken makes gdb target sim useless
> because breakpoints cannot be set.

Indeed, but the problem here isn't that sim_engine_halt is broken,
it's that something else is broken, and stopping the simulator reveals
the issue.

So in this case the bug is about pids not being 0 when GDB tries to
lookup the inferior in its internal database.  It will do this lookup
when an inferior stops, for example, when the simulator stops by
calling sim_engine_halt.

However, there's a clear separation of responsibilities here, the
simulator code simulates a target, while wrapping the simulator up as
a "fake" process, with a pid is done in gdb/remote-sim.c, so one thing
we can be reasonably certain of is that the issue is not with
sim_engine_halt, but with remote-sim.

Further, I've tested using a different simulator, and I don't see this
same problem, nor looking at the code do I see how this problem could
occur, so my instinct right now is that this is something that has
maybe been broken in your out of tree port.

> 
> >
> > You could also try to collect a backtrace from GDB at the point the
> > assertion triggers, I believe GDB offers to dump core on an assertion,
> > so you can use that to obtain the backtrace.  We might get lucky, and
> > something in the backtrace could be the offender.
> 
> See below backtrace from GDB when assert triggers due to
> sim_engine_halt() getting called for a breakpoint using SIM_SIGTRAP:
> 
> ../../binutils-gdb/gdb/inferior.c:279: internal-error: inferior*
> find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0'
> failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n)
> (gdb) bt
> #0  0x00007ffff66c9bf9 in __GI___poll (fds=0x555555fd36e0, nfds=4,
> timeout=timeout@entry=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
> #1  0x00005555556ecf90 in poll (__timeout=-1, __nfds=<optimized out>,
> __fds=<optimized out>) at
> /usr/include/x86_64-linux-gnu/bits/poll2.h:46
> #2  gdb_wait_for_event (block=block@entry=1) at
> ../../binutils-gdb/gdb/event-loop.c:769
> #3  0x00005555556ed152 in gdb_do_one_event () at
> ../../binutils-gdb/gdb/event-loop.c:346
> #4  0x00005555556ed286 in gdb_do_one_event () at
> ../../binutils-gdb/gdb/event-loop.c:352
> #5  0x000055555585d60f in gdb_readline_wrapper (prompt=<optimized
> out>) at ../../binutils-gdb/gdb/top.c:1078
> #6  0x000055555588db16 in defaulted_query(const char *, char, typedef
> __va_list_tag __va_list_tag *) (ctlstr=<optimized out>,
> defchar=defchar@entry=0 '\000', args=args@entry=0x7fffffffd740)
>     at ../../binutils-gdb/gdb/utils.c:867
> #7  0x000055555588de5b in query (ctlstr=<optimized out>) at
> ../../binutils-gdb/gdb/utils.c:959
> #8  0x000055555588e3a6 in internal_vproblem(internal_problem *, const
> char *, int, const char *, typedef __va_list_tag __va_list_tag *) (
>     problem=problem@entry=0x555555d59880 <internal_error_problem>,
> file=<optimized out>, line=279, fmt=<optimized out>,
> ap=ap@entry=0x7fffffffd900) at ../../binutils-gdb/gdb/utils.c:372
> #9  0x000055555588e4eb in internal_verror (file=<optimized out>,
> line=<optimized out>, fmt=<optimized out>, ap=ap@entry=0x7fffffffd900)
> at ../../binutils-gdb/gdb/utils.c:438
> #10 0x00005555559590df in internal_error
> (file=file@entry=0x5555559f2cd0 "../../binutils-gdb/gdb/inferior.c",
> line=line@entry=279, fmt=<optimized out>)
>     at ../../binutils-gdb/gdbsupport/errors.c:55
> #11 0x000055555572e72f in find_inferior_pid (targ=<optimized out>,
> pid=<optimized out>) at ../../binutils-gdb/gdb/inferior.c:279
> #12 0x0000555555856905 in find_thread_ptid
> (targ=targ@entry=0x555555d58f60 <gdbsim_ops>, ptid=...) at
> ../../binutils-gdb/gdb/thread.c:527
> #13 0x000055555573f42d in handle_inferior_event
> (ecs=ecs@entry=0x7fffffffdd30) at ../../binutils-gdb/gdb/infrun.c:5062
> #14 0x000055555574096f in fetch_inferior_event (client_data=<optimized
> out>) at ../../binutils-gdb/gdb/infrun.c:4002
> #15 0x00005555556ed19c in check_async_event_handlers () at
> ../../binutils-gdb/gdb/event-loop.c:1062
> #16 gdb_do_one_event () at ../../binutils-gdb/gdb/event-loop.c:325
> #17 0x00005555556ed2be in gdb_do_one_event () at
> ../../binutils-gdb/gdb/event-loop.c:303
> #18 start_event_loop () at ../../binutils-gdb/gdb/event-loop.c:370
> #19 0x0000555555765018 in captured_command_loop () at
> ../../binutils-gdb/gdb/main.c:360
> #20 0x0000555555766f75 in captured_main (data=<optimized out>) at
> ../../binutils-gdb/gdb/main.c:1203
> #21 gdb_main (args=<optimized out>) at ../../binutils-gdb/gdb/main.c:1218
> #22 0x00005555555bff1b in main (argc=<optimized out>, argv=<optimized
> out>) at ../../binutils-gdb/gdb/gdb.c:32

What this shows us is as follows:

  1. The null_ptid (with the 0 value) is present in
  handle_inferior_event at line 5062 (frame 13).  Now your line
  numbers don't match up exactly with top of tree, but the closest
  line looks like this:

  ecs->event_thread = find_thread_ptid (ecs->target, ecs->ptid);

  So we're interested in where ecs->ptid came from.

  2. Follow this back into frame 14, then look back and ecs is
  populated with this call:

  if (!do_target_wait (minus_one_ptid, ecs, TARGET_WNOHANG))
     ....

  3. In do_target_wait ecs->ptid is initialised with:

  ecs->ptid = do_target_wait_1 (inf, wait_ptid, &ecs->ws, options);

  4. The return value of do_target_wait_1 comes from this code:

  if (deprecated_target_wait_hook)
    event_ptid = deprecated_target_wait_hook (ptid, status, options);
  else
    event_ptid = target_wait (ptid, status, options);

  return event_ptid;

  And we're almost certainly following the 'target_wait' path unless
  your target is doing something vert weird, and 'target_wait' just
  calls the 'wait' method on the current target class, which should
  get us into 'gdbsim_target::wait'.

  5. In gdbsim_target::wait we always return
  sim_data->remote_sim_ptid, so the question becomes how is this
  initialised.

  6. The only initialisation I can see for this variable is this line:

  remote_sim_ptid (next_pid, 0, next_pid)

  So then we ask, what is next_pid?  At this point I would add the
  following assertion to gdb:

  ## START ##

diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 281232cc4e5..b7ce4672a1a 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -86,6 +86,7 @@ struct sim_inferior_data {
     : gdbsim_desc (desc),
       remote_sim_ptid (next_pid, 0, next_pid)
   {
+    gdb_assert (remote_sim_ptid != null_ptid);
     ++next_pid;
   }

  ## END ##

  7. My guess is that in your port this will fail, but for other ports
  this doesn't fail.  For other ports next_pid is initialised to 42000
  in gdbsim_target_open - is it possible that this initialisation has
  been removed in your tree somehow?

Thanks,
Andrew

Re: sim_engine_halt() broken in binutils 2.34

[William Tambe]

On Mon, Feb 3, 2020 at 5:05 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>
> * William Tambe <tambewilliam@gmail.com> [2020-02-02 21:12:00 -0500]:
>
> > On Sun, Feb 2, 2020 at 4:47 PM Andrew Burgess
> > <andrew.burgess@embecosm.com> wrote:
> > >
> > > * William Tambe <tambewilliam@gmail.com> [2020-02-02 00:03:40 -0500]:
> > >
> > > > I am running GDB with target sim.
> > > >
> > > > Calling sim_engine_halt() triggers the following error:
> > > > ../../binutils-gdb/gdb/inferior.c:286: internal-error: inferior*
> > > > find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0'
> > > > failed.
> > > > A problem internal to GDB has been detected,
> > > > further debugging may prove unreliable.
> > > > Quit this debugging session? (y or n)
> > > >
> > > > Any idea idea what changed ?
> > >
> > > I suspect that this is a similar issue (but not the exact same fix) as
> > > something I already posted here:
> > >
> > >    https://sourceware.org/ml/gdb-patches/2020-01/msg00980.html
> >
> > I tried using above fix, but it did not resolve the issue.
> >
> > >
> > > You could double check if you like, I believe if you check out the
> > > commit just before 5b6d1e4fa4fc682 then you'll find your issue goes
> > > away.
> >
> > I could not quickly test above as I need to do rebase adjustments for
> > the custom port that I am using.
> >
> > >
> > > The problem is caused, I suspect, by a use of `inferior_ptid' at a
> > > place where this used to not be the null_ptid, but now is null_ptid.
> > >
> > > I had a little nose around for places related to the simulator where
> > > this bug might be hiding, but couldn't see anything obvious.
> > >
> > > If you could let us know which simulator you're using, and what you do
> > > to trigger the call to sim_engine_halt then we might be able to
> > > reproduce the issue.
> >
> > The simulator I am using is a custom port that is not upstream.
> >
> > sim_engine_halt() is used by simulators to do things like SIM_SIGTRAP,
> > SIGABRT, SIGILL or SIM_SIGSEGV;
> > The fact that sim_engine_halt() is broken makes gdb target sim useless
> > because breakpoints cannot be set.
>
> Indeed, but the problem here isn't that sim_engine_halt is broken,
> it's that something else is broken, and stopping the simulator reveals
> the issue.
>
> So in this case the bug is about pids not being 0 when GDB tries to
> lookup the inferior in its internal database.  It will do this lookup
> when an inferior stops, for example, when the simulator stops by
> calling sim_engine_halt.
>
> However, there's a clear separation of responsibilities here, the
> simulator code simulates a target, while wrapping the simulator up as
> a "fake" process, with a pid is done in gdb/remote-sim.c, so one thing
> we can be reasonably certain of is that the issue is not with
> sim_engine_halt, but with remote-sim.
>
> Further, I've tested using a different simulator, and I don't see this
> same problem, nor looking at the code do I see how this problem could
> occur, so my instinct right now is that this is something that has
> maybe been broken in your out of tree port.
>
> >
> > >
> > > You could also try to collect a backtrace from GDB at the point the
> > > assertion triggers, I believe GDB offers to dump core on an assertion,
> > > so you can use that to obtain the backtrace.  We might get lucky, and
> > > something in the backtrace could be the offender.
> >
> > See below backtrace from GDB when assert triggers due to
> > sim_engine_halt() getting called for a breakpoint using SIM_SIGTRAP:
> >
> > ../../binutils-gdb/gdb/inferior.c:279: internal-error: inferior*
> > find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0'
> > failed.
> > A problem internal to GDB has been detected,
> > further debugging may prove unreliable.
> > Quit this debugging session? (y or n)
> > (gdb) bt
> > #0  0x00007ffff66c9bf9 in __GI___poll (fds=0x555555fd36e0, nfds=4,
> > timeout=timeout@entry=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
> > #1  0x00005555556ecf90 in poll (__timeout=-1, __nfds=<optimized out>,
> > __fds=<optimized out>) at
> > /usr/include/x86_64-linux-gnu/bits/poll2.h:46
> > #2  gdb_wait_for_event (block=block@entry=1) at
> > ../../binutils-gdb/gdb/event-loop.c:769
> > #3  0x00005555556ed152 in gdb_do_one_event () at
> > ../../binutils-gdb/gdb/event-loop.c:346
> > #4  0x00005555556ed286 in gdb_do_one_event () at
> > ../../binutils-gdb/gdb/event-loop.c:352
> > #5  0x000055555585d60f in gdb_readline_wrapper (prompt=<optimized
> > out>) at ../../binutils-gdb/gdb/top.c:1078
> > #6  0x000055555588db16 in defaulted_query(const char *, char, typedef
> > __va_list_tag __va_list_tag *) (ctlstr=<optimized out>,
> > defchar=defchar@entry=0 '\000', args=args@entry=0x7fffffffd740)
> >     at ../../binutils-gdb/gdb/utils.c:867
> > #7  0x000055555588de5b in query (ctlstr=<optimized out>) at
> > ../../binutils-gdb/gdb/utils.c:959
> > #8  0x000055555588e3a6 in internal_vproblem(internal_problem *, const
> > char *, int, const char *, typedef __va_list_tag __va_list_tag *) (
> >     problem=problem@entry=0x555555d59880 <internal_error_problem>,
> > file=<optimized out>, line=279, fmt=<optimized out>,
> > ap=ap@entry=0x7fffffffd900) at ../../binutils-gdb/gdb/utils.c:372
> > #9  0x000055555588e4eb in internal_verror (file=<optimized out>,
> > line=<optimized out>, fmt=<optimized out>, ap=ap@entry=0x7fffffffd900)
> > at ../../binutils-gdb/gdb/utils.c:438
> > #10 0x00005555559590df in internal_error
> > (file=file@entry=0x5555559f2cd0 "../../binutils-gdb/gdb/inferior.c",
> > line=line@entry=279, fmt=<optimized out>)
> >     at ../../binutils-gdb/gdbsupport/errors.c:55
> > #11 0x000055555572e72f in find_inferior_pid (targ=<optimized out>,
> > pid=<optimized out>) at ../../binutils-gdb/gdb/inferior.c:279
> > #12 0x0000555555856905 in find_thread_ptid
> > (targ=targ@entry=0x555555d58f60 <gdbsim_ops>, ptid=...) at
> > ../../binutils-gdb/gdb/thread.c:527
> > #13 0x000055555573f42d in handle_inferior_event
> > (ecs=ecs@entry=0x7fffffffdd30) at ../../binutils-gdb/gdb/infrun.c:5062
> > #14 0x000055555574096f in fetch_inferior_event (client_data=<optimized
> > out>) at ../../binutils-gdb/gdb/infrun.c:4002
> > #15 0x00005555556ed19c in check_async_event_handlers () at
> > ../../binutils-gdb/gdb/event-loop.c:1062
> > #16 gdb_do_one_event () at ../../binutils-gdb/gdb/event-loop.c:325
> > #17 0x00005555556ed2be in gdb_do_one_event () at
> > ../../binutils-gdb/gdb/event-loop.c:303
> > #18 start_event_loop () at ../../binutils-gdb/gdb/event-loop.c:370
> > #19 0x0000555555765018 in captured_command_loop () at
> > ../../binutils-gdb/gdb/main.c:360
> > #20 0x0000555555766f75 in captured_main (data=<optimized out>) at
> > ../../binutils-gdb/gdb/main.c:1203
> > #21 gdb_main (args=<optimized out>) at ../../binutils-gdb/gdb/main.c:1218
> > #22 0x00005555555bff1b in main (argc=<optimized out>, argv=<optimized
> > out>) at ../../binutils-gdb/gdb/gdb.c:32
>
> What this shows us is as follows:
>
>   1. The null_ptid (with the 0 value) is present in
>   handle_inferior_event at line 5062 (frame 13).  Now your line
>   numbers don't match up exactly with top of tree, but the closest
>   line looks like this:
>
>   ecs->event_thread = find_thread_ptid (ecs->target, ecs->ptid);
>
>   So we're interested in where ecs->ptid came from.
>
>   2. Follow this back into frame 14, then look back and ecs is
>   populated with this call:
>
>   if (!do_target_wait (minus_one_ptid, ecs, TARGET_WNOHANG))
>      ....
>
>   3. In do_target_wait ecs->ptid is initialised with:
>
>   ecs->ptid = do_target_wait_1 (inf, wait_ptid, &ecs->ws, options);
>
>   4. The return value of do_target_wait_1 comes from this code:
>
>   if (deprecated_target_wait_hook)
>     event_ptid = deprecated_target_wait_hook (ptid, status, options);
>   else
>     event_ptid = target_wait (ptid, status, options);
>
>   return event_ptid;
>
>   And we're almost certainly following the 'target_wait' path unless
>   your target is doing something vert weird, and 'target_wait' just
>   calls the 'wait' method on the current target class, which should
>   get us into 'gdbsim_target::wait'.
>
>   5. In gdbsim_target::wait we always return
>   sim_data->remote_sim_ptid, so the question becomes how is this
>   initialised.

No, I see that in 'gdbsim_target::wait' , inferior_ptid is used as the
return value; its value is set only if ptid is different from
minus_one_ptid.
In my target when I break at 'gdbsim_target::wait', I see the following

(gdb) p minus_one_ptid
$7 = {m_pid = -1, m_lwp = 0, m_tid = 0}
(gdb) p ptid
$8 = {m_pid = -1, m_lwp = 0, m_tid = 0}
(gdb) p inferior_ptid
$9 = {m_pid = 0, m_lwp = 0, m_tid = 0}

hence the return value of 'gdbsim_target::wait' become {m_pid = 0,
m_lwp = 0, m_tid = 0};

When I watch for what modifies the value of inferior_ptid, I find it
getting first set in gdbsim_target::create_inferior() to {m_pid =
42000, m_lwp = 0, m_tid = 42000}; however, before calling
do_target_wait_1(), I see it being set to null by a call to
switch_to_no_thread() with the following backtrace:

Old value = {m_pid = 42000, m_lwp = 0, m_tid = 42000}
New value = {m_pid = 0, m_lwp = 0, m_tid = 0}
switch_to_no_thread () at ../../binutils-gdb/gdb/thread.c:1332
1332      reinit_frame_cache ();
=> 0x0000555555857d3b <switch_to_no_thread()+59>:       e9 a0 6e ea ff
 jmpq   0x5555556febe0 <reinit_frame_cache()>
(gdb) bt
#0  switch_to_no_thread () at ../../binutils-gdb/gdb/thread.c:1332
#1  0x000055555572ea0e in switch_to_inferior_no_thread
(inf=0x555555e4fb00) at ../../binutils-gdb/gdb/inferior.c:623
#2  0x0000555555735b64 in <lambda(inferior*)>::operator()
(inf=0x555555e4fb00, __closure=<synthetic pointer>) at
../../binutils-gdb/gdb/infrun.c:3666
#3  do_target_wait (wait_ptid=..., ecs=0x7fffffffdd30,
options=<optimized out>) at ../../binutils-gdb/gdb/infrun.c:3681
#4  0x00005555557408ca in fetch_inferior_event (client_data=<optimized
out>) at ../../binutils-gdb/gdb/infrun.c:3971
#5  0x00005555556ed19c in check_async_event_handlers () at
../../binutils-gdb/gdb/event-loop.c:1062
#6  gdb_do_one_event () at ../../binutils-gdb/gdb/event-loop.c:325
#7  0x00005555556ed2be in gdb_do_one_event () at
../../binutils-gdb/gdb/event-loop.c:303
#8  start_event_loop () at ../../binutils-gdb/gdb/event-loop.c:370
#9  0x0000555555765018 in captured_command_loop () at
../../binutils-gdb/gdb/main.c:360
#10 0x0000555555766f75 in captured_main (data=<optimized out>) at
../../binutils-gdb/gdb/main.c:1203
#11 gdb_main (args=<optimized out>) at ../../binutils-gdb/gdb/main.c:1218
#12 0x00005555555bff1b in main (argc=<optimized out>, argv=<optimized
out>) at ../../binutils-gdb/gdb/gdb.c:32


From there on, because inferior_ptid is now {m_pid = 0, m_lwp = 0,
m_tid = 0}, it becomes the return value of 'gdbsim_target::wait',
which in turn becomes the return value of target_wait() and
do_target_wait_1() and finally get set in ecs->ptid.


>
>   6. The only initialisation I can see for this variable is this line:
>
>   remote_sim_ptid (next_pid, 0, next_pid)
>
>   So then we ask, what is next_pid?  At this point I would add the
>   following assertion to gdb:
>
>   ## START ##
>
> diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
> index 281232cc4e5..b7ce4672a1a 100644
> --- a/gdb/remote-sim.c
> +++ b/gdb/remote-sim.c
> @@ -86,6 +86,7 @@ struct sim_inferior_data {
>      : gdbsim_desc (desc),
>        remote_sim_ptid (next_pid, 0, next_pid)
>    {
> +    gdb_assert (remote_sim_ptid != null_ptid);
>      ++next_pid;
>    }

I tried the above change; the gdb_assert never triggers.

>
>   ## END ##
>
>   7. My guess is that in your port this will fail, but for other ports
>   this doesn't fail.  For other ports next_pid is initialised to 42000
>   in gdbsim_target_open - is it possible that this initialisation has
>   been removed in your tree somehow?

I am using commit d7f734bc7e9e5fb6c33b433973b57e1eed3a7e9f (tag: binutils-2_34)

next_pid is set at remote-sim.c:758 .

>
> Thanks,
> Andrew

Re: sim_engine_halt() broken in binutils 2.34

[Andrew Burgess]

* William Tambe <tambewilliam@gmail.com> [2020-02-03 21:59:39 -0500]:
> 
> I am using commit d7f734bc7e9e5fb6c33b433973b57e1eed3a7e9f (tag: binutils-2_34)

You need to include:

  commit cf1d9e092f871df3b34a58cfcde915c689ac9067
  Date:   Sun Jan 19 19:47:49 2020 -0500

      sim: don't rely on inferior_ptid in gdbsim_target::wait


Thanks,
Andrew

Re: sim_engine_halt() broken in binutils 2.34

[William Tambe]

On Tue, Feb 4, 2020 at 5:32 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>
> * William Tambe <tambewilliam@gmail.com> [2020-02-03 21:59:39 -0500]:
> >
> > I am using commit d7f734bc7e9e5fb6c33b433973b57e1eed3a7e9f (tag: binutils-2_34)
>
> You need to include:
>
>   commit cf1d9e092f871df3b34a58cfcde915c689ac9067
>   Date:   Sun Jan 19 19:47:49 2020 -0500
>
>       sim: don't rely on inferior_ptid in gdbsim_target::wait

Thank you very much; it fixed the issue after cherry-picking the above commit.

>
>
> Thanks,
> Andrew