Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* 3/5 - Rework stepping over longjmp support
@ 2008-04-07  2:34 Pedro Alves
  2008-04-07 19:43 ` Tom Tromey
  2008-04-14 18:57 ` Daniel Jacobowitz
  0 siblings, 2 replies; 7+ messages in thread
From: Pedro Alves @ 2008-04-07  2:34 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 4671 bytes --]

This rework was motivated by the non-stop mode.

To handle stepping over longjmp, currently, we create internal
disabled breakpoints on everything that looks like a 
longjmp : "longjmp", "_longjmp", "siglongjmp", and "_siglongjmp".
These should cover all cases and all OSs.  The basic
idea it that when one of these breakpoints is hit, we look
into the jmp_buf passed into longjmp, and extract the
destination PC from it.  We then set a breakpoint at
that address, and let the inferior hit it.  This handling
is only desirable when the user is activelly doing a 
next or a step.  We don't care for the inferior hitting
a longjmp when the inferior is running freely -- if we
left the breakpoint enabled all the time, even when
not stepping (e.g., user did a continue, and no user
breakpoints were hit, the inferior just runs
uninterrupted), anytime the inferior called longjmp, it
would hit the breakpoint, and then gdb would notice the
inferior was not being stepped, so it should be resumed
immediately.  It is much more efficient to not hit that
breakpoint at all in that case.  So far, so good, this
works OK in all-stop mode.  Well, in theory, because the
current implementation is broken.

However, in non-stop mode, we can step more than one thread
independenly and simultaneously.  Having one thread finish a
step, and disable the global longjmp breakpoint at that
point, while another thread is still stepping is definitelly
wrong.  The other thread may happen to step a longjmp and it
would go unnoticed.  Basically, we need to associate
the longjmp breakpoints with each stepping thread.
Fortunally, the concept of per-thread breakpoints already
exists in GDB.

The current implementation sets a handling_longjmp variable
when a longjmp is hit, and we insert a longjmp-resume
breakpoint.  If you look at infrun.c, you'll see there's
always a FIXME associated with it.  And it's right, because
it's handling is broken.

You can easilly see it breaking, by trying to step over
something like this:

#include <setjmp.h>

jmp_buf env;

int
call_longjmp (jmp_buf *buf)
{
  longjmp (*buf, 1);
}

int main ()
{
  if (setjmp (env) == 0) /* patt2 */
    {
      call_longjmp (&env);  <<<<<<<< try to step over this.
    }
  else
    {
      printf ("resumed\n");
    }
}

Basically, the symptom is that GDB will not stop at the longjmp-resume
address, instead the inferior will run to exit.  Quite annoying.

Instead of trying to explain deeply why the current implementation
is broken (has to do with bad breakpoint handling, thread hopping,
removing breakpoints at the wrong time, and the handling_longjmp
variable getting in the way), I propose another implementation.
We get rid of handling_longjmp, and instead, we handle a
longjmp-resume breakpoint much like a step-resume breakpoint.  It's
natural to not have both set at the same time, and the current
code already gets rid of the active step-resume breakpoint when
inserting the  longjmp-resume breakpoint.  Setting
step_resume_breakpoint to the new longjmp-resume breakpoint
is exactly what we want.  We want to keep stepping while it
is set, we want to context switch it.  We want to delete it everywhere
a step-resume is being deleted.  Of course, we could have a
seperate per-thread context-switchable longjmp_resume_breakpoint
instead, since we never have both a longjmp-resume and a
step-resume breakpoint simulatenously (in a given thread), and
it's handling would be equal everywhere else, it just feels better
to overload step_resume_breakpoint.

This patch implements the same behaviour that's in HEAD (if it weren't broken 
a lot of the times) where any longjmp hit while stepping, causes the
step to stop.  

-- 
Pedro Alves

P.S.:

However, we could extend this to be smarter.  E.g.:

void
hidden_longjmp (void)
{
  if (setjmp (env) == 0)
    {
      longjmp (env, 1);
    }
  else
    {
      printf ("resumed\n");
    }
}

int
main ()
{
   hidden_longjmp (); <<<< step over this.
}

The longjmp inside hidden_longjmp is going to land inside
hidden_longjmp.  GDB could ignore it leave the step-resume
breakpoint at the return of hidden_longjmp and pretend
that longjmp was never seen.  Think of stepping over a function
in gdb's sources, and an exception being thrown and caught all
somewhere inner to the function you're stepping.  I've
implemented a prototype patch that does this.  Does anyone
else think that behaviour is useful?  (I'm aware that any
smartness we add can be defeated by longjmp changing stacks,
or stuff like debugging coroutines implemented with set/longjmp,
but those feel like rare enough that a smart mode could be
the default and useful most of (almost-all) the times.)


[-- Attachment #2: longjmp_rewrite.diff --]
[-- Type: text/x-diff, Size: 21794 bytes --]

2008-04-07  Pedro Alves  <pedro@codesourcery.com>

	* breakpoint.c (update_breakpoints_after_exec): Delete bp_longjmp
	and bp_longjmp_resume breakpoints.
	(breakpoint_address_is_meaningful): Claim bp_longjmp_resume as
	meaningful.
	(create_longjmp_breakpoint): Don't create bp_longjmp_resume
	breakpoints.  Create bp_longjmp breakpoints as momentary
	breakpoints.
	(enable_longjmp_breakpoint): Delete.
	(set_longjmp_breakpoint): New.
	(disable_longjmp_breakpoint): Delete.
	(delete_longjmp_breakpoint_thread, delete_longjmp_breakpoint):
	New.
	(set_longjmp_resume_breakpoint): Delete.
	(set_momentary_breakpoint_at_pc): New.
	(breakpoint_re_set_one): Don't delete bp_longjmp and
	bp_longjmp_resume breakpoints.
	(breakpoint_re_set): Don't create longjmp and longjmp-resume
	breakpoints.

	* infrun.c (step_resume_breakpoint): Add comment.
	(struct execution_control_state): Delete handling_longjmp member.
	(init_execution_control_state). Don't clear handling_longjmp.
	(context_switch): Don't context switch handling_longjmp.
	(handle_inferior_event): If handling a bp_longjmp breakpoint,
	create a bp_longjmp_resume breakpoint, and set it as current
	step_resume_breakpoint, then step over the longjmp breakpoint.  If
	handling a bp_longjmp_resume breakpoint, don't delete the longjmp
	breakpoint, delete the longjmp-resume breakpoint, and stop
	stepping.
	(currently_stepping): Remove handling_longjmp from expression.
	(insert_step_resume_breakpoint_at_sal): Update comment.
	(insert_longjmp_resume_breakpoint): New.

	* breakpoint.h (set_momentary_breakpoint_at_pc): Declare.
	(enable_longjmp_breakpoint, disable_longjmp_breakpoint): Delete
	declarations.
	(set_longjmp_breakpoint, delete_longjmp_breakpoint)
	(delete_longjmp_breakpoint_thread): Declare.
	(set_longjmp_resume_breakpoint): Delete declaration.

	* gdbthread.h (save_infrun_state): Remove handling_longjmp parameter.
	(load_infrun_state): Delete *handling_longjmp parameter.
	* thread.c (save_infrun_state): Remove handling_longjmp parameter.  Update body.
	(load_infrun_state): Delete *handling_longjmp parameter.  Update body.

	* infcmd.c Include "gdbthread.h".
	(disable_longjmp_breakpoint_cleanup): Delete.
	(delete_longjmp_breakpoint_cleanup): New.
	(step_1): Call set_longjmp_breakpoint instead of
	enable_longjmp_breakpoint.  Use delete_longjmp_breakpoint_cleanup
	instead of disable_longjmp_breakpoint_cleanup when making cleanup.

	* Makefile.in (infcmd.o): Update.

---
 gdb/Makefile.in  |    2 
 gdb/breakpoint.c |  112 +++++++++++++++++++++----------------------------------
 gdb/breakpoint.h |    9 ++--
 gdb/gdbthread.h  |    3 -
 gdb/infcmd.c     |   26 ++++++++++--
 gdb/infrun.c     |   86 +++++++++++++++++++++++++++++-------------
 gdb/thread.c     |    4 -
 7 files changed, 131 insertions(+), 111 deletions(-)

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2008-04-07 02:24:30.000000000 +0100
+++ src/gdb/breakpoint.c	2008-04-07 02:28:24.000000000 +0100
@@ -149,8 +149,6 @@ static int watchpoint_check (void *);
 
 static void maintenance_info_breakpoints (char *, int);
 
-static void create_longjmp_breakpoint (char *);
-
 static void create_overlay_event_breakpoint (char *);
 
 static int hw_breakpoint_used_count (void);
@@ -1430,6 +1428,14 @@ update_breakpoints_after_exec (void)
 	continue;
       }
 
+    /* Longjmp and longjmp-resume breakpoints are also meaningless
+       after an exec.  */
+    if (b->type == bp_longjmp || b->type == bp_longjmp_resume)
+      {
+	delete_breakpoint (b);
+	continue;
+      }
+
     /* Don't delete an exec catchpoint, because else the inferior
        won't stop when it ought!
 
@@ -3968,7 +3974,6 @@ set_default_breakpoint (int valid, CORE_
       bp_read_watchpoint
       bp_access_watchpoint
       bp_catch_exec
-      bp_longjmp_resume
       bp_catch_fork
       bp_catch_vork */
 
@@ -3982,7 +3987,6 @@ breakpoint_address_is_meaningful (struct
 	  && type != bp_read_watchpoint
 	  && type != bp_access_watchpoint
 	  && type != bp_catch_exec
-	  && type != bp_longjmp_resume
 	  && type != bp_catch_fork
 	  && type != bp_catch_vfork);
 }
@@ -4349,20 +4353,9 @@ create_longjmp_breakpoint (char *func_na
   struct breakpoint *b;
   struct minimal_symbol *m;
 
-  if (func_name == NULL)
-    b = create_internal_breakpoint (0, bp_longjmp_resume);
-  else
-    {
-      if ((m = lookup_minimal_symbol_text (func_name, NULL)) == NULL)
-	return;
- 
-      b = create_internal_breakpoint (SYMBOL_VALUE_ADDRESS (m), bp_longjmp);
-    }
-
-  b->enable_state = bp_disabled;
-  b->silent = 1;
-  if (func_name)
-    b->addr_string = xstrdup (func_name);
+  if ((m = lookup_minimal_symbol_text (func_name, NULL)) == NULL)
+    return;
+  set_momentary_breakpoint_at_pc (SYMBOL_VALUE_ADDRESS (m), bp_longjmp);
 }
 
 /* Call this routine when stepping and nexting to enable a breakpoint
@@ -4370,30 +4363,31 @@ create_longjmp_breakpoint (char *func_na
    set_longjmp_resume_breakpoint() to figure out where we are going. */
 
 void
-enable_longjmp_breakpoint (void)
+set_longjmp_breakpoint (void)
 {
   struct breakpoint *b;
 
-  ALL_BREAKPOINTS (b)
-    if (b->type == bp_longjmp)
+  if (gdbarch_get_longjmp_target_p (current_gdbarch))
     {
-      b->enable_state = bp_enabled;
-      check_duplicates (b);
+      create_longjmp_breakpoint ("longjmp");
+      create_longjmp_breakpoint ("_longjmp");
+      create_longjmp_breakpoint ("siglongjmp");
+      create_longjmp_breakpoint ("_siglongjmp");
     }
 }
 
+/* Delete all longjmp breakpoints from PTID.  */
 void
-disable_longjmp_breakpoint (void)
+delete_longjmp_breakpoint (int thread)
 {
-  struct breakpoint *b;
+  struct breakpoint *b, *temp;
 
-  ALL_BREAKPOINTS (b)
-    if (b->type == bp_longjmp
-	|| b->type == bp_longjmp_resume)
-    {
-      b->enable_state = bp_disabled;
-      check_duplicates (b);
-    }
+  ALL_BREAKPOINTS_SAFE (b, temp)
+    if (b->type == bp_longjmp)
+      {
+	if (b->thread == thread)
+	  delete_breakpoint (b);
+      }
 }
 
 static void
@@ -4679,30 +4673,6 @@ hw_watchpoint_used_count (enum bptype ty
   return i;
 }
 
-/* Call this after hitting the longjmp() breakpoint.  Use this to set
-   a new breakpoint at the target of the jmp_buf.
-
-   FIXME - This ought to be done by setting a temporary breakpoint
-   that gets deleted automatically... */
-
-void
-set_longjmp_resume_breakpoint (CORE_ADDR pc, struct frame_id frame_id)
-{
-  struct breakpoint *b;
-
-  ALL_BREAKPOINTS (b)
-    if (b->type == bp_longjmp_resume)
-    {
-      b->loc->requested_address = pc;
-      b->loc->address = adjust_breakpoint_address (b->loc->requested_address,
-                                                   b->type);
-      b->enable_state = bp_enabled;
-      b->frame_id = frame_id;
-      check_duplicates (b);
-      return;
-    }
-}
-
 void
 disable_watchpoints_before_interactive_call_start (void)
 {
@@ -4764,6 +4734,19 @@ set_momentary_breakpoint (struct symtab_
 
   return b;
 }
+
+struct breakpoint *
+set_momentary_breakpoint_at_pc (CORE_ADDR pc, enum bptype type)
+{
+  struct symtab_and_line sal;
+
+  sal = find_pc_line (pc, 0);
+  sal.pc = pc;
+  sal.section = find_pc_overlay (pc);
+  sal.explicit_pc = 1;
+
+  return set_momentary_breakpoint (sal, null_frame_id, type);
+}
 \f
 
 /* Tell the user we have just set a breakpoint B.  */
@@ -7423,10 +7406,8 @@ breakpoint_re_set_one (void *bint)
     default:
       printf_filtered (_("Deleting unknown breakpoint type %d\n"), b->type);
       /* fall through */
-      /* Delete longjmp and overlay event breakpoints; they will be
-         reset later by breakpoint_re_set.  */
-    case bp_longjmp:
-    case bp_longjmp_resume:
+      /* Delete overlay event breakpoints; they will be reset later by
+         breakpoint_re_set.  */
     case bp_overlay_event:
       delete_breakpoint (b);
       break;
@@ -7448,6 +7429,8 @@ breakpoint_re_set_one (void *bint)
     case bp_watchpoint_scope:
     case bp_call_dummy:
     case bp_step_resume:
+    case bp_longjmp:
+    case bp_longjmp_resume:
       break;
     }
 
@@ -7475,15 +7458,6 @@ breakpoint_re_set (void)
   }
   set_language (save_language);
   input_radix = save_input_radix;
-
-  if (gdbarch_get_longjmp_target_p (current_gdbarch))
-    {
-      create_longjmp_breakpoint ("longjmp");
-      create_longjmp_breakpoint ("_longjmp");
-      create_longjmp_breakpoint ("siglongjmp");
-      create_longjmp_breakpoint ("_siglongjmp");
-      create_longjmp_breakpoint (NULL);
-    }
   
   create_overlay_event_breakpoint ("_ovly_debug_event");
 }
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-04-07 02:24:30.000000000 +0100
+++ src/gdb/infrun.c	2008-04-07 02:41:01.000000000 +0100
@@ -271,6 +271,7 @@ struct regcache *stop_registers;
 
 static int stop_print_frame;
 
+/* Step-resume or longjmp-resume breakpoint.  */
 static struct breakpoint *step_resume_breakpoint = NULL;
 
 /* This is a cached copy of the pid/waitstatus of the last event
@@ -947,7 +948,6 @@ struct execution_control_state
   struct symtab_and_line sal;
   int current_line;
   struct symtab *current_symtab;
-  int handling_longjmp;		/* FIXME */
   ptid_t ptid;
   ptid_t saved_inferior_ptid;
   int step_after_step_resume_breakpoint;
@@ -969,6 +969,8 @@ static void insert_step_resume_breakpoin
 static void insert_step_resume_breakpoint_at_caller (struct frame_info *);
 static void insert_step_resume_breakpoint_at_sal (struct symtab_and_line sr_sal,
 						  struct frame_id sr_id);
+static void insert_longjmp_resume_breakpoint (CORE_ADDR);
+
 static void stop_stepping (struct execution_control_state *ecs);
 static void prepare_to_wait (struct execution_control_state *ecs);
 static void keep_going (struct execution_control_state *ecs);
@@ -1118,7 +1120,6 @@ init_execution_control_state (struct exe
   ecs->stepping_over_breakpoint = 0;
   ecs->random_signal = 0;
   ecs->step_after_step_resume_breakpoint = 0;
-  ecs->handling_longjmp = 0;	/* FIXME */
   ecs->stepping_through_solib_after_catch = 0;
   ecs->stepping_through_solib_catchpoints = NULL;
   ecs->sal = find_pc_line (prev_pc, 0);
@@ -1173,7 +1174,7 @@ context_switch (struct execution_control
 			 stepping_over_breakpoint, step_resume_breakpoint,
 			 step_range_start,
 			 step_range_end, &step_frame_id,
-			 ecs->handling_longjmp, ecs->stepping_over_breakpoint,
+			 ecs->stepping_over_breakpoint,
 			 ecs->stepping_through_solib_after_catch,
 			 ecs->stepping_through_solib_catchpoints,
 			 ecs->current_line, ecs->current_symtab);
@@ -1183,7 +1184,7 @@ context_switch (struct execution_control
 			 &stepping_over_breakpoint, &step_resume_breakpoint,
 			 &step_range_start,
 			 &step_range_end, &step_frame_id,
-			 &ecs->handling_longjmp, &ecs->stepping_over_breakpoint,
+			 &ecs->stepping_over_breakpoint,
 			 &ecs->stepping_through_solib_after_catch,
 			 &ecs->stepping_through_solib_catchpoints,
 			 &ecs->current_line, &ecs->current_symtab);
@@ -2106,38 +2107,50 @@ process_event_stop_test:
     switch (what.main_action)
       {
       case BPSTAT_WHAT_SET_LONGJMP_RESUME:
-	/* If we hit the breakpoint at longjmp, disable it for the
-	   duration of this command.  Then, install a temporary
-	   breakpoint at the target of the jmp_buf. */
-        if (debug_infrun)
-	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
-	disable_longjmp_breakpoint ();
+	/* If we hit the breakpoint at longjmp while stepping, we
+	   install a momentary breakpoint at the target of the
+	   jmp_buf.  */
+
+	if (debug_infrun)
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
+
+	ecs->stepping_over_breakpoint = 1;
+
 	if (!gdbarch_get_longjmp_target_p (current_gdbarch)
 	    || !gdbarch_get_longjmp_target (current_gdbarch,
 					    get_current_frame (), &jmp_buf_pc))
 	  {
+	    if (debug_infrun)
+	      fprintf_unfiltered (gdb_stdlog, "\
+infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
 	    keep_going (ecs);
 	    return;
 	  }
 
-	/* Need to blow away step-resume breakpoint, as it
-	   interferes with us */
+	/* We're going to replace the current step-resume breakpoint
+	   with a longjmp-resume breakpoint.  */
 	if (step_resume_breakpoint != NULL)
-	  {
-	    delete_step_resume_breakpoint (&step_resume_breakpoint);
-	  }
+	  delete_step_resume_breakpoint (&step_resume_breakpoint);
+
+	/* Insert a breakpoint at resume address.  */
+	insert_longjmp_resume_breakpoint (jmp_buf_pc);
 
-	set_longjmp_resume_breakpoint (jmp_buf_pc, null_frame_id);
-	ecs->handling_longjmp = 1;	/* FIXME */
 	keep_going (ecs);
 	return;
 
       case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
         if (debug_infrun)
-	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
-	disable_longjmp_breakpoint ();
-	ecs->handling_longjmp = 0;	/* FIXME */
-	break;
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
+
+	gdb_assert (step_resume_breakpoint != NULL);
+	delete_step_resume_breakpoint (&step_resume_breakpoint);
+
+	stop_step = 1;
+	print_stop_reason (END_STEPPING_RANGE, 0);
+	stop_stepping (ecs);
+	return;
 
       case BPSTAT_WHAT_SINGLE:
         if (debug_infrun)
@@ -2704,9 +2717,8 @@ process_event_stop_test:
 static int
 currently_stepping (struct execution_control_state *ecs)
 {
-  return ((!ecs->handling_longjmp
-	   && ((step_range_end && step_resume_breakpoint == NULL)
-	       || stepping_over_breakpoint))
+  return (((step_range_end && step_resume_breakpoint == NULL)
+	   || stepping_over_breakpoint)
 	  || ecs->stepping_through_solib_after_catch
 	  || bpstat_should_step ());
 }
@@ -2793,8 +2805,8 @@ static void
 insert_step_resume_breakpoint_at_sal (struct symtab_and_line sr_sal,
 				      struct frame_id sr_id)
 {
-  /* There should never be more than one step-resume breakpoint per
-     thread, so we should never be setting a new
+  /* There should never be more than one step-resume or longjmp-resume
+     breakpoint per thread, so we should never be setting a new
      step_resume_breakpoint when one is already active.  */
   gdb_assert (step_resume_breakpoint == NULL);
 
@@ -2862,6 +2874,28 @@ insert_step_resume_breakpoint_at_caller 
   insert_step_resume_breakpoint_at_sal (sr_sal, frame_unwind_id (next_frame));
 }
 
+/* Insert a "longjmp-resume" breakpoint at PC.  This is used to set a
+   new breakpoint at the target of a jmp_buf.  The handling of
+   longjmp-resume uses the same mechanisms used for handling
+   "step-resume" breakpoints.  */
+
+static void
+insert_longjmp_resume_breakpoint (CORE_ADDR pc)
+{
+  /* There should never be more than one step-resume or longjmp-resume
+     breakpoint per thread, so we should never be setting a new
+     longjmp_resume_breakpoint when one is already active.  */
+  gdb_assert (step_resume_breakpoint == NULL);
+
+  if (debug_infrun)
+    fprintf_unfiltered (gdb_stdlog,
+			"infrun: inserting longjmp-resume breakpoint at 0x%s\n",
+			paddr_nz (pc));
+
+  step_resume_breakpoint =
+    set_momentary_breakpoint_at_pc (pc, bp_longjmp_resume);
+}
+
 static void
 stop_stepping (struct execution_control_state *ecs)
 {
Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h	2008-04-07 02:24:30.000000000 +0100
+++ src/gdb/breakpoint.h	2008-04-07 02:28:24.000000000 +0100
@@ -686,6 +686,9 @@ extern int ep_is_exception_catchpoint (s
 extern struct breakpoint *set_momentary_breakpoint
   (struct symtab_and_line, struct frame_id, enum bptype);
 
+extern struct breakpoint *set_momentary_breakpoint_at_pc
+  (CORE_ADDR pc, enum bptype type);
+
 extern void set_ignore_count (int, int, int);
 
 extern void set_default_breakpoint (int, CORE_ADDR, struct symtab *, int);
@@ -756,12 +759,12 @@ extern void update_breakpoints_after_exe
    inferior_ptid.  */
 extern int detach_breakpoints (int);
 
-extern void enable_longjmp_breakpoint (void);
-extern void disable_longjmp_breakpoint (void);
+extern void set_longjmp_breakpoint (void);
+extern void delete_longjmp_breakpoint (int thread);
+
 extern void enable_overlay_breakpoints (void);
 extern void disable_overlay_breakpoints (void);
 
-extern void set_longjmp_resume_breakpoint (CORE_ADDR, struct frame_id);
 /* These functions respectively disable or reenable all currently
    enabled watchpoints.  When disabled, the watchpoints are marked
    call_disabled.  When reenabled, they are marked enabled.
Index: src/gdb/gdbthread.h
===================================================================
--- src.orig/gdb/gdbthread.h	2008-04-07 02:24:30.000000000 +0100
+++ src/gdb/gdbthread.h	2008-04-07 02:28:24.000000000 +0100
@@ -50,7 +50,6 @@ struct thread_info
   int current_line;
   struct symtab *current_symtab;
   int trap_expected;
-  int handling_longjmp;
   int stepping_over_breakpoint;
 
   /* This is set TRUE when a catchpoint of a shared library event
@@ -123,7 +122,6 @@ extern void save_infrun_state (ptid_t pt
 			       CORE_ADDR step_range_start,
 			       CORE_ADDR step_range_end,
 			       const struct frame_id *step_frame_id,
-			       int       handling_longjmp,
 			       int       another_trap,
 			       int       stepping_through_solib_after_catch,
 			       bpstat    stepping_through_solib_catchpoints,
@@ -139,7 +137,6 @@ extern void load_infrun_state (ptid_t pt
 			       CORE_ADDR *step_range_start,
 			       CORE_ADDR *step_range_end,
 			       struct frame_id *step_frame_id,
-			       int       *handling_longjmp,
 			       int       *another_trap,
 			       int       *stepping_through_solib_affter_catch,
 			       bpstat    *stepping_through_solib_catchpoints,
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c	2008-04-07 02:24:30.000000000 +0100
+++ src/gdb/thread.c	2008-04-07 02:28:24.000000000 +0100
@@ -316,7 +316,6 @@ load_infrun_state (ptid_t ptid,
 		   CORE_ADDR *step_range_start,
 		   CORE_ADDR *step_range_end,
 		   struct frame_id *step_frame_id,
-		   int *handling_longjmp,
 		   int *stepping_over_breakpoint,
 		   int *stepping_through_solib_after_catch,
 		   bpstat *stepping_through_solib_catchpoints,
@@ -337,7 +336,6 @@ load_infrun_state (ptid_t ptid,
   *step_range_start = tp->step_range_start;
   *step_range_end = tp->step_range_end;
   *step_frame_id = tp->step_frame_id;
-  *handling_longjmp = tp->handling_longjmp;
   *stepping_over_breakpoint = tp->stepping_over_breakpoint;
   *stepping_through_solib_after_catch =
     tp->stepping_through_solib_after_catch;
@@ -357,7 +355,6 @@ save_infrun_state (ptid_t ptid,
 		   CORE_ADDR step_range_start,
 		   CORE_ADDR step_range_end,
 		   const struct frame_id *step_frame_id,
-		   int handling_longjmp,
 		   int stepping_over_breakpoint,
 		   int stepping_through_solib_after_catch,
 		   bpstat stepping_through_solib_catchpoints,
@@ -378,7 +375,6 @@ save_infrun_state (ptid_t ptid,
   tp->step_range_start = step_range_start;
   tp->step_range_end = step_range_end;
   tp->step_frame_id = (*step_frame_id);
-  tp->handling_longjmp = handling_longjmp;
   tp->stepping_over_breakpoint = stepping_over_breakpoint;
   tp->stepping_through_solib_after_catch = stepping_through_solib_after_catch;
   tp->stepping_through_solib_catchpoints = stepping_through_solib_catchpoints;
Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c	2008-04-07 02:24:30.000000000 +0100
+++ src/gdb/infcmd.c	2008-04-07 02:28:24.000000000 +0100
@@ -49,6 +49,7 @@
 #include "target-descriptions.h"
 #include "user-regs.h"
 #include "exceptions.h"
+#include "gdbthread.h"
 
 /* Functions exported for general use, in inferior.h: */
 
@@ -689,9 +690,11 @@ nexti_command (char *count_string, int f
 }
 
 static void
-disable_longjmp_breakpoint_cleanup (void *ignore)
+delete_longjmp_breakpoint_cleanup (void *arg)
 {
-  disable_longjmp_breakpoint ();
+  int thread = * (int *) arg;
+  xfree (arg);
+  delete_longjmp_breakpoint (thread);
 }
 
 static void
@@ -724,11 +727,24 @@ step_1 (int skip_subroutines, int single
 
   if (!single_inst || skip_subroutines)		/* leave si command alone */
     {
-      enable_longjmp_breakpoint ();
+      struct cleanup *old_chain;
+      int *thread;
+
+      thread = xmalloc (sizeof (int));
+      old_chain = make_cleanup (xfree, thread);
+
+      if (in_thread_list (inferior_ptid))
+	*thread = pid_to_thread_id (inferior_ptid);
+      else
+	*thread = -1;
+
+      set_longjmp_breakpoint ();
+
+      discard_cleanups (old_chain);
       if (!target_can_async_p ())
-	cleanups = make_cleanup (disable_longjmp_breakpoint_cleanup, 0 /*ignore*/);
+	cleanups = make_cleanup (delete_longjmp_breakpoint_cleanup, thread);
       else
-        make_exec_cleanup (disable_longjmp_breakpoint_cleanup, 0 /*ignore*/);
+        make_exec_cleanup (delete_longjmp_breakpoint_cleanup, thread);
     }
 
   /* In synchronous case, all is well, just use the regular for loop. */
Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2008-04-07 02:28:11.000000000 +0100
+++ src/gdb/Makefile.in	2008-04-07 02:41:05.000000000 +0100
@@ -2291,7 +2291,7 @@ infcmd.o: infcmd.c $(defs_h) $(gdb_strin
 	$(objfiles_h) $(completer_h) $(ui_out_h) $(event_top_h) \
 	$(parser_defs_h) $(regcache_h) $(reggroups_h) $(block_h) \
 	$(solib_h) $(gdb_assert_h) $(observer_h) $(target_descriptions_h) \
-	$(user_regs_h) $(exceptions_h)
+	$(user_regs_h) $(exceptions_h) $(gdbthread_h)
 inf-loop.o: inf-loop.c $(defs_h) $(inferior_h) $(target_h) $(event_loop_h) \
 	$(event_top_h) $(inf_loop_h) $(remote_h) $(exceptions_h) \
 	$(language_h)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: 3/5 - Rework stepping over longjmp support
  2008-04-07  2:34 3/5 - Rework stepping over longjmp support Pedro Alves
@ 2008-04-07 19:43 ` Tom Tromey
  2008-04-14 18:57 ` Daniel Jacobowitz
  1 sibling, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2008-04-07 19:43 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> The longjmp inside hidden_longjmp is going to land inside
Pedro> hidden_longjmp.  GDB could ignore it leave the step-resume
Pedro> breakpoint at the return of hidden_longjmp and pretend
Pedro> that longjmp was never seen.  Think of stepping over a function
Pedro> in gdb's sources, and an exception being thrown and caught all
Pedro> somewhere inner to the function you're stepping.  I've
Pedro> implemented a prototype patch that does this.  Does anyone
Pedro> else think that behaviour is useful?

I do.  Speaking as a user, this is the behavior I expect.  That is, if
I 'next' over a function call (and assuming the "easy" case -- no user
breakpoints or watchpoints or the like), anything that happens during
that function call should be invisible to me.  And, in my view, the
inferior should stop at the "next" instruction after the function
call; if that is in an outer frame, then I want to stop there.

Ideally, of course, all this should happen for exceptions in c++ and
java as well :-).  FWIW the c++ case is more important to me ... but
I'm rather fascinated by this patch series because I didn't know this
code even existed, and it makes handling exceptions seem more
possible.

Pedro> (I'm aware that any
Pedro> smartness we add can be defeated by longjmp changing stacks,
Pedro> or stuff like debugging coroutines implemented with set/longjmp,
Pedro> but those feel like rare enough that a smart mode could be
Pedro> the default and useful most of (almost-all) the times.)

As long as folks doing unusual stuff like this can cope for
themselves, I think your proposed behavior is preferable.

Tom


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: 3/5 - Rework stepping over longjmp support
  2008-04-07  2:34 3/5 - Rework stepping over longjmp support Pedro Alves
  2008-04-07 19:43 ` Tom Tromey
@ 2008-04-14 18:57 ` Daniel Jacobowitz
  2008-04-25 18:29   ` Pedro Alves
  1 sibling, 1 reply; 7+ messages in thread
From: Daniel Jacobowitz @ 2008-04-14 18:57 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Mon, Apr 07, 2008 at 03:31:14AM +0100, Pedro Alves wrote:
> 2008-04-07  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* breakpoint.c (update_breakpoints_after_exec): Delete bp_longjmp
> 	and bp_longjmp_resume breakpoints.

...

OK, thanks.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: 3/5 - Rework stepping over longjmp support
  2008-04-14 18:57 ` Daniel Jacobowitz
@ 2008-04-25 18:29   ` Pedro Alves
  2008-05-02 14:41     ` Daniel Jacobowitz
  0 siblings, 1 reply; 7+ messages in thread
From: Pedro Alves @ 2008-04-25 18:29 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 823 bytes --]

A Monday 14 April 2008 19:41:06, Daniel Jacobowitz wrote:
> On Mon, Apr 07, 2008 at 03:31:14AM +0100, Pedro Alves wrote:
> > 2008-04-07  Pedro Alves  <pedro@codesourcery.com>
> >
> > 	* breakpoint.c (update_breakpoints_after_exec): Delete bp_longjmp
> > 	and bp_longjmp_resume breakpoints.
>
> ...
>
> OK, thanks.

I've readjusted the patch now that the always-inserted breakpoints
work is in, and the exec-cleanup infrastructure is gone.

Most of the changes are mechanic.  One addition I've done is that
I saw one XFAIL -> FAIL in the annota2.exp test, and I thought
it was sporadic but is not.  The longjmp breakpoint inserting/removing
changes the places the breakpoints-invalid annotation is emitted so the
test needs fixing up.

Still OK?  I'll check the new longjmp.exp test in along with this one.

-- 
Pedro Alves

[-- Attachment #2: longjmp_rewrite.diff --]
[-- Type: text/x-diff, Size: 27274 bytes --]

2008-04-25  Pedro Alves  <pedro@codesourcery.com>

	gdb/

	* breakpoint.c (update_breakpoints_after_exec): Delete bp_longjmp
	and bp_longjmp_resume breakpoints.
	(breakpoint_address_is_meaningful): Claim bp_longjmp_resume as
	meaningful.
	(create_longjmp_breakpoint): Don't create bp_longjmp_resume
	breakpoints.  Create bp_longjmp breakpoints as momentary
	breakpoints.
	(enable_longjmp_breakpoint): Delete.
	(set_longjmp_breakpoint): New.
	(disable_longjmp_breakpoint): Delete.
	(delete_longjmp_breakpoint): New.
	(set_longjmp_resume_breakpoint): Delete.
	(set_momentary_breakpoint_at_pc): New.
	(breakpoint_re_set_one): Don't delete bp_longjmp and
	bp_longjmp_resume breakpoints.
	(breakpoint_re_set): Don't create longjmp and longjmp-resume
	breakpoints.

	* infrun.c (step_resume_breakpoint): Add comment.
	(struct execution_control_state): Delete handling_longjmp member.
	(init_execution_control_state). Don't clear handling_longjmp.
	(context_switch): Don't context switch handling_longjmp.
	(handle_inferior_event): If handling a bp_longjmp breakpoint,
	create a bp_longjmp_resume breakpoint, and set it as current
	step_resume_breakpoint, then step over the longjmp breakpoint.  If
	handling a bp_longjmp_resume breakpoint, don't delete the longjmp
	breakpoint, delete the longjmp-resume breakpoint, and stop
	stepping.
	(currently_stepping): Remove handling_longjmp from expression.
	(insert_step_resume_breakpoint_at_sal): Update comment.
	(insert_longjmp_resume_breakpoint): New.

	* breakpoint.h (set_momentary_breakpoint_at_pc): Declare.
	(enable_longjmp_breakpoint, disable_longjmp_breakpoint): Delete
	declarations.
	(set_longjmp_breakpoint, delete_longjmp_breakpoint): Declare.
	(set_longjmp_resume_breakpoint): Delete declaration.

	* gdbthread.h (save_infrun_state): Remove handling_longjmp parameter.
	(load_infrun_state): Delete *handling_longjmp parameter.
	* thread.c (save_infrun_state): Remove handling_longjmp parameter.  Update body.
	(load_infrun_state): Delete *handling_longjmp parameter.  Update body.

	* infcmd.c Include "gdbthread.h".
	(disable_longjmp_breakpoint_cleanup): Delete.
	(delete_longjmp_breakpoint_cleanup): New.
	(step_1): Call set_longjmp_breakpoint instead of
	enable_longjmp_breakpoint.  Use delete_longjmp_breakpoint_cleanup
	instead of disable_longjmp_breakpoint_cleanup when making cleanup.
	(step_1_continuation): Pass thread id in the continuation args to
	step_once.
	(step_once): Add thread parameter.  Pass thread id the the
	continuation.

	* Makefile.in (infcmd.o): Update.

	gdb/testsuite:
	* gdb.cp/annota2.exp: Adjust to breakpoints invalidations at
	different times.

---
 gdb/Makefile.in                  |    2 
 gdb/breakpoint.c                 |  112 ++++++++++++++-------------------------
 gdb/breakpoint.h                 |    9 ++-
 gdb/gdbthread.h                  |    3 -
 gdb/infcmd.c                     |   42 ++++++++++----
 gdb/infrun.c                     |   86 ++++++++++++++++++++---------
 gdb/testsuite/gdb.cp/annota2.exp |    4 -
 gdb/thread.c                     |    4 -
 8 files changed, 143 insertions(+), 119 deletions(-)

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2008-04-25 14:54:13.000000000 +0100
+++ src/gdb/breakpoint.c	2008-04-25 14:54:13.000000000 +0100
@@ -152,8 +152,6 @@ static int watchpoint_check (void *);
 
 static void maintenance_info_breakpoints (char *, int);
 
-static void create_longjmp_breakpoint (char *);
-
 static void create_overlay_event_breakpoint (char *);
 
 static int hw_breakpoint_used_count (void);
@@ -1499,6 +1497,14 @@ update_breakpoints_after_exec (void)
 	continue;
       }
 
+    /* Longjmp and longjmp-resume breakpoints are also meaningless
+       after an exec.  */
+    if (b->type == bp_longjmp || b->type == bp_longjmp_resume)
+      {
+	delete_breakpoint (b);
+	continue;
+      }
+
     /* Don't delete an exec catchpoint, because else the inferior
        won't stop when it ought!
 
@@ -4093,7 +4099,6 @@ set_default_breakpoint (int valid, CORE_
       bp_read_watchpoint
       bp_access_watchpoint
       bp_catch_exec
-      bp_longjmp_resume
       bp_catch_fork
       bp_catch_vork */
 
@@ -4107,7 +4112,6 @@ breakpoint_address_is_meaningful (struct
 	  && type != bp_read_watchpoint
 	  && type != bp_access_watchpoint
 	  && type != bp_catch_exec
-	  && type != bp_longjmp_resume
 	  && type != bp_catch_fork
 	  && type != bp_catch_vfork);
 }
@@ -4465,20 +4469,9 @@ create_longjmp_breakpoint (char *func_na
   struct breakpoint *b;
   struct minimal_symbol *m;
 
-  if (func_name == NULL)
-    b = create_internal_breakpoint (0, bp_longjmp_resume);
-  else
-    {
-      if ((m = lookup_minimal_symbol_text (func_name, NULL)) == NULL)
-	return;
- 
-      b = create_internal_breakpoint (SYMBOL_VALUE_ADDRESS (m), bp_longjmp);
-    }
-
-  b->enable_state = bp_disabled;
-  b->silent = 1;
-  if (func_name)
-    b->addr_string = xstrdup (func_name);
+  if ((m = lookup_minimal_symbol_text (func_name, NULL)) == NULL)
+    return;
+  set_momentary_breakpoint_at_pc (SYMBOL_VALUE_ADDRESS (m), bp_longjmp);
   update_global_location_list ();
 }
 
@@ -4487,30 +4480,31 @@ create_longjmp_breakpoint (char *func_na
    set_longjmp_resume_breakpoint() to figure out where we are going. */
 
 void
-enable_longjmp_breakpoint (void)
+set_longjmp_breakpoint (void)
 {
   struct breakpoint *b;
 
-  ALL_BREAKPOINTS (b)
-    if (b->type == bp_longjmp)
+  if (gdbarch_get_longjmp_target_p (current_gdbarch))
     {
-      b->enable_state = bp_enabled;
-      update_global_location_list ();
+      create_longjmp_breakpoint ("longjmp");
+      create_longjmp_breakpoint ("_longjmp");
+      create_longjmp_breakpoint ("siglongjmp");
+      create_longjmp_breakpoint ("_siglongjmp");
     }
 }
 
+/* Delete all longjmp breakpoints from THREAD.  */
 void
-disable_longjmp_breakpoint (void)
+delete_longjmp_breakpoint (int thread)
 {
-  struct breakpoint *b;
+  struct breakpoint *b, *temp;
 
-  ALL_BREAKPOINTS (b)
-    if (b->type == bp_longjmp
-	|| b->type == bp_longjmp_resume)
-    {
-      b->enable_state = bp_disabled;
-      update_global_location_list ();
-    }
+  ALL_BREAKPOINTS_SAFE (b, temp)
+    if (b->type == bp_longjmp)
+      {
+	if (b->thread == thread)
+	  delete_breakpoint (b);
+      }
 }
 
 static void
@@ -4803,30 +4797,6 @@ hw_watchpoint_used_count (enum bptype ty
   return i;
 }
 
-/* Call this after hitting the longjmp() breakpoint.  Use this to set
-   a new breakpoint at the target of the jmp_buf.
-
-   FIXME - This ought to be done by setting a temporary breakpoint
-   that gets deleted automatically... */
-
-void
-set_longjmp_resume_breakpoint (CORE_ADDR pc, struct frame_id frame_id)
-{
-  struct breakpoint *b;
-
-  ALL_BREAKPOINTS (b)
-    if (b->type == bp_longjmp_resume)
-    {
-      b->loc->requested_address = pc;
-      b->loc->address = adjust_breakpoint_address (b->loc->requested_address,
-                                                   b->type);
-      b->enable_state = bp_enabled;
-      b->frame_id = frame_id;
-      update_global_location_list ();
-      return;
-    }
-}
-
 void
 disable_watchpoints_before_interactive_call_start (void)
 {
@@ -4890,6 +4860,19 @@ set_momentary_breakpoint (struct symtab_
 
   return b;
 }
+
+struct breakpoint *
+set_momentary_breakpoint_at_pc (CORE_ADDR pc, enum bptype type)
+{
+  struct symtab_and_line sal;
+
+  sal = find_pc_line (pc, 0);
+  sal.pc = pc;
+  sal.section = find_pc_overlay (pc);
+  sal.explicit_pc = 1;
+
+  return set_momentary_breakpoint (sal, null_frame_id, type);
+}
 \f
 
 /* Tell the user we have just set a breakpoint B.  */
@@ -7547,10 +7530,8 @@ breakpoint_re_set_one (void *bint)
     default:
       printf_filtered (_("Deleting unknown breakpoint type %d\n"), b->type);
       /* fall through */
-      /* Delete longjmp and overlay event breakpoints; they will be
-         reset later by breakpoint_re_set.  */
-    case bp_longjmp:
-    case bp_longjmp_resume:
+      /* Delete overlay event breakpoints; they will be reset later by
+         breakpoint_re_set.  */
     case bp_overlay_event:
       delete_breakpoint (b);
       break;
@@ -7572,6 +7553,8 @@ breakpoint_re_set_one (void *bint)
     case bp_watchpoint_scope:
     case bp_call_dummy:
     case bp_step_resume:
+    case bp_longjmp:
+    case bp_longjmp_resume:
       break;
     }
 
@@ -7599,15 +7582,6 @@ breakpoint_re_set (void)
   }
   set_language (save_language);
   input_radix = save_input_radix;
-
-  if (gdbarch_get_longjmp_target_p (current_gdbarch))
-    {
-      create_longjmp_breakpoint ("longjmp");
-      create_longjmp_breakpoint ("_longjmp");
-      create_longjmp_breakpoint ("siglongjmp");
-      create_longjmp_breakpoint ("_siglongjmp");
-      create_longjmp_breakpoint (NULL);
-    }
   
   create_overlay_event_breakpoint ("_ovly_debug_event");
 }
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-04-25 14:54:13.000000000 +0100
+++ src/gdb/infrun.c	2008-04-25 14:54:13.000000000 +0100
@@ -271,6 +271,7 @@ struct regcache *stop_registers;
 
 static int stop_print_frame;
 
+/* Step-resume or longjmp-resume breakpoint.  */
 static struct breakpoint *step_resume_breakpoint = NULL;
 
 /* This is a cached copy of the pid/waitstatus of the last event
@@ -954,7 +955,6 @@ struct execution_control_state
   struct symtab_and_line sal;
   int current_line;
   struct symtab *current_symtab;
-  int handling_longjmp;		/* FIXME */
   ptid_t ptid;
   ptid_t saved_inferior_ptid;
   int step_after_step_resume_breakpoint;
@@ -976,6 +976,8 @@ static void insert_step_resume_breakpoin
 static void insert_step_resume_breakpoint_at_caller (struct frame_info *);
 static void insert_step_resume_breakpoint_at_sal (struct symtab_and_line sr_sal,
 						  struct frame_id sr_id);
+static void insert_longjmp_resume_breakpoint (CORE_ADDR);
+
 static void stop_stepping (struct execution_control_state *ecs);
 static void prepare_to_wait (struct execution_control_state *ecs);
 static void keep_going (struct execution_control_state *ecs);
@@ -1120,7 +1122,6 @@ init_execution_control_state (struct exe
   ecs->stepping_over_breakpoint = 0;
   ecs->random_signal = 0;
   ecs->step_after_step_resume_breakpoint = 0;
-  ecs->handling_longjmp = 0;	/* FIXME */
   ecs->stepping_through_solib_after_catch = 0;
   ecs->stepping_through_solib_catchpoints = NULL;
   ecs->sal = find_pc_line (prev_pc, 0);
@@ -1175,7 +1176,7 @@ context_switch (struct execution_control
 			 stepping_over_breakpoint, step_resume_breakpoint,
 			 step_range_start,
 			 step_range_end, &step_frame_id,
-			 ecs->handling_longjmp, ecs->stepping_over_breakpoint,
+			 ecs->stepping_over_breakpoint,
 			 ecs->stepping_through_solib_after_catch,
 			 ecs->stepping_through_solib_catchpoints,
 			 ecs->current_line, ecs->current_symtab);
@@ -1185,7 +1186,7 @@ context_switch (struct execution_control
 			 &stepping_over_breakpoint, &step_resume_breakpoint,
 			 &step_range_start,
 			 &step_range_end, &step_frame_id,
-			 &ecs->handling_longjmp, &ecs->stepping_over_breakpoint,
+			 &ecs->stepping_over_breakpoint,
 			 &ecs->stepping_through_solib_after_catch,
 			 &ecs->stepping_through_solib_catchpoints,
 			 &ecs->current_line, &ecs->current_symtab);
@@ -2104,38 +2105,50 @@ process_event_stop_test:
     switch (what.main_action)
       {
       case BPSTAT_WHAT_SET_LONGJMP_RESUME:
-	/* If we hit the breakpoint at longjmp, disable it for the
-	   duration of this command.  Then, install a temporary
-	   breakpoint at the target of the jmp_buf. */
-        if (debug_infrun)
-	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
-	disable_longjmp_breakpoint ();
+	/* If we hit the breakpoint at longjmp while stepping, we
+	   install a momentary breakpoint at the target of the
+	   jmp_buf.  */
+
+	if (debug_infrun)
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
+
+	ecs->stepping_over_breakpoint = 1;
+
 	if (!gdbarch_get_longjmp_target_p (current_gdbarch)
 	    || !gdbarch_get_longjmp_target (current_gdbarch,
 					    get_current_frame (), &jmp_buf_pc))
 	  {
+	    if (debug_infrun)
+	      fprintf_unfiltered (gdb_stdlog, "\
+infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
 	    keep_going (ecs);
 	    return;
 	  }
 
-	/* Need to blow away step-resume breakpoint, as it
-	   interferes with us */
+	/* We're going to replace the current step-resume breakpoint
+	   with a longjmp-resume breakpoint.  */
 	if (step_resume_breakpoint != NULL)
-	  {
-	    delete_step_resume_breakpoint (&step_resume_breakpoint);
-	  }
+	  delete_step_resume_breakpoint (&step_resume_breakpoint);
+
+	/* Insert a breakpoint at resume address.  */
+	insert_longjmp_resume_breakpoint (jmp_buf_pc);
 
-	set_longjmp_resume_breakpoint (jmp_buf_pc, null_frame_id);
-	ecs->handling_longjmp = 1;	/* FIXME */
 	keep_going (ecs);
 	return;
 
       case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
         if (debug_infrun)
-	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
-	disable_longjmp_breakpoint ();
-	ecs->handling_longjmp = 0;	/* FIXME */
-	break;
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
+
+	gdb_assert (step_resume_breakpoint != NULL);
+	delete_step_resume_breakpoint (&step_resume_breakpoint);
+
+	stop_step = 1;
+	print_stop_reason (END_STEPPING_RANGE, 0);
+	stop_stepping (ecs);
+	return;
 
       case BPSTAT_WHAT_SINGLE:
         if (debug_infrun)
@@ -2698,9 +2711,8 @@ process_event_stop_test:
 static int
 currently_stepping (struct execution_control_state *ecs)
 {
-  return ((!ecs->handling_longjmp
-	   && ((step_range_end && step_resume_breakpoint == NULL)
-	       || stepping_over_breakpoint))
+  return (((step_range_end && step_resume_breakpoint == NULL)
+	   || stepping_over_breakpoint)
 	  || ecs->stepping_through_solib_after_catch
 	  || bpstat_should_step ());
 }
@@ -2787,8 +2799,8 @@ static void
 insert_step_resume_breakpoint_at_sal (struct symtab_and_line sr_sal,
 				      struct frame_id sr_id)
 {
-  /* There should never be more than one step-resume breakpoint per
-     thread, so we should never be setting a new
+  /* There should never be more than one step-resume or longjmp-resume
+     breakpoint per thread, so we should never be setting a new
      step_resume_breakpoint when one is already active.  */
   gdb_assert (step_resume_breakpoint == NULL);
 
@@ -2856,6 +2868,28 @@ insert_step_resume_breakpoint_at_caller 
   insert_step_resume_breakpoint_at_sal (sr_sal, frame_unwind_id (next_frame));
 }
 
+/* Insert a "longjmp-resume" breakpoint at PC.  This is used to set a
+   new breakpoint at the target of a jmp_buf.  The handling of
+   longjmp-resume uses the same mechanisms used for handling
+   "step-resume" breakpoints.  */
+
+static void
+insert_longjmp_resume_breakpoint (CORE_ADDR pc)
+{
+  /* There should never be more than one step-resume or longjmp-resume
+     breakpoint per thread, so we should never be setting a new
+     longjmp_resume_breakpoint when one is already active.  */
+  gdb_assert (step_resume_breakpoint == NULL);
+
+  if (debug_infrun)
+    fprintf_unfiltered (gdb_stdlog,
+			"infrun: inserting longjmp-resume breakpoint at 0x%s\n",
+			paddr_nz (pc));
+
+  step_resume_breakpoint =
+    set_momentary_breakpoint_at_pc (pc, bp_longjmp_resume);
+}
+
 static void
 stop_stepping (struct execution_control_state *ecs)
 {
Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h	2008-04-25 14:54:13.000000000 +0100
+++ src/gdb/breakpoint.h	2008-04-25 14:54:13.000000000 +0100
@@ -689,6 +689,9 @@ extern int ep_is_exception_catchpoint (s
 extern struct breakpoint *set_momentary_breakpoint
   (struct symtab_and_line, struct frame_id, enum bptype);
 
+extern struct breakpoint *set_momentary_breakpoint_at_pc
+  (CORE_ADDR pc, enum bptype type);
+
 extern void set_ignore_count (int, int, int);
 
 extern void set_default_breakpoint (int, CORE_ADDR, struct symtab *, int);
@@ -757,12 +760,12 @@ extern void update_breakpoints_after_exe
    inferior_ptid.  */
 extern int detach_breakpoints (int);
 
-extern void enable_longjmp_breakpoint (void);
-extern void disable_longjmp_breakpoint (void);
+extern void set_longjmp_breakpoint (void);
+extern void delete_longjmp_breakpoint (int thread);
+
 extern void enable_overlay_breakpoints (void);
 extern void disable_overlay_breakpoints (void);
 
-extern void set_longjmp_resume_breakpoint (CORE_ADDR, struct frame_id);
 /* These functions respectively disable or reenable all currently
    enabled watchpoints.  When disabled, the watchpoints are marked
    call_disabled.  When reenabled, they are marked enabled.
Index: src/gdb/gdbthread.h
===================================================================
--- src.orig/gdb/gdbthread.h	2008-04-25 14:50:20.000000000 +0100
+++ src/gdb/gdbthread.h	2008-04-25 14:54:13.000000000 +0100
@@ -50,7 +50,6 @@ struct thread_info
   int current_line;
   struct symtab *current_symtab;
   int trap_expected;
-  int handling_longjmp;
   int stepping_over_breakpoint;
 
   /* This is set TRUE when a catchpoint of a shared library event
@@ -123,7 +122,6 @@ extern void save_infrun_state (ptid_t pt
 			       CORE_ADDR step_range_start,
 			       CORE_ADDR step_range_end,
 			       const struct frame_id *step_frame_id,
-			       int       handling_longjmp,
 			       int       another_trap,
 			       int       stepping_through_solib_after_catch,
 			       bpstat    stepping_through_solib_catchpoints,
@@ -139,7 +137,6 @@ extern void load_infrun_state (ptid_t pt
 			       CORE_ADDR *step_range_start,
 			       CORE_ADDR *step_range_end,
 			       struct frame_id *step_frame_id,
-			       int       *handling_longjmp,
 			       int       *another_trap,
 			       int       *stepping_through_solib_affter_catch,
 			       bpstat    *stepping_through_solib_catchpoints,
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c	2008-04-25 14:50:20.000000000 +0100
+++ src/gdb/thread.c	2008-04-25 14:54:13.000000000 +0100
@@ -316,7 +316,6 @@ load_infrun_state (ptid_t ptid,
 		   CORE_ADDR *step_range_start,
 		   CORE_ADDR *step_range_end,
 		   struct frame_id *step_frame_id,
-		   int *handling_longjmp,
 		   int *stepping_over_breakpoint,
 		   int *stepping_through_solib_after_catch,
 		   bpstat *stepping_through_solib_catchpoints,
@@ -337,7 +336,6 @@ load_infrun_state (ptid_t ptid,
   *step_range_start = tp->step_range_start;
   *step_range_end = tp->step_range_end;
   *step_frame_id = tp->step_frame_id;
-  *handling_longjmp = tp->handling_longjmp;
   *stepping_over_breakpoint = tp->stepping_over_breakpoint;
   *stepping_through_solib_after_catch =
     tp->stepping_through_solib_after_catch;
@@ -357,7 +355,6 @@ save_infrun_state (ptid_t ptid,
 		   CORE_ADDR step_range_start,
 		   CORE_ADDR step_range_end,
 		   const struct frame_id *step_frame_id,
-		   int handling_longjmp,
 		   int stepping_over_breakpoint,
 		   int stepping_through_solib_after_catch,
 		   bpstat stepping_through_solib_catchpoints,
@@ -378,7 +375,6 @@ save_infrun_state (ptid_t ptid,
   tp->step_range_start = step_range_start;
   tp->step_range_end = step_range_end;
   tp->step_frame_id = (*step_frame_id);
-  tp->handling_longjmp = handling_longjmp;
   tp->stepping_over_breakpoint = stepping_over_breakpoint;
   tp->stepping_through_solib_after_catch = stepping_through_solib_after_catch;
   tp->stepping_through_solib_catchpoints = stepping_through_solib_catchpoints;
Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c	2008-04-25 14:50:20.000000000 +0100
+++ src/gdb/infcmd.c	2008-04-25 14:54:13.000000000 +0100
@@ -50,6 +50,7 @@
 #include "user-regs.h"
 #include "exceptions.h"
 #include "cli/cli-decode.h"
+#include "gdbthread.h"
 
 /* Functions exported for general use, in inferior.h: */
 
@@ -106,7 +107,7 @@ static void signal_command (char *, int)
 static void jump_command (char *, int);
 
 static void step_1 (int, int, char *);
-static void step_once (int skip_subroutines, int single_inst, int count);
+static void step_once (int skip_subroutines, int single_inst, int count, int thread);
 static void step_1_continuation (struct continuation_arg *arg, int error_p);
 
 static void next_command (char *, int);
@@ -692,9 +693,10 @@ nexti_command (char *count_string, int f
 }
 
 static void
-disable_longjmp_breakpoint_cleanup (void *ignore)
+delete_longjmp_breakpoint_cleanup (void *arg)
 {
-  disable_longjmp_breakpoint ();
+  int thread = * (int *) arg;
+  delete_longjmp_breakpoint (thread);
 }
 
 static void
@@ -704,6 +706,7 @@ step_1 (int skip_subroutines, int single
   struct frame_info *frame;
   struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
   int async_exec = 0;
+  int *thread_p = NULL;
 
   ERROR_NO_INFERIOR;
 
@@ -727,8 +730,17 @@ step_1 (int skip_subroutines, int single
 
   if (!single_inst || skip_subroutines)		/* leave si command alone */
     {
-      enable_longjmp_breakpoint ();
-      make_cleanup (disable_longjmp_breakpoint_cleanup, 0 /*ignore*/);
+      thread_p = xmalloc (sizeof (int));
+      make_cleanup (xfree, thread_p);
+
+      if (in_thread_list (inferior_ptid))
+ 	*thread_p = pid_to_thread_id (inferior_ptid);
+      else
+ 	*thread_p = -1;
+
+      set_longjmp_breakpoint ();
+
+      make_cleanup (delete_longjmp_breakpoint_cleanup, thread_p);
     }
 
   /* In synchronous case, all is well, just use the regular for loop. */
@@ -789,10 +801,11 @@ which has no line number information.\n"
      and handle them one at the time, through step_once(). */
   else
     {
-      step_once (skip_subroutines, single_inst, count);
-      /* We are running, and the contination is installed.  It will
+      step_once (skip_subroutines, single_inst, count, *thread_p);
+      /* We are running, and the continuation is installed.  It will
 	 disable the longjmp breakpoint as appropriate.  */
       discard_cleanups (cleanups);
+      xfree (thread_p);
     }
 }
 
@@ -807,10 +820,12 @@ step_1_continuation (struct continuation
   int count;
   int skip_subroutines;
   int single_inst;
+  int thread;
       
   skip_subroutines = arg->data.integer;
   single_inst      = arg->next->data.integer;
   count            = arg->next->next->data.integer;
+  thread           = arg->next->next->next->data.integer;
 
   if (error_p || !step_multi || !stop_step)
     {
@@ -818,11 +833,11 @@ step_1_continuation (struct continuation
 	 that is not stepping, or there are no further steps
 	 to make.  Cleanup.  */
       if (!single_inst || skip_subroutines)
-	disable_longjmp_breakpoint ();
+	delete_longjmp_breakpoint (thread);
       step_multi = 0;
     }
   else
-    step_once (skip_subroutines, single_inst, count - 1);
+    step_once (skip_subroutines, single_inst, count - 1, thread);
 }
 
 /* Do just one step operation. If count >1 we will have to set up a
@@ -833,11 +848,12 @@ step_1_continuation (struct continuation
    called in case of step n with n>1, after the first step operation has
    been completed.*/
 static void 
-step_once (int skip_subroutines, int single_inst, int count)
+step_once (int skip_subroutines, int single_inst, int count, int thread)
 { 
   struct continuation_arg *arg1; 
   struct continuation_arg *arg2;
   struct continuation_arg *arg3; 
+  struct continuation_arg *arg4;
   struct frame_info *frame;
 
   if (count > 0)
@@ -893,12 +909,16 @@ which has no line number information.\n"
 	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
       arg3 =
 	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
+      arg4 =
+	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
       arg1->next = arg2;
       arg1->data.integer = skip_subroutines;
       arg2->next = arg3;
       arg2->data.integer = single_inst;
-      arg3->next = NULL;
+      arg3->next = arg4;
       arg3->data.integer = count;
+      arg4->next = NULL;
+      arg4->data.integer = thread;
       add_intermediate_continuation (step_1_continuation, arg1);
     }
 }
Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2008-04-25 14:50:20.000000000 +0100
+++ src/gdb/Makefile.in	2008-04-25 15:27:19.000000000 +0100
@@ -2315,7 +2315,7 @@ infcmd.o: infcmd.c $(defs_h) $(gdb_strin
 	$(objfiles_h) $(completer_h) $(ui_out_h) $(event_top_h) \
 	$(parser_defs_h) $(regcache_h) $(reggroups_h) $(block_h) \
 	$(solib_h) $(gdb_assert_h) $(observer_h) $(target_descriptions_h) \
-	$(user_regs_h) $(exceptions_h) $(cli_decode_h)
+	$(user_regs_h) $(exceptions_h) $(cli_decode_h) $(gdbthread_h)
 inf-loop.o: inf-loop.c $(defs_h) $(inferior_h) $(target_h) $(event_loop_h) \
 	$(event_top_h) $(inf_loop_h) $(remote_h) $(exceptions_h) \
 	$(language_h)
Index: src/gdb/testsuite/gdb.cp/annota2.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.cp/annota2.exp	2008-04-25 15:27:33.000000000 +0100
+++ src/gdb/testsuite/gdb.cp/annota2.exp	2008-04-25 15:28:04.000000000 +0100
@@ -190,9 +190,9 @@ gdb_expect {
 #
 send_gdb "next\n"
 gdb_expect {
-   -re "\r\n\032\032post-prompt\r\n\r\n\032\032starting\r\n\r\n(\032\032frames-invalid\r\n\r\n)*\032\032watchpoint 3\r\n.*atchpoint 3: a.x\r\n\r\nOld value = 0\r\nNew value = 1\r\n\r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nmain\r\n\032\032frame-args\r\n \\(\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*$srcfile\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n$decimal\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n.*$gdb_prompt$" \
+   -re "\r\n\032\032post-prompt\r\n\r\n(\032\032breakpoints-invalid\r\n\r\n)*\032\032starting\r\n\r\n(\032\032frames-invalid\r\n\r\n)*\032\032watchpoint 3\r\n.*atchpoint 3: a.x\r\n\r\nOld value = 0\r\nNew value = 1\r\n\r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nmain\r\n\032\032frame-args\r\n \\(\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*$srcfile\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n$decimal\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n.*$gdb_prompt$" \
 	   { pass "watch triggered on a.x" }
-   -re "\r\n\032\032post-prompt\r\n\r\n\032\032starting\r\n\r\n\032\032frames-invalid\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" \
+   -re "\r\n\032\032post-prompt\r\n\r\n(\032\032breakpoints-invalid\r\n\r\n)*\032\032starting\r\n\r\n\032\032frames-invalid\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" \
 	   { kfail "gdb/38" "watch triggered on a.x" }
    -re ".*$gdb_prompt$"  { fail "watch triggered on a.x" }
    timeout    { fail "watch triggered on a.x (timeout)" }

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: 3/5 - Rework stepping over longjmp support
  2008-04-25 18:29   ` Pedro Alves
@ 2008-05-02 14:41     ` Daniel Jacobowitz
  2008-05-04 19:59       ` Pedro Alves
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Jacobowitz @ 2008-05-02 14:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Fri, Apr 25, 2008 at 05:15:45PM +0100, Pedro Alves wrote:
> Still OK?  I'll check the new longjmp.exp test in along with this one.

Looks fine to me.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: 3/5 - Rework stepping over longjmp support
  2008-05-02 14:41     ` Daniel Jacobowitz
@ 2008-05-04 19:59       ` Pedro Alves
  2008-05-05 19:23         ` Pedro Alves
  0 siblings, 1 reply; 7+ messages in thread
From: Pedro Alves @ 2008-05-04 19:59 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 341 bytes --]

A Friday 02 May 2008 15:38:22, Daniel Jacobowitz wrote:
> On Fri, Apr 25, 2008 at 05:15:45PM +0100, Pedro Alves wrote:
> > Still OK?  I'll check the new longjmp.exp test in along with this one.
>
> Looks fine to me.

Thanks.  I've checked this one in, and the test too.

Non-stop mode should be now safer regarding longjmp.

-- 
Pedro Alves

[-- Attachment #2: longjmp_rework.diff --]
[-- Type: text/x-diff, Size: 26312 bytes --]

2008-05-04  Pedro Alves  <pedro@codesourcery.com>

	gdb/

	* breakpoint.c (update_breakpoints_after_exec): Delete bp_longjmp
	and bp_longjmp_resume breakpoints.
	(breakpoint_address_is_meaningful): Claim bp_longjmp_resume as
	meaningful.
	(create_longjmp_breakpoint): Don't create bp_longjmp_resume
	breakpoints.  Create bp_longjmp breakpoints as momentary
	breakpoints.
	(enable_longjmp_breakpoint): Delete.
	(set_longjmp_breakpoint): New.
	(disable_longjmp_breakpoint): Delete.
	(delete_longjmp_breakpoint): New.
	(set_longjmp_resume_breakpoint): Delete.
	(set_momentary_breakpoint_at_pc): New.
	(breakpoint_re_set_one): Don't delete bp_longjmp and
	bp_longjmp_resume breakpoints.
	(breakpoint_re_set): Don't create longjmp and longjmp-resume
	breakpoints.

	* infrun.c (step_resume_breakpoint): Add comment.
	(struct execution_control_state): Delete handling_longjmp member.
	(init_execution_control_state). Don't clear handling_longjmp.
	(context_switch): Don't context switch handling_longjmp.
	(handle_inferior_event): If handling a bp_longjmp breakpoint,
	create a bp_longjmp_resume breakpoint, and set it as current
	step_resume_breakpoint, then step over the longjmp breakpoint.  If
	handling a bp_longjmp_resume breakpoint, don't delete the longjmp
	breakpoint, delete the longjmp-resume breakpoint, and stop
	stepping.
	(currently_stepping): Remove handling_longjmp from expression.
	(insert_step_resume_breakpoint_at_sal): Update comment.
	(insert_longjmp_resume_breakpoint): New.

	* breakpoint.h (set_momentary_breakpoint_at_pc): Declare.
	(enable_longjmp_breakpoint, disable_longjmp_breakpoint): Delete
	declarations.
	(set_longjmp_breakpoint, delete_longjmp_breakpoint): Declare.
	(set_longjmp_resume_breakpoint): Delete declaration.

	* gdbthread.h (save_infrun_state): Remove handling_longjmp parameter.
	(load_infrun_state): Delete *handling_longjmp parameter.
	* thread.c (save_infrun_state): Remove handling_longjmp parameter.  Update body.
	(load_infrun_state): Delete *handling_longjmp parameter.  Update body.

	* infcmd.c Include "gdbthread.h".
	(disable_longjmp_breakpoint_cleanup): Delete.
	(delete_longjmp_breakpoint_cleanup): New.
	(step_1): Call set_longjmp_breakpoint instead of
	enable_longjmp_breakpoint.  Use delete_longjmp_breakpoint_cleanup
	instead of disable_longjmp_breakpoint_cleanup when making cleanup.
	(step_1_continuation): Pass thread id in the continuation args to
	step_once.
	(step_once): Add thread parameter.  Pass thread id the the
	continuation.

	* Makefile.in (infcmd.o): Update.

	gdb/testsuite:
	* gdb.cp/annota2.exp: Adjust to breakpoints invalidations at
	different times.

---
 gdb/breakpoint.c                 |  112 ++++++++++++++-------------------------
 gdb/breakpoint.h                 |    9 ++-
 gdb/gdbthread.h                  |    3 -
 gdb/infcmd.c                     |   41 ++++++++++----
 gdb/infrun.c                     |   86 ++++++++++++++++++++---------
 gdb/testsuite/gdb.cp/annota2.exp |    4 -
 gdb/thread.c                     |    4 -
 7 files changed, 141 insertions(+), 118 deletions(-)

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2008-05-04 20:15:42.000000000 +0100
+++ src/gdb/breakpoint.c	2008-05-04 20:20:14.000000000 +0100
@@ -145,8 +145,6 @@ static int watchpoint_check (void *);
 
 static void maintenance_info_breakpoints (char *, int);
 
-static void create_longjmp_breakpoint (char *);
-
 static void create_overlay_event_breakpoint (char *);
 
 static int hw_breakpoint_used_count (void);
@@ -1486,6 +1484,14 @@ update_breakpoints_after_exec (void)
 	continue;
       }
 
+    /* Longjmp and longjmp-resume breakpoints are also meaningless
+       after an exec.  */
+    if (b->type == bp_longjmp || b->type == bp_longjmp_resume)
+      {
+	delete_breakpoint (b);
+	continue;
+      }
+
     /* Don't delete an exec catchpoint, because else the inferior
        won't stop when it ought!
 
@@ -4081,7 +4087,6 @@ set_default_breakpoint (int valid, CORE_
       bp_read_watchpoint
       bp_access_watchpoint
       bp_catch_exec
-      bp_longjmp_resume
       bp_catch_fork
       bp_catch_vork */
 
@@ -4095,7 +4100,6 @@ breakpoint_address_is_meaningful (struct
 	  && type != bp_read_watchpoint
 	  && type != bp_access_watchpoint
 	  && type != bp_catch_exec
-	  && type != bp_longjmp_resume
 	  && type != bp_catch_fork
 	  && type != bp_catch_vfork);
 }
@@ -4453,20 +4457,9 @@ create_longjmp_breakpoint (char *func_na
   struct breakpoint *b;
   struct minimal_symbol *m;
 
-  if (func_name == NULL)
-    b = create_internal_breakpoint (0, bp_longjmp_resume);
-  else
-    {
-      if ((m = lookup_minimal_symbol_text (func_name, NULL)) == NULL)
-	return;
- 
-      b = create_internal_breakpoint (SYMBOL_VALUE_ADDRESS (m), bp_longjmp);
-    }
-
-  b->enable_state = bp_disabled;
-  b->silent = 1;
-  if (func_name)
-    b->addr_string = xstrdup (func_name);
+  if ((m = lookup_minimal_symbol_text (func_name, NULL)) == NULL)
+    return;
+  set_momentary_breakpoint_at_pc (SYMBOL_VALUE_ADDRESS (m), bp_longjmp);
   update_global_location_list ();
 }
 
@@ -4475,30 +4468,31 @@ create_longjmp_breakpoint (char *func_na
    set_longjmp_resume_breakpoint() to figure out where we are going. */
 
 void
-enable_longjmp_breakpoint (void)
+set_longjmp_breakpoint (void)
 {
   struct breakpoint *b;
 
-  ALL_BREAKPOINTS (b)
-    if (b->type == bp_longjmp)
+  if (gdbarch_get_longjmp_target_p (current_gdbarch))
     {
-      b->enable_state = bp_enabled;
-      update_global_location_list ();
+      create_longjmp_breakpoint ("longjmp");
+      create_longjmp_breakpoint ("_longjmp");
+      create_longjmp_breakpoint ("siglongjmp");
+      create_longjmp_breakpoint ("_siglongjmp");
     }
 }
 
+/* Delete all longjmp breakpoints from THREAD.  */
 void
-disable_longjmp_breakpoint (void)
+delete_longjmp_breakpoint (int thread)
 {
-  struct breakpoint *b;
+  struct breakpoint *b, *temp;
 
-  ALL_BREAKPOINTS (b)
-    if (b->type == bp_longjmp
-	|| b->type == bp_longjmp_resume)
-    {
-      b->enable_state = bp_disabled;
-      update_global_location_list ();
-    }
+  ALL_BREAKPOINTS_SAFE (b, temp)
+    if (b->type == bp_longjmp)
+      {
+	if (b->thread == thread)
+	  delete_breakpoint (b);
+      }
 }
 
 static void
@@ -4791,30 +4785,6 @@ hw_watchpoint_used_count (enum bptype ty
   return i;
 }
 
-/* Call this after hitting the longjmp() breakpoint.  Use this to set
-   a new breakpoint at the target of the jmp_buf.
-
-   FIXME - This ought to be done by setting a temporary breakpoint
-   that gets deleted automatically... */
-
-void
-set_longjmp_resume_breakpoint (CORE_ADDR pc, struct frame_id frame_id)
-{
-  struct breakpoint *b;
-
-  ALL_BREAKPOINTS (b)
-    if (b->type == bp_longjmp_resume)
-    {
-      b->loc->requested_address = pc;
-      b->loc->address = adjust_breakpoint_address (b->loc->requested_address,
-                                                   b->type);
-      b->enable_state = bp_enabled;
-      b->frame_id = frame_id;
-      update_global_location_list ();
-      return;
-    }
-}
-
 void
 disable_watchpoints_before_interactive_call_start (void)
 {
@@ -4878,6 +4848,19 @@ set_momentary_breakpoint (struct symtab_
 
   return b;
 }
+
+struct breakpoint *
+set_momentary_breakpoint_at_pc (CORE_ADDR pc, enum bptype type)
+{
+  struct symtab_and_line sal;
+
+  sal = find_pc_line (pc, 0);
+  sal.pc = pc;
+  sal.section = find_pc_overlay (pc);
+  sal.explicit_pc = 1;
+
+  return set_momentary_breakpoint (sal, null_frame_id, type);
+}
 \f
 
 /* Tell the user we have just set a breakpoint B.  */
@@ -7529,10 +7512,8 @@ breakpoint_re_set_one (void *bint)
     default:
       printf_filtered (_("Deleting unknown breakpoint type %d\n"), b->type);
       /* fall through */
-      /* Delete longjmp and overlay event breakpoints; they will be
-         reset later by breakpoint_re_set.  */
-    case bp_longjmp:
-    case bp_longjmp_resume:
+      /* Delete overlay event breakpoints; they will be reset later by
+         breakpoint_re_set.  */
     case bp_overlay_event:
       delete_breakpoint (b);
       break;
@@ -7554,6 +7535,8 @@ breakpoint_re_set_one (void *bint)
     case bp_watchpoint_scope:
     case bp_call_dummy:
     case bp_step_resume:
+    case bp_longjmp:
+    case bp_longjmp_resume:
       break;
     }
 
@@ -7581,15 +7564,6 @@ breakpoint_re_set (void)
   }
   set_language (save_language);
   input_radix = save_input_radix;
-
-  if (gdbarch_get_longjmp_target_p (current_gdbarch))
-    {
-      create_longjmp_breakpoint ("longjmp");
-      create_longjmp_breakpoint ("_longjmp");
-      create_longjmp_breakpoint ("siglongjmp");
-      create_longjmp_breakpoint ("_siglongjmp");
-      create_longjmp_breakpoint (NULL);
-    }
   
   create_overlay_event_breakpoint ("_ovly_debug_event");
 }
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-05-04 20:15:45.000000000 +0100
+++ src/gdb/infrun.c	2008-05-04 20:20:14.000000000 +0100
@@ -279,6 +279,7 @@ struct regcache *stop_registers;
 
 static int stop_print_frame;
 
+/* Step-resume or longjmp-resume breakpoint.  */
 static struct breakpoint *step_resume_breakpoint = NULL;
 
 /* This is a cached copy of the pid/waitstatus of the last event
@@ -1380,7 +1381,6 @@ struct execution_control_state
   struct symtab_and_line sal;
   int current_line;
   struct symtab *current_symtab;
-  int handling_longjmp;		/* FIXME */
   ptid_t ptid;
   ptid_t saved_inferior_ptid;
   int step_after_step_resume_breakpoint;
@@ -1402,6 +1402,8 @@ static void insert_step_resume_breakpoin
 static void insert_step_resume_breakpoint_at_caller (struct frame_info *);
 static void insert_step_resume_breakpoint_at_sal (struct symtab_and_line sr_sal,
 						  struct frame_id sr_id);
+static void insert_longjmp_resume_breakpoint (CORE_ADDR);
+
 static void stop_stepping (struct execution_control_state *ecs);
 static void prepare_to_wait (struct execution_control_state *ecs);
 static void keep_going (struct execution_control_state *ecs);
@@ -1546,7 +1548,6 @@ init_execution_control_state (struct exe
   ecs->stepping_over_breakpoint = 0;
   ecs->random_signal = 0;
   ecs->step_after_step_resume_breakpoint = 0;
-  ecs->handling_longjmp = 0;	/* FIXME */
   ecs->stepping_through_solib_after_catch = 0;
   ecs->stepping_through_solib_catchpoints = NULL;
   ecs->sal = find_pc_line (prev_pc, 0);
@@ -1601,7 +1602,7 @@ context_switch (struct execution_control
 			 stepping_over_breakpoint, step_resume_breakpoint,
 			 step_range_start,
 			 step_range_end, &step_frame_id,
-			 ecs->handling_longjmp, ecs->stepping_over_breakpoint,
+			 ecs->stepping_over_breakpoint,
 			 ecs->stepping_through_solib_after_catch,
 			 ecs->stepping_through_solib_catchpoints,
 			 ecs->current_line, ecs->current_symtab);
@@ -1611,7 +1612,7 @@ context_switch (struct execution_control
 			 &stepping_over_breakpoint, &step_resume_breakpoint,
 			 &step_range_start,
 			 &step_range_end, &step_frame_id,
-			 &ecs->handling_longjmp, &ecs->stepping_over_breakpoint,
+			 &ecs->stepping_over_breakpoint,
 			 &ecs->stepping_through_solib_after_catch,
 			 &ecs->stepping_through_solib_catchpoints,
 			 &ecs->current_line, &ecs->current_symtab);
@@ -2574,38 +2575,50 @@ process_event_stop_test:
     switch (what.main_action)
       {
       case BPSTAT_WHAT_SET_LONGJMP_RESUME:
-	/* If we hit the breakpoint at longjmp, disable it for the
-	   duration of this command.  Then, install a temporary
-	   breakpoint at the target of the jmp_buf. */
-        if (debug_infrun)
-	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
-	disable_longjmp_breakpoint ();
+	/* If we hit the breakpoint at longjmp while stepping, we
+	   install a momentary breakpoint at the target of the
+	   jmp_buf.  */
+
+	if (debug_infrun)
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
+
+	ecs->stepping_over_breakpoint = 1;
+
 	if (!gdbarch_get_longjmp_target_p (current_gdbarch)
 	    || !gdbarch_get_longjmp_target (current_gdbarch,
 					    get_current_frame (), &jmp_buf_pc))
 	  {
+	    if (debug_infrun)
+	      fprintf_unfiltered (gdb_stdlog, "\
+infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
 	    keep_going (ecs);
 	    return;
 	  }
 
-	/* Need to blow away step-resume breakpoint, as it
-	   interferes with us */
+	/* We're going to replace the current step-resume breakpoint
+	   with a longjmp-resume breakpoint.  */
 	if (step_resume_breakpoint != NULL)
-	  {
-	    delete_step_resume_breakpoint (&step_resume_breakpoint);
-	  }
+	  delete_step_resume_breakpoint (&step_resume_breakpoint);
+
+	/* Insert a breakpoint at resume address.  */
+	insert_longjmp_resume_breakpoint (jmp_buf_pc);
 
-	set_longjmp_resume_breakpoint (jmp_buf_pc, null_frame_id);
-	ecs->handling_longjmp = 1;	/* FIXME */
 	keep_going (ecs);
 	return;
 
       case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
         if (debug_infrun)
-	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
-	disable_longjmp_breakpoint ();
-	ecs->handling_longjmp = 0;	/* FIXME */
-	break;
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
+
+	gdb_assert (step_resume_breakpoint != NULL);
+	delete_step_resume_breakpoint (&step_resume_breakpoint);
+
+	stop_step = 1;
+	print_stop_reason (END_STEPPING_RANGE, 0);
+	stop_stepping (ecs);
+	return;
 
       case BPSTAT_WHAT_SINGLE:
         if (debug_infrun)
@@ -3168,9 +3181,8 @@ process_event_stop_test:
 static int
 currently_stepping (struct execution_control_state *ecs)
 {
-  return ((!ecs->handling_longjmp
-	   && ((step_range_end && step_resume_breakpoint == NULL)
-	       || stepping_over_breakpoint))
+  return (((step_range_end && step_resume_breakpoint == NULL)
+	   || stepping_over_breakpoint)
 	  || ecs->stepping_through_solib_after_catch
 	  || bpstat_should_step ());
 }
@@ -3257,8 +3269,8 @@ static void
 insert_step_resume_breakpoint_at_sal (struct symtab_and_line sr_sal,
 				      struct frame_id sr_id)
 {
-  /* There should never be more than one step-resume breakpoint per
-     thread, so we should never be setting a new
+  /* There should never be more than one step-resume or longjmp-resume
+     breakpoint per thread, so we should never be setting a new
      step_resume_breakpoint when one is already active.  */
   gdb_assert (step_resume_breakpoint == NULL);
 
@@ -3326,6 +3338,28 @@ insert_step_resume_breakpoint_at_caller 
   insert_step_resume_breakpoint_at_sal (sr_sal, frame_unwind_id (next_frame));
 }
 
+/* Insert a "longjmp-resume" breakpoint at PC.  This is used to set a
+   new breakpoint at the target of a jmp_buf.  The handling of
+   longjmp-resume uses the same mechanisms used for handling
+   "step-resume" breakpoints.  */
+
+static void
+insert_longjmp_resume_breakpoint (CORE_ADDR pc)
+{
+  /* There should never be more than one step-resume or longjmp-resume
+     breakpoint per thread, so we should never be setting a new
+     longjmp_resume_breakpoint when one is already active.  */
+  gdb_assert (step_resume_breakpoint == NULL);
+
+  if (debug_infrun)
+    fprintf_unfiltered (gdb_stdlog,
+			"infrun: inserting longjmp-resume breakpoint at 0x%s\n",
+			paddr_nz (pc));
+
+  step_resume_breakpoint =
+    set_momentary_breakpoint_at_pc (pc, bp_longjmp_resume);
+}
+
 static void
 stop_stepping (struct execution_control_state *ecs)
 {
Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h	2008-05-04 20:15:42.000000000 +0100
+++ src/gdb/breakpoint.h	2008-05-04 20:20:14.000000000 +0100
@@ -687,6 +687,9 @@ extern void breakpoint_re_set_thread (st
 extern struct breakpoint *set_momentary_breakpoint
   (struct symtab_and_line, struct frame_id, enum bptype);
 
+extern struct breakpoint *set_momentary_breakpoint_at_pc
+  (CORE_ADDR pc, enum bptype type);
+
 extern void set_ignore_count (int, int, int);
 
 extern void set_default_breakpoint (int, CORE_ADDR, struct symtab *, int);
@@ -755,12 +758,12 @@ extern void update_breakpoints_after_exe
    inferior_ptid.  */
 extern int detach_breakpoints (int);
 
-extern void enable_longjmp_breakpoint (void);
-extern void disable_longjmp_breakpoint (void);
+extern void set_longjmp_breakpoint (void);
+extern void delete_longjmp_breakpoint (int thread);
+
 extern void enable_overlay_breakpoints (void);
 extern void disable_overlay_breakpoints (void);
 
-extern void set_longjmp_resume_breakpoint (CORE_ADDR, struct frame_id);
 /* These functions respectively disable or reenable all currently
    enabled watchpoints.  When disabled, the watchpoints are marked
    call_disabled.  When reenabled, they are marked enabled.
Index: src/gdb/gdbthread.h
===================================================================
--- src.orig/gdb/gdbthread.h	2008-05-04 20:15:44.000000000 +0100
+++ src/gdb/gdbthread.h	2008-05-04 20:20:14.000000000 +0100
@@ -50,7 +50,6 @@ struct thread_info
   int current_line;
   struct symtab *current_symtab;
   int trap_expected;
-  int handling_longjmp;
   int stepping_over_breakpoint;
 
   /* This is set TRUE when a catchpoint of a shared library event
@@ -123,7 +122,6 @@ extern void save_infrun_state (ptid_t pt
 			       CORE_ADDR step_range_start,
 			       CORE_ADDR step_range_end,
 			       const struct frame_id *step_frame_id,
-			       int       handling_longjmp,
 			       int       another_trap,
 			       int       stepping_through_solib_after_catch,
 			       bpstat    stepping_through_solib_catchpoints,
@@ -139,7 +137,6 @@ extern void load_infrun_state (ptid_t pt
 			       CORE_ADDR *step_range_start,
 			       CORE_ADDR *step_range_end,
 			       struct frame_id *step_frame_id,
-			       int       *handling_longjmp,
 			       int       *another_trap,
 			       int       *stepping_through_solib_affter_catch,
 			       bpstat    *stepping_through_solib_catchpoints,
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c	2008-05-04 20:15:45.000000000 +0100
+++ src/gdb/thread.c	2008-05-04 20:20:14.000000000 +0100
@@ -319,7 +319,6 @@ load_infrun_state (ptid_t ptid,
 		   CORE_ADDR *step_range_start,
 		   CORE_ADDR *step_range_end,
 		   struct frame_id *step_frame_id,
-		   int *handling_longjmp,
 		   int *stepping_over_breakpoint,
 		   int *stepping_through_solib_after_catch,
 		   bpstat *stepping_through_solib_catchpoints,
@@ -340,7 +339,6 @@ load_infrun_state (ptid_t ptid,
   *step_range_start = tp->step_range_start;
   *step_range_end = tp->step_range_end;
   *step_frame_id = tp->step_frame_id;
-  *handling_longjmp = tp->handling_longjmp;
   *stepping_over_breakpoint = tp->stepping_over_breakpoint;
   *stepping_through_solib_after_catch =
     tp->stepping_through_solib_after_catch;
@@ -360,7 +358,6 @@ save_infrun_state (ptid_t ptid,
 		   CORE_ADDR step_range_start,
 		   CORE_ADDR step_range_end,
 		   const struct frame_id *step_frame_id,
-		   int handling_longjmp,
 		   int stepping_over_breakpoint,
 		   int stepping_through_solib_after_catch,
 		   bpstat stepping_through_solib_catchpoints,
@@ -381,7 +378,6 @@ save_infrun_state (ptid_t ptid,
   tp->step_range_start = step_range_start;
   tp->step_range_end = step_range_end;
   tp->step_frame_id = (*step_frame_id);
-  tp->handling_longjmp = handling_longjmp;
   tp->stepping_over_breakpoint = stepping_over_breakpoint;
   tp->stepping_through_solib_after_catch = stepping_through_solib_after_catch;
   tp->stepping_through_solib_catchpoints = stepping_through_solib_catchpoints;
Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c	2008-05-04 20:15:45.000000000 +0100
+++ src/gdb/infcmd.c	2008-05-04 20:20:14.000000000 +0100
@@ -107,7 +107,7 @@ static void signal_command (char *, int)
 static void jump_command (char *, int);
 
 static void step_1 (int, int, char *);
-static void step_once (int skip_subroutines, int single_inst, int count);
+static void step_once (int skip_subroutines, int single_inst, int count, int thread);
 static void step_1_continuation (struct continuation_arg *arg, int error_p);
 
 static void next_command (char *, int);
@@ -693,9 +693,10 @@ nexti_command (char *count_string, int f
 }
 
 static void
-disable_longjmp_breakpoint_cleanup (void *ignore)
+delete_longjmp_breakpoint_cleanup (void *arg)
 {
-  disable_longjmp_breakpoint ();
+  int thread = * (int *) arg;
+  delete_longjmp_breakpoint (thread);
 }
 
 static void
@@ -705,6 +706,7 @@ step_1 (int skip_subroutines, int single
   struct frame_info *frame;
   struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
   int async_exec = 0;
+  int *thread_p = NULL;
 
   ERROR_NO_INFERIOR;
 
@@ -728,8 +730,17 @@ step_1 (int skip_subroutines, int single
 
   if (!single_inst || skip_subroutines)		/* leave si command alone */
     {
-      enable_longjmp_breakpoint ();
-      make_cleanup (disable_longjmp_breakpoint_cleanup, 0 /*ignore*/);
+      thread_p = xmalloc (sizeof (int));
+      make_cleanup (xfree, thread_p);
+
+      if (in_thread_list (inferior_ptid))
+ 	*thread_p = pid_to_thread_id (inferior_ptid);
+      else
+ 	*thread_p = -1;
+
+      set_longjmp_breakpoint ();
+
+      make_cleanup (delete_longjmp_breakpoint_cleanup, thread_p);
     }
 
   /* In synchronous case, all is well, just use the regular for loop. */
@@ -790,10 +801,11 @@ which has no line number information.\n"
      and handle them one at the time, through step_once(). */
   else
     {
-      step_once (skip_subroutines, single_inst, count);
-      /* We are running, and the contination is installed.  It will
+      step_once (skip_subroutines, single_inst, count, *thread_p);
+      /* We are running, and the continuation is installed.  It will
 	 disable the longjmp breakpoint as appropriate.  */
       discard_cleanups (cleanups);
+      xfree (thread_p);
     }
 }
 
@@ -808,10 +820,12 @@ step_1_continuation (struct continuation
   int count;
   int skip_subroutines;
   int single_inst;
+  int thread;
       
   skip_subroutines = arg->data.integer;
   single_inst      = arg->next->data.integer;
   count            = arg->next->next->data.integer;
+  thread           = arg->next->next->next->data.integer;
 
   if (error_p || !step_multi || !stop_step)
     {
@@ -819,11 +833,11 @@ step_1_continuation (struct continuation
 	 that is not stepping, or there are no further steps
 	 to make.  Cleanup.  */
       if (!single_inst || skip_subroutines)
-	disable_longjmp_breakpoint ();
+	delete_longjmp_breakpoint (thread);
       step_multi = 0;
     }
   else
-    step_once (skip_subroutines, single_inst, count - 1);
+    step_once (skip_subroutines, single_inst, count - 1, thread);
 }
 
 /* Do just one step operation. If count >1 we will have to set up a
@@ -834,11 +848,12 @@ step_1_continuation (struct continuation
    called in case of step n with n>1, after the first step operation has
    been completed.*/
 static void 
-step_once (int skip_subroutines, int single_inst, int count)
+step_once (int skip_subroutines, int single_inst, int count, int thread)
 { 
   struct continuation_arg *arg1; 
   struct continuation_arg *arg2;
   struct continuation_arg *arg3; 
+  struct continuation_arg *arg4;
   struct frame_info *frame;
 
   if (count > 0)
@@ -894,12 +909,16 @@ which has no line number information.\n"
 	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
       arg3 =
 	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
+      arg4 =
+	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
       arg1->next = arg2;
       arg1->data.integer = skip_subroutines;
       arg2->next = arg3;
       arg2->data.integer = single_inst;
-      arg3->next = NULL;
+      arg3->next = arg4;
       arg3->data.integer = count;
+      arg4->next = NULL;
+      arg4->data.integer = thread;
       add_intermediate_continuation (step_1_continuation, arg1);
     }
 }
Index: src/gdb/testsuite/gdb.cp/annota2.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.cp/annota2.exp	2008-05-04 20:15:46.000000000 +0100
+++ src/gdb/testsuite/gdb.cp/annota2.exp	2008-05-04 20:20:14.000000000 +0100
@@ -190,9 +190,9 @@ gdb_expect {
 #
 send_gdb "next\n"
 gdb_expect {
-   -re "\r\n\032\032post-prompt\r\n\r\n\032\032starting\r\n\r\n(\032\032frames-invalid\r\n\r\n)*\032\032watchpoint 3\r\n.*atchpoint 3: a.x\r\n\r\nOld value = 0\r\nNew value = 1\r\n\r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nmain\r\n\032\032frame-args\r\n \\(\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*$srcfile\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n$decimal\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n.*$gdb_prompt$" \
+   -re "\r\n\032\032post-prompt\r\n\r\n(\032\032breakpoints-invalid\r\n\r\n)*\032\032starting\r\n\r\n(\032\032frames-invalid\r\n\r\n)*\032\032watchpoint 3\r\n.*atchpoint 3: a.x\r\n\r\nOld value = 0\r\nNew value = 1\r\n\r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nmain\r\n\032\032frame-args\r\n \\(\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*$srcfile\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n$decimal\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n.*$gdb_prompt$" \
 	   { pass "watch triggered on a.x" }
-   -re "\r\n\032\032post-prompt\r\n\r\n\032\032starting\r\n\r\n\032\032frames-invalid\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" \
+   -re "\r\n\032\032post-prompt\r\n\r\n(\032\032breakpoints-invalid\r\n\r\n)*\032\032starting\r\n\r\n\032\032frames-invalid\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" \
 	   { kfail "gdb/38" "watch triggered on a.x" }
    -re ".*$gdb_prompt$"  { fail "watch triggered on a.x" }
    timeout    { fail "watch triggered on a.x (timeout)" }

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: 3/5 - Rework stepping over longjmp support
  2008-05-04 19:59       ` Pedro Alves
@ 2008-05-05 19:23         ` Pedro Alves
  0 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2008-05-05 19:23 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

A Sunday 04 May 2008 20:49:43, Pedro Alves wrote:
> A Friday 02 May 2008 15:38:22, Daniel Jacobowitz wrote:
> > On Fri, Apr 25, 2008 at 05:15:45PM +0100, Pedro Alves wrote:
> > > Still OK?  I'll check the new longjmp.exp test in along with this one.
> >
> > Looks fine to me.
>
> Thanks.  I've checked this one in, and the test too.
>
> Non-stop mode should be now safer regarding longjmp.

Except, it's crashing in async mode ...

thread_p was only allocated when (!single_inst || skip_subroutines),
because on the other cases, we don't need longjmp breakpoints,
but, it was always being dereferenced in async mode.

There's really no reason to be using the heap.  Fixed by moving
the variable to the stack (as cleanup memory is supposed to be
managed in the first place).

Checked in as obvious.

-- 
Pedro Alves

/me teaches himself to never do last minute changes.
-- 
Pedro Alves

[-- Attachment #2: fix_crash.diff --]
[-- Type: text/x-diff, Size: 1811 bytes --]

2008-05-05  Pedro Alves  <pedro@codesourcery.com>

	* infcmd.c (step_1): Put thread id on the stack to avoid possible
	NULL dereferencing.

---
 gdb/infcmd.c |   14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c	2008-05-05 17:33:46.000000000 +0100
+++ src/gdb/infcmd.c	2008-05-05 17:34:01.000000000 +0100
@@ -706,7 +706,7 @@ step_1 (int skip_subroutines, int single
   struct frame_info *frame;
   struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
   int async_exec = 0;
-  int *thread_p = NULL;
+  int thread = -1;
 
   ERROR_NO_INFERIOR;
 
@@ -730,17 +730,12 @@ step_1 (int skip_subroutines, int single
 
   if (!single_inst || skip_subroutines)		/* leave si command alone */
     {
-      thread_p = xmalloc (sizeof (int));
-      make_cleanup (xfree, thread_p);
-
       if (in_thread_list (inferior_ptid))
- 	*thread_p = pid_to_thread_id (inferior_ptid);
-      else
- 	*thread_p = -1;
+ 	thread = pid_to_thread_id (inferior_ptid);
 
       set_longjmp_breakpoint ();
 
-      make_cleanup (delete_longjmp_breakpoint_cleanup, thread_p);
+      make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
     }
 
   /* In synchronous case, all is well, just use the regular for loop. */
@@ -801,11 +796,10 @@ which has no line number information.\n"
      and handle them one at the time, through step_once(). */
   else
     {
-      step_once (skip_subroutines, single_inst, count, *thread_p);
+      step_once (skip_subroutines, single_inst, count, thread);
       /* We are running, and the continuation is installed.  It will
 	 disable the longjmp breakpoint as appropriate.  */
       discard_cleanups (cleanups);
-      xfree (thread_p);
     }
 }
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2008-05-05 16:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-04-07  2:34 3/5 - Rework stepping over longjmp support Pedro Alves
2008-04-07 19:43 ` Tom Tromey
2008-04-14 18:57 ` Daniel Jacobowitz
2008-04-25 18:29   ` Pedro Alves
2008-05-02 14:41     ` Daniel Jacobowitz
2008-05-04 19:59       ` Pedro Alves
2008-05-05 19:23         ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox