Re: [djgpp/commit] Fix go32_pid_to_str and go32_thread_alive

Pedro Alves
2009-05-01T13:32:42Z

On Friday 01 May 2009 13:57:40, Eli Zaretskii wrote:
> > From: Pedro Alves <pedro@codesourcery.com>
> > Date: Fri, 1 May 2009 11:18:51 +0100
> >
> > On Friday 01 May 2009 09:17:15, Eli Zaretskii wrote:
> >
> > > 2009-05-01 Eli Zaretskii <eliz@gnu.org>
> > >
> > > * go32-nat.c (go32_pid_to_str): Call normal_pid_to_str instead of
> > > printing a bogus "Thread ".
> >
> > I thought that the inferior's PID on DJGPP is always the
> > fake SOMEPID (42)
>
> True.
>
> > an internal implementation detail, that we'd
> > never want to show to the user, but, normal_pid_to_str will
> > print "process 42" here. Isn't that bogus as well?
>
> It is, to some degree. But IMO talking about threads in an
> environment that doesn't support threads is a greater lie, so
> to say ;-)

Okay, but I disagree with the lying :-) There's no kernel
multi-thread support, or a thread library, but there's still
a thread of control here.

> I wanted the output to be more like when one debugs
> a single-threaded program on Unix, because when I first saw
^^^^^^^^^^^^^^^

see, "single-threaded" implies a single (== 1) thread. :-) From
GDB's perpective there is a thread of control here. There's
no thread support library, though, but that's a bit different.
GDB doesn't care much.

> that "Thread " displayed by "set debug infrun 1", I
> thought there's some bug somewhere whereby GDB thinks it's
> debugging a multithreaded program.
>
> DJGPP users are already used to see 42 popping here and there; e.g.,
> that's what getuid and getgid return for the only user and group they
> support.

Ah, I see. Makes sense then.

> I briefly considered a possibility to add a function to more
> faithfully fake the PID of the inferior (DJGPP's version of getpid
> simply returns the current BIOS clock tick counter, so I could fetch
> that from GDB), but eventually I decided it wasn't worth the hassle.

Yeah.

> > > (go32_thread_alive): Don't return 1 for null_ptid.
> >
> > Interesting. This may be masking some other problem. How
> > did you get here with inferior_ptid == null_ptid?
>
> I didn't. It just seemed unclean to me to return 1 for null_ptid,
> when just a little ways above, in go32_stop, we assign it when we
> delete the single alive thread.
>
> > AFAICS, when the inferior exits or is killed, the go32_ops target is
> > unpushed.
>
> I have no doubt that you are right, but at some future point this
> method could potentially be called in some other context.
>

If in non-stop mode, and debugging more than one process
simultaneously (multiprocess mode), you may get here without any
thread selected, thus inferior_ptid == null_ptid, but, there may
the live threads in the thread list. This function should never
rely or care about what's in inferior_ptid, but instead only consider
the ptid passed in as argument. But, DJGPP doesn't support
non-stop or multiprocess, and single-process + non-stop doesn't
make much sense for a target that doesn't support multi-threading,
of course.

> If you think my change may mask other problems, now or in the future,
> I'm okay with adding some appropriate gdb_assert here.

I can't predict the future, so I always prefer not putting asserts like
that, since if the design direction changes, we may trip on an assert for no
good reason. I don't think djgpp will even gain multi-process + non-stop
modes, so, it doesn't matter much either way. I prefered the previous
version as it was simpler in that it didn't assume anything at all ---
the current version added a bit of dead code, that makes the reader
stop and consider why it is done the way it is --- , but it's fine
with me either way, and it's really your call.

> Thanks for the feedback.

Thanks for explaining!

--
Pedro Alves