This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA] dummy frame handling cleanup, plus inferior fun call signal handling improvement


Hi.  This patch was in progress when I wrote
http://sourceware.org/ml/gdb-patches/2008-11/msg00391.html
This patch subsumes that one.

There are two things this patch does:
1) properly pop dummy frames more often
2) make the inferior resumable after an inferior function call gets a
   signal without having to remember to do "signal 0"

1) properly pop dummy frames more often

Ulrich asked:
> I agree that it would be better if call_function_by_hand were to call
> dummy_frame_pop in this case.  However, why exactly is this a bug?

It's a bug because it's confusing to not pop the frame in the places
where one is able to.  Plus it's, well, unclean.
I recognize that the dummy frame stack can't be precisely managed because
the user can do things like:

set $orig_pc = $pc
set $orig_sp = $sp
break my_fun
call my_fun()
set $pc = $orig_pc
set $sp = $orig_sp
continue

[This doesn't always work, but you get the idea.]
At the "continue", the inferior function call no longer exists and
gdb's mechanisms for removing the dummy frame from dummy_frame_stack
will never trigger (until the program is resumed and cleanup_dummy_frames
is called).  But we can still keep things clean
and unconfusing for the common case.

2) make the inferior resumable after an inferior function call gets a
   signal without having to remember to do "signal 0"

If an inferior function call gets a signal (say SIGSEGV or SIGABRT),
one should be able to easily resume program execution after popping the stack.
It works today, but after manually popping the stack (e.g. with
"frame <dummy> ; return" where <dummy> is the dummy stack frame's number)
one has to remember to resume the program with "signal 0".
This is because stop_signal doesn't get restored.
Maybe there's a reason it shouldn't be, or maybe should be made an option,
but the current behaviour seems user-unfriendly for the normal case.

Restoring stop_signal also has the benefit that if an inferior function
call is made after getting a signal, and the inferior function call
doesn't complete (e.g. has a breakpoint and doesn't complete right away),
the user can resume the program (after popping the inf fun call off the
stack) with "continue" without having to remember what the signal was
and remember to use "signal N".

[BTW, IWBN if there was a command that did the equivalent of
"frame <dummy> ; return", named say "abandon", so that one didn't have
to go to the trouble of finding the dummy frame's stack number.
"abandon" would have the additional benefit that if the stack
was corrupted and one couldn't find the dummy frame, it would still
work since everything it needs is in dummy_frame_stack - it doesn't need
to examine the inferior's stack, except maybe for some sanity checking.
Continuing the inferior may still not be possible, but it does give the
user a more straightforward way to abandon an inferior function call
than exists today.]

The bulk of the patch goes into making (2) work in a clean way.
Right now the inferior state that can be restored is a collection of
inferior_status, regcache, and misc. things like stop_pc (see the
assignment of stop_pc in normal_stop() when stop_stack_dummy).
The patch organizes the state into two pieces: inferior program state
(registers, stop_pc, stop_signal) and gdb session state
(the rest of inferior_status).
Program state is recorded on the dummy frame stack and is always
restored, either when the inferior function call completes or
when the stack frame is manually popped.
inferior_status contains the things that only get restored
if either the inferior function call successfully completes or
it gets a signal and unwindonsignal is set.

P.S. I've removed one copy of the regcache.  Hopefully I've structured
things in a way that doesn't lose.

NOTES:
- this adds the unwindonsignal.exp testcase from before, plus a new
  testcase that verifies one can resume the inferior after abandoning
  an inferior function call that gets a signal

It's a big patch so presumably there are issues with it.
Comments?

[tested on i386-linux and x86_64-linux]

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

	* dummy-frame.c (dummy_frame): Replace regcache member with
	caller_state.
	(dummy_frame_push): Replace caller_regcache arg with caller_state.
	Return pointer to created dummy frame.  All callers updated.
	(remove_dummy_frame,do_dummy_frame_cleanup,pop_dummy_frame,
	assert_dummy_present,pop_dummy_frame_below,lookup_dummy_id,
	dummy_frame_discard,do_pop_dummy_frame_cleanup,
	make_cleanup_pop_dummy_frame): New functions.
	(dummy_frame_pop): Rewrite.  Verify requested frame is in the
	dummy frame stack.  Restore program state.  Discard dummy frames below
	the one being deleted.
	(dummy_frame_sniffer): Update.
	* dummy-frame.h (dummy_frame_push): Update prototype.
	(dummy_frame_discard,make_cleanup_pop_dummy_frame): Declare.
	* frame.c (frame_pop): dummy_frame_pop now does all the work for
	DUMMY_FRAMEs.
	* infcall.c (call_function_by_hand): Replace caller_regcache,
	caller_regcache_cleanup with caller_state, caller_state_cleanup.
	New locals dummy_frame, dummy_frame_cleanup.
	Ensure dummy frame is popped/discarded for all possible exit paths.
	* inferior.h (inferior_program_state): Declare (opaque type).
	(save_inferior_program_state,restore_inferior_program_state,
	make_cleanup_restore_inferior_program_state,
	discard_inferior_program_state,
	get_inferior_program_state_regcache): Declare.
	(save_inferior_status): Update prototype.
	* infrun.c: #include "dummy-frame.h"
	(normal_stop): When stopped for the completion of an inferior function
	call, verify the expected stack frame kind and use dummy_frame_pop.
	(inferior_program_state): New struct.
	(save_inferior_program_state,restore_inferior_program_state,
	make_cleanup_restore_inferior_program_state,
	discard_inferior_program_state,
	get_inferior_program_state_regcache): New functions.
	(save_inferior_status): Remove arg restore_stack_info.
	All callers updated.  Remove saving of state now saved by
	save_inferior_program_state.
	(restore_inferior_status): Remove restoration of state now done by
	restore_inferior_program_state.
	(discard_inferior_status): Remove freeing of registers, now done by
	discard_inferior_program_state.

	* gdb.base/call-signal-resume.exp: New file.
	* gdb.base/call-signals.c: New file.
	* gdb.base/unwindonsignal.exp: New file.

Index: dummy-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dummy-frame.c,v
retrieving revision 1.53
diff -u -p -u -p -r1.53 dummy-frame.c
--- dummy-frame.c	30 Oct 2008 20:35:30 -0000	1.53
+++ dummy-frame.c	18 Nov 2008 11:23:36 -0000
@@ -42,8 +42,8 @@ struct dummy_frame
   /* This frame's ID.  Must match the value returned by
      gdbarch_dummy_id.  */
   struct frame_id id;
-  /* The caller's regcache.  */
-  struct regcache *regcache;
+  /* The caller's state prior to the call.  */
+  struct inferior_program_state *caller_state;
 };
 
 static struct dummy_frame *dummy_frame_stack = NULL;
@@ -81,61 +81,183 @@ deprecated_pc_in_call_dummy (CORE_ADDR p
   return 0;
 }
 
-/* Push the caller's state, along with the dummy frame info, onto a
+/* Push the caller's state, along with the dummy frame info, onto the
    dummy-frame stack.  */
 
-void
-dummy_frame_push (struct regcache *caller_regcache,
+struct dummy_frame *
+dummy_frame_push (struct inferior_program_state *caller_state,
 		  const struct frame_id *dummy_id)
 {
   struct dummy_frame *dummy_frame;
 
   dummy_frame = XZALLOC (struct dummy_frame);
-  dummy_frame->regcache = caller_regcache;
+  dummy_frame->caller_state = caller_state;
   dummy_frame->id = (*dummy_id);
   dummy_frame->next = dummy_frame_stack;
   dummy_frame_stack = dummy_frame;
+
+  return dummy_frame;
 }
 
-/* Pop the dummy frame with ID dummy_id from the dummy-frame stack.  */
+/* Remove *DUMMY_PTR from the dummy frame stack.  */
 
-void
-dummy_frame_pop (struct frame_id dummy_id)
+static void
+remove_dummy_frame (struct dummy_frame **dummy_ptr)
+{
+  struct dummy_frame *dummy = *dummy_ptr;
+
+  *dummy_ptr = dummy->next;
+  discard_inferior_program_state (dummy->caller_state);
+  xfree (dummy);
+}
+
+/* Cleanup handler for dummy_frame_pop.  */
+
+static void
+do_dummy_frame_cleanup (void *arg)
 {
-  struct dummy_frame **dummy_ptr;
+  struct dummy_frame **dummy_ptr = arg;
+
+  remove_dummy_frame (dummy_ptr);
+}
+
+/* Pop *DUMMY_PTR, restoring program state to that before the
+   frame was created.  */
+
+static void
+pop_dummy_frame (struct dummy_frame **dummy_ptr)
+{
+  struct cleanup *cleanups;
+  struct dummy_frame *dummy;
+
+  cleanups = make_cleanup (do_dummy_frame_cleanup, dummy_ptr);
 
-  for (dummy_ptr = &dummy_frame_stack;
-       (*dummy_ptr) != NULL;
-       dummy_ptr = &(*dummy_ptr)->next)
+  restore_inferior_program_state ((*dummy_ptr)->caller_state);
+
+  /* restore_inferior_status frees inf_status,
+     all that remains is to pop *dummy_ptr */
+  discard_cleanups (cleanups);
+  dummy = *dummy_ptr;
+  *dummy_ptr = dummy->next;
+  xfree (dummy);
+
+  /* We've made right mess of GDB's local state, just discard
+     everything.  */
+  reinit_frame_cache ();
+}
+
+/* Assert that DUMMY is in the dummy frame stack.  */
+
+static void
+assert_dummy_present (struct dummy_frame *dummy)
+{
+  struct dummy_frame *d;
+  int found = 0;
+
+  for (d = dummy_frame_stack; d != NULL; d = d->next)
     {
-      struct dummy_frame *dummy = *dummy_ptr;
-      if (frame_id_eq (dummy->id, dummy_id))
+      if (d == dummy)
 	{
-	  *dummy_ptr = dummy->next;
-	  regcache_xfree (dummy->regcache);
-	  xfree (dummy);
+	  found = 1;
 	  break;
 	}
     }
+  gdb_assert (found);
 }
 
-/* There may be stale dummy frames, perhaps left over from when a longjump took us
-   out of a function that was called by the debugger.  Clean them up at least once
-   whenever we start a new inferior.  */
+/* Pop DUMMY, restore inferior state,
+   and remove all dummy frames below DUMMY from the stack.
+   If the frame isn't found, flag an internal error.  */
 
 static void
-cleanup_dummy_frames (struct target_ops *target, int from_tty)
+pop_dummy_frame_and_below (struct dummy_frame *dummy)
+{
+  assert_dummy_present (dummy);
+  while (dummy_frame_stack != dummy)
+    {
+      remove_dummy_frame (&dummy_frame_stack);
+    }
+  pop_dummy_frame (&dummy_frame_stack);
+}
+
+/* Look up DUMMY_ID.
+   Return NULL if not found.  */
+
+static struct dummy_frame *
+lookup_dummy_id (struct frame_id dummy_id)
 {
-  struct dummy_frame *dummy, *next;
+  struct dummy_frame *dummy;
 
-  for (dummy = dummy_frame_stack; dummy; dummy = next)
+  for (dummy = dummy_frame_stack; dummy != NULL; dummy = dummy->next)
     {
-      next = dummy->next;
-      regcache_xfree (dummy->regcache);
-      xfree (dummy);
+      if (frame_id_eq (dummy->id, dummy_id))
+	return dummy;
     }
 
-  dummy_frame_stack = NULL;
+  return NULL;
+}
+
+/* Pop the dummy frame DUMMY_ID, restoring program state to that before the
+   frame was created.
+   This also removes all dummy frames below DUMMY_ID from the stack.
+   On return reinit_frame_cache has been called.
+
+   If the frame isn't found, flag an internal error.  */
+
+void
+dummy_frame_pop (struct frame_id dummy_id)
+{
+  struct dummy_frame *dummy;
+
+  dummy = lookup_dummy_id (dummy_id);
+  gdb_assert (dummy != NULL);
+
+  pop_dummy_frame_and_below (dummy);
+}
+
+/* Discard DUMMY and remove it from the dummy frame stack.
+   This also removes all dummy frames below DUMMY from the stack.
+
+   If the frame isn't found, flag an internal error.  */
+
+void
+dummy_frame_discard (struct dummy_frame *dummy)
+{
+  assert_dummy_present (dummy);
+  while (dummy_frame_stack != dummy)
+    {
+      remove_dummy_frame (&dummy_frame_stack);
+    }
+  remove_dummy_frame (&dummy_frame_stack);
+}
+
+/* Utility for make_cleanup_pop_dummy_frame.  */
+
+static void
+do_pop_dummy_frame_cleanup (void *dummy)
+{
+  pop_dummy_frame_and_below (dummy);
+}
+
+/* Schedule a cleanup to pop DUMMY_FRAME and restore inferior state.  */
+
+struct cleanup *
+make_cleanup_pop_dummy_frame (struct dummy_frame *dummy)
+{
+  return make_cleanup (do_pop_dummy_frame_cleanup, dummy);
+}
+
+/* There may be stale dummy frames, perhaps left over from when a longjump took
+   us out of a function that was called by the debugger.  Clean them up at
+   least once whenever we start a new inferior.  */
+
+static void
+cleanup_dummy_frames (struct target_ops *target, int from_tty)
+{
+  while (dummy_frame_stack != NULL)
+    {
+      remove_dummy_frame (&dummy_frame_stack);
+    }
 }
 
 /* Return the dummy frame cache, it contains both the ID, and a
@@ -178,7 +300,7 @@ dummy_frame_sniffer (const struct frame_
 	    {
 	      struct dummy_frame_cache *cache;
 	      cache = FRAME_OBSTACK_ZALLOC (struct dummy_frame_cache);
-	      cache->prev_regcache = dummyframe->regcache;
+	      cache->prev_regcache = get_inferior_program_state_regcache (dummyframe->caller_state);
 	      cache->this_id = this_id;
 	      (*this_prologue_cache) = cache;
 	      return 1;
@@ -215,7 +337,7 @@ dummy_frame_prev_register (struct frame_
   return reg_val;
 }
 
-/* Assuming that THIS frame is a dummy, return the ID of THIS frame.  That ID is
+/* Assuming that THIS_FRAME is a dummy, return its ID.  That ID is
    determined by examining the NEXT frame's unwound registers using
    the method dummy_id().  As a side effect, THIS dummy frame's
    dummy cache is located and and saved in THIS_PROLOGUE_CACHE.  */
Index: dummy-frame.h
===================================================================
RCS file: /cvs/src/src/gdb/dummy-frame.h,v
retrieving revision 1.23
diff -u -p -u -p -r1.23 dummy-frame.h
--- dummy-frame.h	8 Sep 2008 15:23:12 -0000	1.23
+++ dummy-frame.h	18 Nov 2008 11:23:36 -0000
@@ -23,8 +23,10 @@
 #include "frame.h"
 
 struct frame_info;
-struct regcache;
+struct inferior_program_state;
 struct frame_unwind;
+struct dummy_frame;
+struct cleanup;
 
 /* Push the information needed to identify, and unwind from, a dummy
    frame onto the dummy frame stack.  */
@@ -39,11 +41,27 @@ struct frame_unwind;
    be expanded so that it knowns the lower/upper extent of the dummy
    frame's code.  */
 
-extern void dummy_frame_push (struct regcache *regcache,
-			      const struct frame_id *dummy_id);
+extern struct dummy_frame *dummy_frame_push (struct inferior_program_state *caller_state,
+					     const struct frame_id *dummy_id);
+
+/* Pop the dummy frame DUMMY_ID, restoring program state to that before the
+   frame was created.
+   This also removes all dummy frames below DUMMY_ID from the stack,
+   and calls reinit_frame_cache.
+
+   If the frame isn't found, flag an internal error.  */
 
 extern void dummy_frame_pop (struct frame_id dummy_id);
 
+/* Discard DUMMY and remove it from the dummy frame stack.
+   This also removes all dummy frames below DUMMY from the stack.  */
+
+extern void dummy_frame_discard (struct dummy_frame *dummy);
+
+/* Schedule a cleanup to pop DUMMY_FRAME.  */
+
+extern struct cleanup *make_cleanup_pop_dummy_frame (struct dummy_frame *);
+
 /* If the PC falls in a dummy frame, return a dummy frame
    unwinder.  */
 
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.255
diff -u -p -u -p -r1.255 frame.c
--- frame.c	24 Sep 2008 12:59:49 -0000	1.255
+++ frame.c	18 Nov 2008 11:23:36 -0000
@@ -536,6 +536,14 @@ frame_pop (struct frame_info *this_frame
   struct regcache *scratch;
   struct cleanup *cleanups;
 
+  if (get_frame_type (this_frame) == DUMMY_FRAME)
+    {
+      /* Popping a dummy frame involves restoring more than just registers.
+	 dummy_frame_pop does all the work.  */
+      dummy_frame_pop (get_frame_id (this_frame));
+      return;
+    }
+
   /* Ensure that we have a frame to pop to.  */
   prev_frame = get_prev_frame_1 (this_frame);
 
@@ -549,11 +557,6 @@ frame_pop (struct frame_info *this_frame
   scratch = frame_save_as_regcache (prev_frame);
   cleanups = make_cleanup_regcache_xfree (scratch);
 
-  /* If we are popping a dummy frame, clean up the associated
-     data as well.  */
-  if (get_frame_type (this_frame) == DUMMY_FRAME)
-    dummy_frame_pop (get_frame_id (this_frame));
-
   /* FIXME: cagney/2003-03-16: It should be possible to tell the
      target's register cache that it is about to be hit with a burst
      register transfer and that the sequence of register writes should
Index: infcall.c
===================================================================
RCS file: /cvs/src/src/gdb/infcall.c,v
retrieving revision 1.106
diff -u -p -u -p -r1.106 infcall.c
--- infcall.c	18 Nov 2008 00:13:02 -0000	1.106
+++ infcall.c	18 Nov 2008 11:23:36 -0000
@@ -318,12 +318,14 @@ call_function_by_hand (struct value *fun
   struct cleanup *retbuf_cleanup;
   struct inferior_status *inf_status;
   struct cleanup *inf_status_cleanup;
+  struct inferior_program_state *caller_state;
+  struct cleanup *caller_state_cleanup;
+  struct dummy_frame *dummy_frame;
+  struct cleanup *dummy_frame_cleanup;
   CORE_ADDR funaddr;
   CORE_ADDR real_pc;
   struct type *ftype = check_typedef (value_type (function));
   CORE_ADDR bp_addr;
-  struct regcache *caller_regcache;
-  struct cleanup *caller_regcache_cleanup;
   struct frame_id dummy_id;
   struct cleanup *args_cleanup;
   struct frame_info *frame;
@@ -351,15 +353,16 @@ call_function_by_hand (struct value *fun
   /* A cleanup for the inferior status.  Create this AFTER the retbuf
      so that this can be discarded or applied without interfering with
      the regbuf.  */
-  inf_status = save_inferior_status (1);
+  inf_status = save_inferior_status ();
   inf_status_cleanup = make_cleanup_restore_inferior_status (inf_status);
 
-  /* Save the caller's registers so that they can be restored once the
+  /* Save the caller's registers and other state associated with the
+     inferior itself 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).  */
-  caller_regcache = frame_save_as_regcache (frame);
-  caller_regcache_cleanup = make_cleanup_regcache_xfree (caller_regcache);
+  caller_state = save_inferior_program_state ();
+  caller_state_cleanup = make_cleanup_restore_inferior_program_state (caller_state);
 
   /* Ensure that the initial SP is correctly aligned.  */
   {
@@ -642,8 +645,10 @@ call_function_by_hand (struct value *fun
   /* Everything's ready, push all the info needed to restore the
      caller (and identify the dummy-frame) onto the dummy-frame
      stack.  */
-  dummy_frame_push (caller_regcache, &dummy_id);
-  discard_cleanups (caller_regcache_cleanup);
+  dummy_frame = dummy_frame_push (caller_state, &dummy_id);
+  /* Do this before calling make_cleanup_pop_dummy_frame.  */
+  discard_cleanups (caller_state_cleanup);
+  dummy_frame_cleanup = make_cleanup_pop_dummy_frame (dummy_frame);
 
   /* - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP -
      If you're looking to implement asynchronous dummy-frames, then
@@ -705,6 +710,7 @@ call_function_by_hand (struct value *fun
 	 we'll crash as the inferior is no longer running.  */
       discard_cleanups (inf_status_cleanup);
       discard_inferior_status (inf_status);
+      dummy_frame_discard (dummy_frame);
       error (_("\
 The program being debugged exited while in a function called from GDB."));
     }
@@ -745,9 +751,10 @@ 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 ());
+	      /* We must get back to the frame we were before the dummy call.
+		 Plus we need to restore inferior status to that before the
+		 dummy call.  This is all handled by cleanups
+		 dummy_frame_cleanup and inf_status_cleanup.  */
 
 	      /* FIXME: Insert a bunch of wrap_here; name can be very
 		 long if it's a C++ name with arguments and stuff.  */
@@ -761,14 +768,16 @@ Evaluation of the expression containing 
 	  else
 	    {
 	      /* The user wants to stay in the frame where we stopped
-                 (default).*/
-	      /* If we restored the inferior status (via the cleanup),
+		 (default).
+		 If we restored the inferior status (via the cleanup),
 		 we would print a spurious error message (Unable to
-		 restore previously selected frame), would write the
-		 registers from the inf_status (which is wrong), and
-		 would do other wrong things.  */
+		 restore previously selected frame), and
+		 would do other wrong things.
+		 Discarding inf_status_cleanup also discards
+		 dummy_frame_cleanup.  */
 	      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 (_("\
@@ -782,14 +791,17 @@ Evaluation of the expression containing 
 
       if (!stop_stack_dummy)
 	{
-	  /* We hit a breakpoint inside the FUNCTION. */
-	  /* If we restored the inferior status (via the cleanup), we
+	  /* We hit a breakpoint inside the FUNCTION.
+	     If we restored the inferior status (via the cleanup), we
 	     would print a spurious error message (Unable to restore
 	     previously selected frame), would write the registers
 	     from the inf_status (which is wrong), and would do other
-	     wrong things.  */
+	     wrong things.
+	     Discarding inf_status_cleanup also discards
+	     dummy_frame_cleanup.  */
 	  discard_cleanups (inf_status_cleanup);
 	  discard_inferior_status (inf_status);
+
 	  /* The following error message used to say "The expression
 	     which contained the function call has been discarded."
 	     It is a hard concept to explain in a few words.  Ideally,
@@ -811,6 +823,9 @@ the function call)."), name);
 
   /* If we get here the called FUNCTION run to completion. */
 
+  /* The dummy frame has been popped so discard its cleanup.  */
+  discard_cleanups (dummy_frame_cleanup);
+
   /* On normal return, the stack dummy has been popped already.  */
   regcache_cpy_no_passthrough (retbuf, stop_registers);
 
Index: inferior.h
===================================================================
RCS file: /cvs/src/src/gdb/inferior.h,v
retrieving revision 1.115
diff -u -p -u -p -r1.115 inferior.h
--- inferior.h	17 Nov 2008 16:37:34 -0000	1.115
+++ inferior.h	18 Nov 2008 11:23:36 -0000
@@ -40,24 +40,40 @@ struct ui_out;
 /* For struct frame_id.  */
 #include "frame.h"
 
-/* Structure in which to save the status of the inferior.  Create/Save
-   through "save_inferior_status", restore through
-   "restore_inferior_status".
-
-   This pair of routines should be called around any transfer of
-   control to the inferior which you don't want showing up in your
-   control variables.  */
+/* Two structures are used to record inferior state.
 
+   inferior_program_state contains state about the program itself like its
+   registers and any signal it received when it last stopped.
+   This state must be restored regardless of how the inferior function call
+   ends (either successfully, or after it hits a breakpoint or signal)
+   if the program is to properly continue where it left off.
+
+   inferior_status contains state regarding gdb's control of the inferior
+   itself like stepping control.  It also contains session state like the
+   user's currently selected frame.
+   This state is only restored upon successful completion of the
+   inferior function call.
+
+   Call these routines around hand called functions, including function calls
+   in conditional breakpoints for example.  */
+
+struct inferior_program_state;
 struct inferior_status;
 
-extern struct inferior_status *save_inferior_status (int);
+extern struct inferior_program_state *save_inferior_program_state (void);
+extern struct inferior_status *save_inferior_status (void);
 
+extern void restore_inferior_program_state (struct inferior_program_state *);
 extern void restore_inferior_status (struct inferior_status *);
 
+extern struct cleanup *make_cleanup_restore_inferior_program_state (struct inferior_program_state *);
 extern struct cleanup *make_cleanup_restore_inferior_status (struct inferior_status *);
 
+extern void discard_inferior_program_state (struct inferior_program_state *);
 extern void discard_inferior_status (struct inferior_status *);
 
+extern struct regcache *get_inferior_program_state_regcache (struct inferior_program_state *);
+
 /* The -1 ptid, often used to indicate either an error condition
    or a "don't care" condition, i.e, "run all threads."  */
 extern ptid_t minus_one_ptid;
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.338
diff -u -p -u -p -r1.338 infrun.c
--- infrun.c	17 Nov 2008 18:50:22 -0000	1.338
+++ infrun.c	18 Nov 2008 11:23:36 -0000
@@ -45,7 +45,7 @@
 #include "language.h"
 #include "solib.h"
 #include "main.h"
-
+#include "dummy-frame.h"
 #include "gdb_assert.h"
 #include "mi/mi-common.h"
 #include "event-top.h"
@@ -4342,15 +4342,23 @@ Further execution is probably impossible
 
   if (stop_stack_dummy)
     {
-      /* Pop the empty frame that contains the stack dummy.  POP_FRAME
-         ends with a setting of the current frame, so we can use that
-         next. */
-      frame_pop (get_current_frame ());
-      /* Set stop_pc to what it was before we called the function.
-         Can't rely on restore_inferior_status because that only gets
-         called if we don't stop in the called function.  */
-      stop_pc = read_pc ();
-      select_frame (get_current_frame ());
+      /* Pop the empty frame that contains the stack dummy.
+	 This also restores all inferior state prior to the call.
+	 If the current frame is not a stack dummy, do nothing and warn
+	 the user.  */
+      struct frame_info *frame = get_current_frame ();
+      if (get_frame_type (frame) == DUMMY_FRAME)
+	{
+	  dummy_frame_pop (get_frame_id (frame));
+	}
+      else
+	{
+	  /* We avoid calling the frame a dummy frame as it has little
+	     meaning to the user.  */
+	  printf_filtered (_("\
+Stopped after an inferior function call, but not in the expected stack frame.\n\
+Proceed with caution.\n"));
+	}
     }
 
 done:
@@ -4746,10 +4754,86 @@ signals_info (char *signum_exp, int from
   printf_filtered (_("\nUse the \"handle\" command to change these tables.\n"));
 }
 
-struct inferior_status
+/* Inferior program state.
+   These are details related to the inferior itself, and don't include
+   things like what frame the user had selected or what gdb was doing
+   with the target at the time.
+   For inferior function calls these are things we want to restore
+   regardless of whether the function call successfully completes
+   or the dummy frame has to be manually popped.  */
+
+struct inferior_program_state
 {
   enum target_signal stop_signal;
   CORE_ADDR stop_pc;
+  struct regcache *registers;
+};
+
+struct inferior_program_state *
+save_inferior_program_state ()
+{
+  struct inferior_program_state *inf_state = XMALLOC (struct inferior_program_state);
+  struct thread_info *tp = inferior_thread ();
+  struct inferior *inf = current_inferior ();
+
+  inf_state->stop_signal = tp->stop_signal;
+  inf_state->stop_pc = stop_pc;
+
+  inf_state->registers = regcache_dup (get_current_regcache ());
+
+  return inf_state;
+}
+
+/* Restore inferior session state to INF_STATE.  */
+
+void
+restore_inferior_program_state (struct inferior_program_state *inf_state)
+{
+  struct thread_info *tp = inferior_thread ();
+  struct inferior *inf = current_inferior ();
+
+  tp->stop_signal = inf_state->stop_signal;
+  stop_pc = inf_state->stop_pc;
+
+  /* The inferior can be gone if the user types "print exit(0)"
+     (and perhaps other times).  */
+  if (target_has_execution)
+    /* NB: The register write goes through to the target.  */
+    regcache_cpy (get_current_regcache (), inf_state->registers);
+  regcache_xfree (inf_state->registers);
+}
+
+static void
+do_restore_inferior_program_state_cleanup (void *state)
+{
+  restore_inferior_program_state (state);
+}
+
+struct cleanup *
+make_cleanup_restore_inferior_program_state (struct inferior_program_state *inf_state)
+{
+  return make_cleanup (do_restore_inferior_program_state_cleanup, inf_state);
+}
+
+void
+discard_inferior_program_state (struct inferior_program_state *inf_state)
+{
+  regcache_xfree (inf_state->registers);
+  xfree (inf_state);
+}
+
+struct regcache *
+get_inferior_program_state_regcache (struct inferior_program_state *inf_state)
+{
+  return inf_state->registers;
+}
+
+/* Session related state for inferior function calls.
+   These are the additional bits of state that need to be restored
+   when an inferior function call successfully completes.  */
+
+struct inferior_status
+{
   bpstat stop_bpstat;
   int stop_step;
   int stop_stack_dummy;
@@ -4763,32 +4847,23 @@ struct inferior_status
   int stop_after_trap;
   int stop_soon;
 
-  /* These are here because if call_function_by_hand has written some
-     registers and then decides to call error(), we better not have changed
-     any registers.  */
-  struct regcache *registers;
-
-  /* A frame unique identifier.  */
+  /* ID if the selected frame when the inferior function call was made.  */
   struct frame_id selected_frame_id;
 
   int breakpoint_proceeded;
-  int restore_stack_info;
   int proceed_to_finish;
 };
 
 /* Save all of the information associated with the inferior<==>gdb
-   connection.  INF_STATUS is a pointer to a "struct inferior_status"
-   (defined in inferior.h).  */
+   connection.  */
 
 struct inferior_status *
-save_inferior_status (int restore_stack_info)
+save_inferior_status ()
 {
   struct inferior_status *inf_status = XMALLOC (struct inferior_status);
   struct thread_info *tp = inferior_thread ();
   struct inferior *inf = current_inferior ();
 
-  inf_status->stop_signal = tp->stop_signal;
-  inf_status->stop_pc = stop_pc;
   inf_status->stop_step = tp->stop_step;
   inf_status->stop_stack_dummy = stop_stack_dummy;
   inf_status->stopped_by_random_signal = stopped_by_random_signal;
@@ -4806,12 +4881,10 @@ save_inferior_status (int restore_stack_
   inf_status->stop_bpstat = tp->stop_bpstat;
   tp->stop_bpstat = bpstat_copy (tp->stop_bpstat);
   inf_status->breakpoint_proceeded = breakpoint_proceeded;
-  inf_status->restore_stack_info = restore_stack_info;
   inf_status->proceed_to_finish = tp->proceed_to_finish;
 
-  inf_status->registers = regcache_dup (get_current_regcache ());
-
   inf_status->selected_frame_id = get_frame_id (get_selected_frame (NULL));
+
   return inf_status;
 }
 
@@ -4836,14 +4909,14 @@ restore_selected_frame (void *args)
   return (1);
 }
 
+/* Restore inferior session state to INF_STATUS.  */
+
 void
 restore_inferior_status (struct inferior_status *inf_status)
 {
   struct thread_info *tp = inferior_thread ();
   struct inferior *inf = current_inferior ();
 
-  tp->stop_signal = inf_status->stop_signal;
-  stop_pc = inf_status->stop_pc;
   tp->stop_step = inf_status->stop_step;
   stop_stack_dummy = inf_status->stop_stack_dummy;
   stopped_by_random_signal = inf_status->stopped_by_random_signal;
@@ -4856,24 +4929,11 @@ restore_inferior_status (struct inferior
   inf->stop_soon = inf_status->stop_soon;
   bpstat_clear (&tp->stop_bpstat);
   tp->stop_bpstat = inf_status->stop_bpstat;
+  inf_status->stop_bpstat = NULL;
   breakpoint_proceeded = inf_status->breakpoint_proceeded;
   tp->proceed_to_finish = inf_status->proceed_to_finish;
 
-  /* The inferior can be gone if the user types "print exit(0)"
-     (and perhaps other times).  */
-  if (target_has_execution)
-    /* NB: The register write goes through to the target.  */
-    regcache_cpy (get_current_regcache (), inf_status->registers);
-  regcache_xfree (inf_status->registers);
-
-  /* FIXME: If we are being called after stopping in a function which
-     is called from gdb, we should not be trying to restore the
-     selected frame; it just prints a spurious error message (The
-     message is useful, however, in detecting bugs in gdb (like if gdb
-     clobbers the stack)).  In fact, should we be restoring the
-     inferior status at all in that case?  .  */
-
-  if (target_has_stack && inf_status->restore_stack_info)
+  if (target_has_stack)
     {
       /* The point of catch_errors is that if the stack is clobbered,
          walking the stack might encounter a garbage pointer and
@@ -4885,7 +4945,6 @@ restore_inferior_status (struct inferior
 	/* Error in restoring the selected frame.  Select the innermost
 	   frame.  */
 	select_frame (get_current_frame ());
-
     }
 
   xfree (inf_status);
@@ -4908,10 +4967,9 @@ discard_inferior_status (struct inferior
 {
   /* See save_inferior_status for info on stop_bpstat. */
   bpstat_clear (&inf_status->stop_bpstat);
-  regcache_xfree (inf_status->registers);
   xfree (inf_status);
 }
-
+
 int
 inferior_has_forked (ptid_t pid, ptid_t *child_pid)
 {
Index: testsuite/gdb.base/call-signal-resume.exp
===================================================================
RCS file: testsuite/gdb.base/call-signal-resume.exp
diff -N testsuite/gdb.base/call-signal-resume.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/call-signal-resume.exp	18 Nov 2008 11:23:37 -0000
@@ -0,0 +1,107 @@
+# 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 call-signal-resume.exp because of no fileio capabilities."
+    continue
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile "call-signals"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested call-signal-resume.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
+}
+
+proc get_dummy_frame_number { } {
+  global gdb_prompt
+
+  send_gdb "bt\n"
+  gdb_expect {
+    -re "#(\[0-9\]*) *<function called from gdb>.*$gdb_prompt $"
+      {
+	return $expect_out(1,string)
+      }
+    -re "$gdb_prompt $"
+      {
+	return ""
+      }
+    timeout
+      {
+	return ""
+      }
+  }
+  return ""
+}
+
+# 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
+}
+
+# Call function (causing the program to get a signal), and see if gdb handles
+# it properly.
+gdb_test_multiple "call gen_signal ()" \
+	"call-signal-resume, inferior function call signaled" {
+    -re "\[\r\n\]*no signal\[\r\n\]+$gdb_prompt $" {
+	unsupported "call-signal-resume, inferior function call signaled"
+	return 0
+    }
+    -re "\[\r\n\]*The program being debugged was signaled.*\[\r\n\]+$gdb_prompt $" {
+	pass "call-signal-resume, inferior function call signaled"
+    }
+}
+
+set frame_number [get_dummy_frame_number]
+if { "$frame_number" == "" } {
+    fail "call-signal-resume, dummy stack frame number"
+    setup_xfail "*-*-*"
+} else {
+    pass "call-signal-resume, dummy stack frame number"
+}
+
+# Pop the dummy frame.
+gdb_test "frame $frame_number" ""
+gdb_test "set confirm off" ""
+gdb_test "return" ""
+
+# Resume execution, the program should successfully complete.
+gdb_test "continue" "Program exited normally."
+
+return 0
Index: testsuite/gdb.base/call-signals.c
===================================================================
RCS file: testsuite/gdb.base/call-signals.c
diff -N testsuite/gdb.base/call-signals.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/call-signals.c	18 Nov 2008 11:23:37 -0000
@@ -0,0 +1,48 @@
+/* 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 inferior function calls
+   in the presence of signals.  */
+
+#include <stdio.h>
+#include <signal.h>
+#include <unistd.h>
+
+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	18 Nov 2008 11:23:37 -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 "call-signals"
+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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]