[RFA] detect inferior exit in call_function_by_hand

[RFA] detect inferior exit in call_function_by_hand

[Doug Evans]

I found this while playing with the archer exception-rewind branch.
The message currently printed when the inferior exits during
call_function_by_hand is misleading.
[On the branch one currently gets a segv,
stopped_by_random_signal/stop_stack_dummy do not fully describe
all the possible states of the inferior.]

foo.cc:
#include <stdlib.h>

char
foo ()
{
  exit (0);
}

int
main ()
{
  return 0;
}

---

(gdb) start
(gdb) call foo ()

Program exited normally.
The program being debugged stopped while in a function called from GDB.
When the function (foo()) is done executing, GDB will silently
stop (instead of continuing to evaluate the expression containing
the function call).
(gdb) 

With the patch gdb prints:

(gdb) call foo ()

Program exited normally.
The program being debugged exited while in a function called from GDB.
(gdb) 

Ok to check in?

2008-11-11  Doug Evans  <dje@google.com>

	* infcall.c (call_function_by_hand): Handle inferior exit.

	* gdb.base/callexit.exp: New file.
	* gdb.base/callexit.c: New file.

diff --git a/gdb/infcall.c b/gdb/infcall.c
index ece57ba..6781989 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -746,6 +746,18 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
     discard_cleanups (old_cleanups);
   }
 
+  if (! target_has_execution)
+    {
+      /* If we try to restore the inferior status (via the cleanup),
+	 we'll crash as the inferior is no longer running.  */
+      discard_cleanups (inf_status_cleanup);
+      discard_inferior_status (inf_status);
+      /* FIXME: Insert a bunch of wrap_here; name can be very long if it's
+	 a C++ name with arguments and stuff.  */
+      error (_("\
+The program being debugged exited while in a function called from GDB."));
+    }
+
   if (stopped_by_random_signal || !stop_stack_dummy)
     {
       /* Find the name of the function we're about to complain about.  */
diff --git a/gdb/infcall.c b/gdb/infcall.c
index ece57ba..6781989 100644
--- a/gdb/testsuite/gdb.base/callexit.exp	2007-10-18 09:27:25.000000000 -0700
+++ b/gdb/testsuite/gdb.base/callexit.exp	2008-11-11 13:45:24.000000000 -0800
@@ -0,0 +1,90 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile "callexit"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested callfuncs.exp
+     return -1
+}
+
+# Some targets can't do function calls, so don't even bother with this
+# test.
+if [target_info exists gdb,cannot_call_functions] {
+    setup_xfail "*-*-*" 2416
+    fail "This target can not call functions"
+    continue
+}
+
+# Set the current language to C.  This counts as a test.  If it
+# fails, then we skip the other tests.
+
+proc set_lang_c {} {
+    global gdb_prompt
+
+    send_gdb "set language c\n"
+    gdb_expect {
+	-re ".*$gdb_prompt $" {}
+	timeout { fail "set language c (timeout)" ; return 0; }
+    }
+
+    send_gdb "show language\n"
+    gdb_expect {
+	-re ".* source language is \"c\".*$gdb_prompt $" {
+	    pass "set language to \"c\""
+	    return 1
+	}
+	-re ".*$gdb_prompt $" {
+	    fail "setting language to \"c\""
+	    return 0
+	}
+	timeout {
+	    fail "can't show language (timeout)"
+	    return 0
+	}
+    }
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if { ![set_lang_c] } {
+    gdb_suppress_tests;
+} else {
+    if { ![runto_main] } {
+	gdb_suppress_tests;
+    }
+}
+
+# Call function (causing the program to exit), and see if gdb handles
+# it properly.
+gdb_test "call callexit()" \
+	"The program being debugged exited.*" \
+	"inferior function call terminated program"
+
+return 0
diff --git a/gdb/infcall.c b/gdb/infcall.c
index ece57ba..6781989 100644
--- a/gdb/testsuite/gdb.base/callexit.c	2007-10-18 09:27:25.000000000 -0700
+++ b/gdb/testsuite/gdb.base/callexit.c	2008-11-11 13:37:56.000000000 -0800
@@ -0,0 +1,33 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2008 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Support program for testing gdb's ability to handle an
+   inferior function call that terminates the program.  */
+
+#include <stdlib.h>
+
+void
+callexit ()
+{
+  exit (0);
+}
+
+int
+main ()
+{
+  return 0;
+}

Re: [RFA] detect inferior exit in call_function_by_hand

[Pedro Alves]

On Tuesday 11 November 2008 22:50:20, Doug Evans wrote:
> I found this while playing with the archer exception-rewind branch.
> The message currently printed when the inferior exits during
> call_function_by_hand is misleading.
> [On the branch one currently gets a segv,
> stopped_by_random_signal/stop_stack_dummy do not fully describe
> all the possible states of the inferior.]

Looks OK to me, apart from the tiny issues below.

>  
> +  if (! target_has_execution)
> +    {
> +      /* If we try to restore the inferior status (via the cleanup),
> +	 we'll crash as the inferior is no longer running.  */
> +      discard_cleanups (inf_status_cleanup);
> +      discard_inferior_status (inf_status);
> +      /* FIXME: Insert a bunch of wrap_here; name can be very long if it's
> +	 a C++ name with arguments and stuff.  */

Remove this copy&pasted FIXME note, it doesn't make sense for
this case (you're not printing a function name).


> +      error (_("\
> +The program being debugged exited while in a function called from GDB."));
> +    }
> +
>    if (stopped_by_random_signal || !stop_stack_dummy)
>      {
>        /* Find the name of the function we're about to complain about.  */


> +
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> +     untested callfuncs.exp
                    ^^^^ ECOPYPASTE (callexit.exp)

> +     return -1


> +# Set the current language to C.  This counts as a test.  If it
> +# fails, then we skip the other tests.
> +

I don't know why all the infcalls tests do this --- they
all seem to do it.  Do you/anyone know(s) why?

> +proc set_lang_c {} {
> +    global gdb_prompt
> +
> +    send_gdb "set language c\n"
> +    gdb_expect {
> +	-re ".*$gdb_prompt $" {}
> +	timeout { fail "set language c (timeout)" ; return 0; }
> +    }
> +
> +    send_gdb "show language\n"
> +    gdb_expect {
> +	-re ".* source language is \"c\".*$gdb_prompt $" {
> +	    pass "set language to \"c\""
> +	    return 1
> +	}
> +	-re ".*$gdb_prompt $" {
> +	    fail "setting language to \"c\""
> +	    return 0
> +	}
> +	timeout {
> +	    fail "can't show language (timeout)"
> +	    return 0
> +	}
> +    }
> +}
> +
> +# Start with a fresh gdb.
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_load ${binfile}
> +

> +if { ![set_lang_c] } {
> +    gdb_suppress_tests;
> +} else {
> +    if { ![runto_main] } {
> +	gdb_suppress_tests;
> +    }
> +}
> +

-- 
Pedro Alves

Re: [RFA] detect inferior exit in call_function_by_hand

[Joel Brobecker]

> +    send_gdb "set language c\n"
> +    gdb_expect {
> +	-re ".*$gdb_prompt $" {}
> +	timeout { fail "set language c (timeout)" ; return 0; }
> +    }
> +
> +    send_gdb "show language\n"
> +    gdb_expect {
> +	-re ".* source language is \"c\".*$gdb_prompt $" {
> +	    pass "set language to \"c\""
> +	    return 1
> +	}
> +	-re ".*$gdb_prompt $" {
> +	    fail "setting language to \"c\""
> +	    return 0
> +	}
> +	timeout {
> +	    fail "can't show language (timeout)"
> +	    return 0
> +	}
> +    }

Generally speaking, we try to avoid send_gdb/gdb_expect, and use
gdb_test_multiple. I realize that you probably inherited this
through copy/paste... In any case, I don't think it will necessary
to rewrite the above, because, Like Pedro, I don't think you need
the set lang c. The only time when I had to explicitly set the language
is after attaching to a program - you never know where you might end up,
and sometimes you find your way into asm frames, for instance. I propose
you remove this part entirely. That should simplify your testcase and
it's easy enough to put it back if we discover a case where it's
really needed.

> +if { ![set_lang_c] } {
> +    gdb_suppress_tests;
> +} else {
> +    if { ![runto_main] } {
> +	gdb_suppress_tests;
> +    }

We don't use gdb_suppress_tests anymore. As far as I know, the usual
way of doing things is to log a FAIL and then return. For instance:

    if ![runto_main] then {
        fail "Can't run to main"
        return 0
    }

-- 
Joel

Re: [RFA] detect inferior exit in call_function_by_hand

[Doug Evans]

On Thu, Nov 13, 2008 at 4:53 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> +    send_gdb "set language c\n"
>> +    gdb_expect {
>> +     -re ".*$gdb_prompt $" {}
>> +     timeout { fail "set language c (timeout)" ; return 0; }
>> +    }
>> +
>> +    send_gdb "show language\n"
>> +    gdb_expect {
>> +     -re ".* source language is \"c\".*$gdb_prompt $" {
>> +         pass "set language to \"c\""
>> +         return 1
>> +     }
>> +     -re ".*$gdb_prompt $" {
>> +         fail "setting language to \"c\""
>> +         return 0
>> +     }
>> +     timeout {
>> +         fail "can't show language (timeout)"
>> +         return 0
>> +     }
>> +    }
>
> Generally speaking, we try to avoid send_gdb/gdb_expect, and use
> gdb_test_multiple. I realize that you probably inherited this
> through copy/paste... In any case, I don't think it will necessary
> to rewrite the above, because, Like Pedro, I don't think you need
> the set lang c. The only time when I had to explicitly set the language
> is after attaching to a program - you never know where you might end up,
> and sometimes you find your way into asm frames, for instance. I propose
> you remove this part entirely. That should simplify your testcase and
> it's easy enough to put it back if we discover a case where it's
> really needed.
>
>> +if { ![set_lang_c] } {
>> +    gdb_suppress_tests;
>> +} else {
>> +    if { ![runto_main] } {
>> +     gdb_suppress_tests;
>> +    }
>
> We don't use gdb_suppress_tests anymore. As far as I know, the usual
> way of doing things is to log a FAIL and then return. For instance:
>
>    if ![runto_main] then {
>        fail "Can't run to main"
>        return 0
>    }

In past lives, I've often found it useful to never introduce fails
that don't also have passes - it makes regression reports cleaner:
otherwise tests that failed the previous time but now pass disappear
and the test count has gone down leading one to wonder if there's a
testsuite issue.  In this case it doesn't matter as the gdb testsuite
is all over the map so I'm happy to just go with the flow.