Re: Can we get rid of go32_stop? (and its normal_stop call?)

[Pedro Alves <pedro at codesourcery dot com>]

On Friday 01 May 2009 16:47:11, Eli Zaretskii wrote:
> > From: Pedro Alves <pedro@codesourcery.com>
> > Date: Fri, 1 May 2009 15:14:27 +0100
> > Cc: Eli Zaretskii <eliz@gnu.org>
> > 
> > - go32_stop is registered as target_stop callback, but,
> > target_stop is only useful if the target supports
> > asynchronous execution control --- djgpp doesn't support it.
> 
> You are probably right about that.  But what will happen if to_stop is
> not registered and target_stop is somehow called nonetheless?

Nothing at all.  The default implementation of to_stop
is target_ignore.

> >   if (prog_has_started)
> >     {
> >       go32_stop (inferior_ptid);
> >       go32_kill_inferior (ops);
> >     }
> > 
> > AFAICS, this is doing cleanup from a previous run.  It seems
> > weird to call go32_kill_inferior here, since go32_create_inferior
> > will only ever be called if the previous inferior is gone already,
> > because the "run" command always kills the previous process.  If the
> > previous process exited by itself, then go32_mourn_inferior is called,
> > which itself calls go32_kill_inferior.
> 

> I'm less sure about this part.  Let me tell you something that I'm not
> sure you know about how DJGPP debugging works; apologies if I'm
> preaching to the choir.

Not at all.  I knew that there are no 2 separate processes involved, but,
your description is very enlightening.  IMO, it would be nice to
paste your description below at the top of go32-nat.c.

> When the DJGPP port of GDB is debugging a DJGPP program natively,
> there are no 2 separate processes, the debuggee and GDB itself, as on
> other systems.  (This is DOS, where there can only be one active
> process at any given time, remember?)  Instead, GDB and the debuggee
> live in the same process.  What go32_create_inferior does (in the
> functions it calls from the DJGPP debug support library libdbg.a) is
> load the debuggee's executable file, set it up for execution as the
> stub loader (a short real-mode program prepended to each DJGPP
> executable) normally would, and do a lot of preparations for swapping
> between GDB's and debuggee's internal state, primarily the wrt
> exception handlers.  Then running the debuggee simply means longjmp
> into it where its PC is and let it run until it stops for some reason.
> When it stops, GDB catches the exception that stopped it and longjmp's
> back into its own code.  All the possible exit points of the debuggee
> are watched; for example, the normal exit point is recognized because
> a DOS program issues a special system call to exit.  If one of those
> exit points is hit, we mourn the inferior and clean up after it.
> Cleaning up is very important, even if the process exits normally,
> because otherwise we might leave behind traces of previous execution,
> and in several cases GDB itself might be left hosed, because all the
> exception handlers were not restored.
> 
> So far so good.  But the plot thickens if abnormal events happen, such
> as an exception inside the debuggee or just a Ctrl-C pressed by the
> user at a wrong time.  We might get back to GDB before we had a chance
> to clean up after the debuggee.  

I see.  To be clear, in this case, is the debuggee considered
dead and gone?  No further debugging possible?

> In this situation, can we still be 
> sure that go32_kill_inferior or go32_mourn_inferior would be called?

I think so.  Currently, this "just-in-case" cleanup, is done on
go32_create_inferior, so only on the next "run".  If for some reason, some
abnormal event happens, and the user is dropped into GDB's shell, the
next "run" will clean up.  But, notice one thing --- the go32_ops target
will still be pushed on the stack, and so the current target still is
considered to have execution (target_has_execution is true, and
inferior_ptid will still point at  SOME_PID).  So, when the user does "run",
GDB will ask if the user wants to kill the current program, and
will refuse to go ahead if the user says "no".  If the user says
"yes", then, target_kill is called to kill the current program
(which ends up in go32_kill_inferior), all before reaching
go32_create_inferior.

 This is run_command->kill_if_already_running->target_kill.

So, if we do the said cleaning up on go32_mourn_inferior, and
make sure that go32_mourn_inferior is called from go32_kill_inferior,
I think you'll get the same ammount of "cleanup after the fact" as we
have currently.  In fact, you'll have more --- if something goes
wild, the user will be able to do full cleaning up with the "kill"
command.  Currently, "kill" doesn't call `cleanup_client', after
my changes, it will --- I'm assuming that to be a good thing.

> If not, then making sure they are called and the necessary cleanup is
> done would not hurt anything.
> 
> > AFAICS, go32_kill_inferior is always called either when you kill the
> > program or when it exits cleanly
> 
> That may be so as far as the normal flow of control is concerned.  But
> given that there could be a lot of longjmp-ing around, I'm less sure.
> Maybe I'm paranoiac.

:-)  I think that with my proposed change, we get the same ammount
of paranoia (even more), but, this needs to be at least minimally
tested, of course.

> So maybe we should make these changes, but leave alone the call to
> go32_kill_inferior inside go32_create_inferior, conditioned on the
> prog_has_started flag, just in case.

I don't think that's required, for the reasons described above.

BTW, have you ever had any success running djgpp from inside an
emulator on linux, say, 'dosemu'?

-- 
Pedro Alves