[RFC] - Unexpected gdb output after inferior exits in simulator run

[Fred Fish <fnf at public dot ninemoons dot com>]

What is the currently accepted way to test inside gdb if an inferior
has exited, that works with the built in simulators as well as
natively executed inferiors?

This issue came up because I decided to take a look at why the gdb
"bang.exp" test was failing for the mipsisa32-elf toolchain:

  FAIL: gdb.base/bang.exp: run program

The gdb.log file has output like:

  (gdb) run
  Starting program: /links3/build/sourceware/gdb/T-mipsisa32-elf/gdb/testsuite/gdb.base/bang!
  0

  Program exited normally.
  Current language:  auto; currently asm	<<<<<<<<<<<======== NOTE THIS LINE
  (gdb) FAIL: gdb.base/bang.exp: run program

The problem is that the test is not prepared to handle the extra line
after the normal exit message (Current language: ...).  Note the
pattern to match ("Program exited normally.") does not end in ".*" in
bang.exp:

  # Verify that we can run the program and that it terminates normally.
  gdb_test "run" \
         ".*Program exited normally\." \
         "run program"

Now of course the test could be tweaked to ignore the extra output,
but I was curious about why gdb would want to tell me about the
current language after a program has exited, and this is where things
get interesting.

Just before gdb's proceed() function returns, it makes two calls:

    wait_for_inferior ();
    normal_stop ();

For the native x86 case, wait_for_inferior ends up calling
unpush_target via child_mourn_inferior:

   static void
   child_mourn_inferior (void)
   {
     unpush_target (&child_ops);
     generic_mourn_inferior ();
   }

which changes the current target from "child", for which
target_has_stack is true to "exec", for which target_has_stack is
false.  Then, later when proceed() calls normal_stop(), there is a
section of code in normal_stop() like:

  if (!target_has_stack)
    {
      goto done;
    }

  /* Select innermost stack frame - i.e., current frame is frame 0,
     and current location is based on that.
     Don't do this on return from a stack dummy routine,
     or if the program has exited. */

  if (!stop_stack_dummy)
    {
      select_frame (get_current_frame ());

Because target_has_stack for the x86 case is false, normal_stop()
jumps to "done", which branches around the call to select_frame().

However for the mips toolchain case when run using the simulator,
wait_for_inferior eventually calls a different mourn_inferior:

  static void
  gdbsim_mourn_inferior (void)
  {
    if (sr_get_debug ())
      printf_filtered ("gdbsim_mourn_inferior:\n");
    remove_breakpoints ();
    generic_mourn_inferior ();
  }

which does not affect the current target, which remains "sim", and for
which target_has_stack is always true.

So the call to select_frame() gets made for the mips case, and
select_frame eventually calls set_language(), which changes the
current language from "c" to "asm", which eventually triggers the
message about the current language being changed.

WHEW!

So how to fix this?  I would seem that one way, implied by the comment
above ("or if the program has exited") would be to fix the first test
so that the jump to "done" is made for cases where a simulated program
has exited, or to change the second test, by testing more than just
stop_stack_dummy, and avoid the select_frame() call.

To decide if an inferior actually has exited, I decided to try testing
the inferior_pid, like is done elsewhere in some places in gdb:

  Index: infrun.c
  ===================================================================
  RCS file: /cvs/src/src/gdb/infrun.c,v
  retrieving revision 1.122
  diff -u -r1.122 infrun.c
  --- infrun.c	25 Nov 2003 16:01:36 -0000	1.122
  +++ infrun.c	5 Dec 2003 00:00:38 -0000
  @@ -3161,7 +3161,7 @@
       catch_errors (hook_stop_stub, stop_command,
   		  "Error while running hook_stop:\n", RETURN_MASK_ALL);
   
  -  if (!target_has_stack)
  +  if (!target_has_stack || PIDGET (inferior_ptid) == 0)
       {
   
         goto done;
  
This did fix the problem and didn't cause any regressions in a native
x86 toolchain.  However it did cause a bunch of differences in the
"mi" tests (some previous FAILS became PASS and vice versa), though
the net effect was positive:

  < # of expected passes          7916
  < # of unexpected failures      443
  ---
  > # of expected passes          8074
  > # of unexpected failures      284

I then reverted that change and tried this one, for the second test:

  Index: infrun.c
  ===================================================================
  RCS file: /cvs/src/src/gdb/infrun.c,v
  retrieving revision 1.122
  diff -u -r1.122 infrun.c
  --- infrun.c	25 Nov 2003 16:01:36 -0000	1.122
  +++ infrun.c	5 Dec 2003 00:03:46 -0000
  @@ -3172,7 +3172,7 @@
        Don't do this on return from a stack dummy routine,
        or if the program has exited. */
   
  -  if (!stop_stack_dummy)
  +  if (!stop_stack_dummy && PIDGET (inferior_ptid) != 0)
       {
         select_frame (get_current_frame ());
   
As before, this didn't cause any x86 native regressions, but diff
affect the pattern of PASS/FAILS in the "mi" tests.  But relative to
the unpatched baseline, the net effect was still positive, though
different:

  < # of expected passes          7916
  < # of unexpected failures      443
  ---
  > # of expected passes          8080
  > # of unexpected failures      278

So obviously there is something in the "mi" tests that is also
sensitive to unexpected output from gdb after the inferior has exited.

I'm not particularly happy about adding another PIDGET test, and
perhaps checking for 0 isn't the right thing either.  On the other
hand, none of the flags like target_has_stack or target_has_execution
seems to be changed other than by pushing/popping target vectors and
in the case of the simulator, that never happens.

Comments or suggestions appreciated.

-Fred