Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Daniel Jacobowitz <drow@false.org>
To: Shaun Jackman <sjackman@gmail.com>, gdb-patches@sourceware.org
Subject: Fix a crash when stepping and unwinding fails
Date: Tue, 21 Feb 2006 04:33:00 -0000	[thread overview]
Message-ID: <20060220220331.GA29363@nevyn.them.org> (raw)

This patch stops GDB from segfaulting when we step into a function,
but can not unwind back out of the function.  We would previously
call get_prev_frame, which would return NULL, and then try to
get_frame_pc (NULL).

Now we'll issue this error instead, and stop stepping:

Could not step out of the function at 0x80144400 - unwinding failed

It's still not great, but at least it's an improvement over crashing.
It is reasonably likely that we've just stepped over a standard
function call, and that consequentially the function return
address is in the standard place for the architecture; in fact,
GDB used to have a hook for this, before the frame overhaul:
SAVED_PC_AFTER_CALL.  But it's gone now and there's no easy analogue,
and it was never 100% reliable anyway.  So unfortunately, if we
single-step out to an address that we can't find a way to unwind from,
we'll stop instead of stepping out.

Hmm.  Alternatively, we could stop stepping without an error.  Would
that be better?  Seems likely.  I'll wait for comments before I try
implementing that, though.  Should we warn when we do that, in addition
to stopping, or is the warning just noise?

Shaun, I believe this is the crash you reported on gdb@ several times.
I can't think of any easy way to write a test for this.

-- 
Daniel Jacobowitz
CodeSourcery

2006-02-20  Daniel Jacobowitz  <dan@codesourcery.com>

	* infrun.c (insert_step_resume_breakpoint_at_frame): Add
	USE_PREVIOUS argument.  Issue an error if we can not unwind
	to the previous frame.
	(handle_inferior_event): Update calls.

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2006-01-04 14:34:58.000000000 -0500
+++ src/gdb/infrun.c	2006-02-20 15:59:18.000000000 -0500
@@ -942,7 +942,7 @@ void init_execution_control_state (struc
 void handle_inferior_event (struct execution_control_state *ecs);
 
 static void step_into_function (struct execution_control_state *ecs);
-static void insert_step_resume_breakpoint_at_frame (struct frame_info *step_frame);
+static void insert_step_resume_breakpoint_at_frame (struct frame_info *, int);
 static void insert_step_resume_breakpoint_at_sal (struct symtab_and_line sr_sal,
 						  struct frame_id sr_id);
 static void stop_stepping (struct execution_control_state *ecs);
@@ -1965,7 +1965,7 @@ process_event_stop_test:
 	     code paths as single-step - set a breakpoint at the
 	     signal return address and then, once hit, step off that
 	     breakpoint.  */
-	  insert_step_resume_breakpoint_at_frame (get_current_frame ());
+	  insert_step_resume_breakpoint_at_frame (get_current_frame (), 0);
 	  ecs->step_after_step_resume_breakpoint = 1;
 	  keep_going (ecs);
 	  return;
@@ -1987,7 +1987,7 @@ process_event_stop_test:
 	     Note that this is only needed for a signal delivered
 	     while in the single-step range.  Nested signals aren't a
 	     problem as they eventually all return.  */
-	  insert_step_resume_breakpoint_at_frame (get_current_frame ());
+	  insert_step_resume_breakpoint_at_frame (get_current_frame (), 0);
 	  keep_going (ecs);
 	  return;
 	}
@@ -2396,7 +2396,7 @@ process_event_stop_test:
 	  /* We're doing a "next", set a breakpoint at callee's return
 	     address (the address at which the caller will
 	     resume).  */
-	  insert_step_resume_breakpoint_at_frame (get_prev_frame (get_current_frame ()));
+	  insert_step_resume_breakpoint_at_frame (get_current_frame (), 1);
 	  keep_going (ecs);
 	  return;
 	}
@@ -2459,7 +2459,7 @@ process_event_stop_test:
 
       /* Set a breakpoint at callee's return address (the address at
          which the caller will resume).  */
-      insert_step_resume_breakpoint_at_frame (get_prev_frame (get_current_frame ()));
+      insert_step_resume_breakpoint_at_frame (get_current_frame (), 1);
       keep_going (ecs);
       return;
     }
@@ -2528,7 +2528,7 @@ process_event_stop_test:
 	{
 	  /* Set a breakpoint at callee's return address (the address
 	     at which the caller will resume).  */
-	  insert_step_resume_breakpoint_at_frame (get_prev_frame (get_current_frame ()));
+	  insert_step_resume_breakpoint_at_frame (get_current_frame (), 1);
 	  keep_going (ecs);
 	  return;
 	}
@@ -2741,22 +2741,33 @@ insert_step_resume_breakpoint_at_sal (st
    that the function/signal handler being skipped eventually returns
    to the breakpoint inserted at RETURN_FRAME.pc.
 
-   For the skip-function case, the function may have been reached by
-   either single stepping a call / return / signal-return instruction,
-   or by hitting a breakpoint.  In all cases, the RETURN_FRAME belongs
-   to the skip-function's caller.
+   If USE_PREVIOUS is zero, RETURN_FRAME belongs to the function being
+   skipped.  The function may have been reached by either single
+   stepping a call / return / signal-return instruction, or by hitting
+   a breakpoint.
 
    For the signals case, this is called with the interrupted
    function's frame.  The signal handler, when it returns, will resume
    the interrupted function at RETURN_FRAME.pc.  */
 
 static void
-insert_step_resume_breakpoint_at_frame (struct frame_info *return_frame)
+insert_step_resume_breakpoint_at_frame (struct frame_info *return_frame,
+					int use_previous)
 {
   struct symtab_and_line sr_sal;
 
   init_sal (&sr_sal);		/* initialize to zeros */
 
+  if (use_previous)
+    {
+      struct frame_info *caller_frame;
+      caller_frame = get_prev_frame (return_frame);
+      if (caller_frame == NULL)
+	error (_("Could not step out of the function at 0x%x - unwinding failed"),
+	       get_frame_pc (return_frame));
+      return_frame = caller_frame;
+    }
+
   sr_sal.pc = ADDR_BITS_REMOVE (get_frame_pc (return_frame));
   sr_sal.section = find_pc_overlay (sr_sal.pc);
 


             reply	other threads:[~2006-02-20 22:03 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-02-21  4:33 Daniel Jacobowitz [this message]
2006-02-21 10:01 ` Eli Zaretskii
2006-02-21 20:28 ` Mark Kettenis
2006-02-21 20:43   ` Daniel Jacobowitz
2006-02-21 20:54     ` Mark Kettenis
2006-02-21 21:03       ` Daniel Jacobowitz
2006-02-21 21:53         ` Mark Kettenis
2006-02-21 22:59           ` Daniel Jacobowitz
2006-02-22 22:00             ` Mark Kettenis
2006-02-23  2:04               ` Daniel Jacobowitz
2006-02-23 17:04                 ` NZG
2006-05-17 17:59               ` Daniel Jacobowitz
2006-06-13 18:42                 ` Daniel Jacobowitz
2006-06-14 12:33                   ` Joel Brobecker
2006-06-16  1:16                     ` Daniel Jacobowitz
2006-02-22  4:28           ` Jim Blandy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060220220331.GA29363@nevyn.them.org \
    --to=drow@false.org \
    --cc=gdb-patches@sourceware.org \
    --cc=sjackman@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox