Re: [RFA] fix popping of dummy frame if inferior gets signal with unwindonsignal

[Michael Snyder <msnyder@vmware.com>]

I don't understand why calling dummy_frame_pop directly works,
but calling frame_pop doesn't work.  Doesn't frame_pop call
dummy_frame_pop?  If not, what prevents it?

Doug Evans wrote:
> Hi.  This fixes a bug where a signal in an inferior function call with
> unwindonsignal set doesn't remove the dummy frame from dummy_frame_stack.
> A testcase is included to exercise the bug.
> 
> It also adds a comment to clear up something that was a bit confusing
> at first.  The lone frame_pop isn't enough to properly restore program
> state, but things still work because registers (and thus stack) get
> restored from inf_status.
> 
> Tested on i386-linux.
> 
> Ok to check in?
> 
> 2008-11-15  Doug Evans  <dje@google.com>
> 
> 	* infcall.c (call_function_by_hand): Properly pop the dummy frame
> 	if the inferior gets a signal and unwindonsignal is set.
> 
> 	* gdb.base/unwindonsignal.c: New file.
> 	* gdb.base/unwindonsignal.exp: New file.
> 
> Index: infcall.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infcall.c,v
> retrieving revision 1.105
> diff -u -p -u -p -r1.105 infcall.c
> --- infcall.c	12 Nov 2008 00:39:28 -0000	1.105
> +++ infcall.c	15 Nov 2008 22:08:54 -0000
> @@ -357,7 +358,10 @@ call_function_by_hand (struct value *fun
>    /* Save the caller's registers so that they can be restored once the
>       callee returns.  To allow nested calls the registers are (further
>       down) pushed onto a dummy frame stack.  Include a cleanup (which
> -     is tossed once the regcache has been pushed).  */
> +     is tossed once the regcache has been pushed).
> +     NOTE: If the inferior gets a signal and unwindonsignal is set,
> +     then this regcache is not used to restore target regs.  Instead
> +     the regcache in inf_status is used.  */
>    caller_regcache = frame_save_as_regcache (frame);
>    caller_regcache_cleanup = make_cleanup_regcache_xfree (caller_regcache);
>  
> @@ -746,8 +749,12 @@ The program being debugged exited while 
>  	      /* The user wants the context restored. */
>  
>  	      /* We must get back to the frame we were before the
> -		 dummy call. */
> -	      frame_pop (get_current_frame ());
> +		 dummy call.
> +		 NOTE: This discards the regcache recorded in the dummy frame
> +		 but that's ok, the registers will get restored from the
> +		 regcache in inf_status (which gets called by a cleanup).  */
> +	      dummy_frame_pop (dummy_id);
> +	      reinit_frame_cache ();
>  
>  	      /* FIXME: Insert a bunch of wrap_here; name can be very
>  		 long if it's a C++ name with arguments and stuff.  */
> Index: testsuite/gdb.base/unwindonsignal.c
> ===================================================================
> RCS file: testsuite/gdb.base/unwindonsignal.c
> diff -N testsuite/gdb.base/unwindonsignal.c
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.base/unwindonsignal.c	16 Nov 2008 01:11:22 -0000
> @@ -0,0 +1,49 @@
> +/* 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 handling of unwindonsignal.  */
> +
> +#include <stdio.h>
> +#include <signal.h>
> +#include <unistd.h>
> +
> +/* This function is handcalled from unwindonsignal.exp.  */
> +
> +void
> +gen_signal ()
> +{
> +  /* According to sigall.exp, SIGABRT is always supported,
> +     so try that first.  */
> +#ifdef SIGABRT
> +  kill (getpid (), SIGABRT);
> +#endif
> +#ifdef SIGSEGV
> +  kill (getpid (), SIGSEGV);
> +#endif
> +  /* If we get here we couldn't generate a signal, tell dejagnu.  */
> +  printf ("no signal\n");
> +}
> +
> +int
> +main ()
> +{
> +#ifdef usestubs
> +  set_debug_traps ();
> +  breakpoint ();
> +#endif
> +  return 0;
> +}
> Index: testsuite/gdb.base/unwindonsignal.exp
> ===================================================================
> RCS file: testsuite/gdb.base/unwindonsignal.exp
> diff -N testsuite/gdb.base/unwindonsignal.exp
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.base/unwindonsignal.exp	16 Nov 2008 01:11:22 -0000
> @@ -0,0 +1,94 @@
> +# 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
> +}
> +
> +if [target_info exists gdb,noinferiorio] {
> +    verbose "Skipping unwindonsignal.exp because of no fileio capabilities."
> +    continue
> +}
> +
> +set prms_id 0
> +set bug_id 0
> +
> +set testfile "unwindonsignal"
> +set srcfile ${testfile}.c
> +set binfile ${objdir}/${subdir}/${testfile}
> +
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> +     untested unwindonsignal.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
> +}
> +
> +# Start with a fresh gdb.
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_load ${binfile}
> +
> +if { ![runto_main] } {
> +    fail "Can't run to main"
> +    return 0
> +}
> +
> +# Turn on unwindonsignal.
> +gdb_test "set unwindonsignal on" \
> +	"" \
> +	"setting unwindonsignal"
> +gdb_test "show unwindonsignal" \
> +	"Unwinding of stack .* is on." \
> +	"showing unwindonsignal"
> +
> +# Call function (causing the program to get a signal), and see if gdb handles
> +# it properly.
> +gdb_test_multiple "call gen_signal ()" \
> +	"unwindonsignal, inferior function call signaled" {
> +    -re "\[\r\n\]*no signal\[\r\n\]+$gdb_prompt $" {
> +	unsupported "unwindonsignal, inferior function call signaled"
> +	return 0
> +    }
> +    -re "\[\r\n\]*The program being debugged was signaled.*\[\r\n\]+$gdb_prompt $" {
> +	pass "unwindonsignal, inferior function call signaled"
> +    }
> +}
> +
> +# Verify the stack got unwound.
> +gdb_test "bt" \
> +	"#0 *main \\(.*\\) at .*" \
> +	"unwindonsignal, stack unwound"
> +
> +# Verify the dummy frame got removed from dummy_frame_stack.
> +gdb_test_multiple "maint print dummy-frames" \
> +	"unwindonsignal, dummy frame removed" {
> +    -re "\[\r\n\]*.*stack=.*code=.*\[\r\n\]+$gdb_prompt $" {
> +	fail "unwindonsignal, dummy frame removed"
> +    }
> +    -re "\[\r\n\]+$gdb_prompt $" {
> +	pass "unwindonsignal, dummy frame removed"
> +    }
> +}
> +
> +return 0