Re: [djgpp/commit] Fix go32_pid_to_str and go32_thread_alive

[Pedro Alves <pedro at codesourcery dot com>]

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 <main>".
> > 
> > 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 <main>" 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