[00/19] Eliminate some more current_gdbarch uses

[00/19] Eliminate some more current_gdbarch uses

[Ulrich Weigand]

Hello,

this series of patches makes some more progress towards the goal of
eliminating the current_gdbarch global.  This series tackles remaining
instances that are relatively simple to eliminate using existing
mechanism and relatively "local" changes.

While some patches in the series do change existing interfaces to
allow passing architecture information to some lower level, these
are typically less-frequently used ones that require only limited
changes to the rest of GDB.

After this patch series, the remaining instances of current_gdbarch
are difficult to eliminate, typically within frequently-called
central infrastructure routines.  A follow-on patch series will
address those remaining occurrences.

This patch series was tested with no regressions on amd64-linux,
powerpc64-linux, spu-elf, s390-ibm-linux, and s390x-ibm-linux.
Also, I made sure an --enable-targets=all build still succeeds.

Comments welcome!  I plan on committing the series within a week
or two (after I'm back from the GCC Summit).

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [00/19] Eliminate some more current_gdbarch uses

[Pedro Alves]

I took a first-pass-cursory look throughout the series, and I
don't have much to add.  It all looked good to me.  The
only things that jumped out were:

- We should do something smarter about longjmp breakpoint
  lookup at some point.

- I have a gut feeling that a single gdbarch per expression is
  going to byte us back somewhere along the line, but that's not
  a problem of this patch.

-- 
Pedro Alves

Re: [00/19] Eliminate some more current_gdbarch uses

[Tom Tromey]

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

Pedro> - We should do something smarter about longjmp breakpoint
Pedro>   lookup at some point.

If you have something in mind, I would like to hear it.
The next-over-throw patch does something very similar to the longjmp
code; I would be happy to update both at once.

Tom

Re: [00/19] Eliminate some more current_gdbarch uses

[Pedro Alves]

On Friday 05 June 2009 23:52:25, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> Pedro> - We should do something smarter about longjmp breakpoint
> Pedro>   lookup at some point.
> 
> If you have something in mind, I would like to hear it.
> The next-over-throw patch does something very similar to the longjmp
> code; I would be happy to update both at once.

I *know* we can do smarter, but exactly what's the best, I haven't
thought much about.  Some data points / suggestions:

 - longjmp breakpoints need to be installed as long as there's a
   thread nexting or stepping or whatever command that needs those,
   for non-stop mode.  The fact that they're momentary breakpoints
   makes it so that a thread that is "continue"ing, simply thread
   hops them, since momentary breakpoints are thread specific, which is
   more efficient than having them all
   set longjmp-resume, hit-longjmp-resume,resume.  The breakpoint location
   machinery takes care of not installing duplicates.  The set of addresses
   where to install such breakpoints however, don't change per-address
   space --- we could cache minimal symbols on a per symbol/address space
   generic framework (similar to objfile_data), and reparse that only when
   symbols change (that is, breakpoint_re_set or similar).

 - perhaps the simplest and most likely the most effective easily: we
   can use the objfile_data mechanism to cache if there are longjmp-like
   functions in a given objfile.  No use looking up the minimal symbol
   everytime.

 - lookup_minimal_symbol still does a linear walk on all objfiles
   when you pass it an objfile.  That may matter a bit.  We could
   split that function in two to not do that loop when we know the
   objfile in advance.

 - we can move the update_global_location_list to the caller of
   create_longjmp_breakpoint, cutting its frequency by 4, if that
   matters.  Probably not currently.

-- 
Pedro Alves

Re: [00/19] Eliminate some more current_gdbarch uses

[Ulrich Weigand]

Pedro Alves wrote:

> I *know* we can do smarter, but exactly what's the best, I haven't
> thought much about.  Some data points / suggestions:
> 
>  - longjmp breakpoints need to be installed as long as there's a
>    thread nexting or stepping or whatever command that needs those,
>    for non-stop mode.  The fact that they're momentary breakpoints
>    makes it so that a thread that is "continue"ing, simply thread
>    hops them, since momentary breakpoints are thread specific, which is
>    more efficient than having them all
>    set longjmp-resume, hit-longjmp-resume,resume.  The breakpoint location
>    machinery takes care of not installing duplicates.  The set of addresses
>    where to install such breakpoints however, don't change per-address
>    space --- we could cache minimal symbols on a per symbol/address space
>    generic framework (similar to objfile_data), and reparse that only when
>    symbols change (that is, breakpoint_re_set or similar).
> 
>  - perhaps the simplest and most likely the most effective easily: we
>    can use the objfile_data mechanism to cache if there are longjmp-like
>    functions in a given objfile.  No use looking up the minimal symbol
>    everytime.

I guess we could treat the longjmp breakpoint similarly to the
overlay event breakpoint and recompute its address(es) only when
objfiles change (i.e. in breakpoint_re_set_objfile).

Of course, this would mean the breakpoints were always active.  However,
this could be improved upon by:
- having infrun ignore longjmp breakpoints if not stepping
- keeping them disabled unless stepping (and still have infrun ignore
  longjmp breakpoints when hit in the wrong thread)
and/or
- keeping them always disabled, but installing momentary clones in
  threads that are stepping

>  - lookup_minimal_symbol still does a linear walk on all objfiles
>    when you pass it an objfile.  That may matter a bit.  We could
>    split that function in two to not do that loop when we know the
>    objfile in advance.

This might be useful anyway ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [00/19] Eliminate some more current_gdbarch uses

[Pedro Alves]

On Monday 08 June 2009 15:38:39, Ulrich Weigand wrote:
> Of course, this would mean the breakpoints were always active.  However,
> this could be improved upon by:

> - having infrun ignore longjmp breakpoints if not stepping

If there isn't any thread doing a command that requires longjmp
breakpoints, I'd really like to not have them inserted, it's inneficient
for "continue".  Plus, threads hit longjmps on their way to
exiting (on linux/pthreads) --- and it's best to avoid it, with all
the nasty longjmp issues (pointer mangling) we have.

> - keeping them disabled unless stepping (and still have infrun ignore
>   longjmp breakpoints when hit in the wrong thread)

Yeah, I can see that working.

> and/or
> - keeping them always disabled, but installing momentary clones in
>   threads that are stepping

Yeah, sounds sort of good too.  I've added a momentary breakpoint
cloning function just a few days ago.  This requires looking up which
threads in the same address space are stepping.  I'm not certain which
version would be uglier.  Currently, it's the address lookup part
that's ineficient.  We could tackle that with per-objfile data, without
making the breakpoints module much aware of stepping.

-- 
Pedro Alves

Re: [00/19] Eliminate some more current_gdbarch uses

[Ulrich Weigand]

> this series of patches makes some more progress towards the goal of
> eliminating the current_gdbarch global.  This series tackles remaining
> instances that are relatively simple to eliminate using existing
> mechanism and relatively "local" changes.
> 
> While some patches in the series do change existing interfaces to
> allow passing architecture information to some lower level, these
> are typically less-frequently used ones that require only limited
> changes to the rest of GDB.
> 
> After this patch series, the remaining instances of current_gdbarch
> are difficult to eliminate, typically within frequently-called
> central infrastructure routines.  A follow-on patch series will
> address those remaining occurrences.
> 
> This patch series was tested with no regressions on amd64-linux,
> powerpc64-linux, spu-elf, s390-ibm-linux, and s390x-ibm-linux.
> Also, I made sure an --enable-targets=all build still succeeds.
> 
> Comments welcome!  I plan on committing the series within a week
> or two (after I'm back from the GCC Summit).

I've now checked this series in, with the exception of:

[03/19] set_longjmp_breakpoint
(I'm still rewriting this as discussed.)

and

[16/19] address class support
(We're still discussing whether a type should always be associated
with an architecture; if so, this patch is no longer needed.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

[rfc] longjmp breakpoints (Re: [00/19] Eliminate some more current_gdbarch uses)

[Ulrich Weigand]

Pedro Alves wrote:
> On Monday 08 June 2009 15:38:39, Ulrich Weigand wrote:
> > and/or
> > - keeping them always disabled, but installing momentary clones in
> > =A0 threads that are stepping
> 
> Yeah, sounds sort of good too.  I've added a momentary breakpoint
> cloning function just a few days ago.  This requires looking up which
> threads in the same address space are stepping.  I'm not certain which
> version would be uglier.  Currently, it's the address lookup part
> that's ineficient.  We could tackle that with per-objfile data, without
> making the breakpoints module much aware of stepping.

This patch implements the idea of maintaining "master copies" of the
longjmp breakpoints that are created at the same places where overlay
event breakpoints are created today, and then installing momentary
clones while we want them to be active within a thread.

What do you think?

Tested on spu-elf.

Bye,
Ulrich


ChangeLog:

	* breakpoint.h (set_longjmp_breakpoint): Add THREAD argument.
	(enum bptype): Add bp_longjmp_master.

	* breakpoint.c (create_longjmp_master_breakpoint): New function.
	(update_breakpoints_after_exec): Handle bp_longjmp_master
	breakpoints.  Call create_longjmp_master_breakpoint.
	(print_it_typical, bpstat_stop_status, bpstat_what,
	print_one_breakpoint_location, allocate_bp_location, mention,
	delete_command, breakpoint_re_set_one): Handle bp_longjmp_master.
	(breakpoint_re_set): Call create_longjmp_master_breakpoint.
	(create_longjmp_breakpoint): Delete.
	(set_longjmp_breakpoint): Add THREAD argument.  Reimplement
	to install momentary clones of bp_longjmp_master breakpoints.

	* infcmd.c (step_1): Pass thread to set_longjmp_breakpoint.


Index: gdb-head/gdb/breakpoint.c
===================================================================
--- gdb-head.orig/gdb/breakpoint.c
+++ gdb-head/gdb/breakpoint.c
@@ -1507,6 +1507,31 @@ create_overlay_event_breakpoint (char *f
   update_global_location_list (1);
 }
 
+static void
+create_longjmp_master_breakpoint (char *func_name)
+{
+  struct objfile *objfile;
+
+  ALL_OBJFILES (objfile)
+    {
+      struct breakpoint *b;
+      struct minimal_symbol *m;
+
+      if (!gdbarch_get_longjmp_target_p (get_objfile_arch (objfile)))
+	continue;
+
+      m = lookup_minimal_symbol_text (func_name, objfile);
+      if (m == NULL)
+        continue;
+
+      b = create_internal_breakpoint (SYMBOL_VALUE_ADDRESS (m),
+                                      bp_longjmp_master);
+      b->addr_string = xstrdup (func_name);
+      b->enable_state = bp_disabled;
+    }
+  update_global_location_list (1);
+}
+
 void
 update_breakpoints_after_exec (void)
 {
@@ -1535,8 +1560,9 @@ update_breakpoints_after_exec (void)
       }
 
     /* Thread event breakpoints must be set anew after an exec(),
-       as must overlay event breakpoints.  */
-    if (b->type == bp_thread_event || b->type == bp_overlay_event)
+       as must overlay event and longjmp master breakpoints.  */
+    if (b->type == bp_thread_event || b->type == bp_overlay_event
+	|| b->type == bp_longjmp_master)
       {
 	delete_breakpoint (b);
 	continue;
@@ -1608,6 +1634,10 @@ update_breakpoints_after_exec (void)
   }
   /* FIXME what about longjmp breakpoints?  Re-create them here?  */
   create_overlay_event_breakpoint ("_ovly_debug_event");
+  create_longjmp_master_breakpoint ("longjmp");
+  create_longjmp_master_breakpoint ("_longjmp");
+  create_longjmp_master_breakpoint ("siglongjmp");
+  create_longjmp_master_breakpoint ("_siglongjmp");
 }
 
 int
@@ -2426,6 +2456,12 @@ print_it_typical (bpstat bs)
       result = PRINT_NOTHING;
       break;
 
+    case bp_longjmp_master:
+      /* These should never be enabled.  */
+      printf_filtered (_("Longjmp Master Breakpoint: gdb should not stop!\n"));
+      result = PRINT_NOTHING;
+      break;
+
     case bp_watchpoint:
     case bp_hardware_watchpoint:
       annotate_watchpoint (b->number);
@@ -3119,7 +3155,8 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
     if (!bs->stop)
       continue;
 
-    if (b->type == bp_thread_event || b->type == bp_overlay_event)
+    if (b->type == bp_thread_event || b->type == bp_overlay_event
+	|| b->type == bp_longjmp_master)
       /* We do not stop for these.  */
       bs->stop = 0;
     else
@@ -3403,6 +3440,7 @@ bpstat_what (bpstat bs)
 	  break;
 	case bp_thread_event:
 	case bp_overlay_event:
+	case bp_longjmp_master:
 	  bs_class = bp_nostop;
 	  break;
 	case bp_catchpoint:
@@ -3529,6 +3567,7 @@ print_one_breakpoint_location (struct br
     {bp_shlib_event, "shlib events"},
     {bp_thread_event, "thread events"},
     {bp_overlay_event, "overlay events"},
+    {bp_longjmp_master, "longjmp master"},
     {bp_catchpoint, "catchpoint"},
     {bp_tracepoint, "tracepoint"},
   };
@@ -3657,6 +3696,7 @@ print_one_breakpoint_location (struct br
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
+      case bp_longjmp_master:
       case bp_tracepoint:
 	if (opts.addressprint)
 	  {
@@ -4274,6 +4314,7 @@ allocate_bp_location (struct breakpoint 
     case bp_shlib_event:
     case bp_thread_event:
     case bp_overlay_event:
+    case bp_longjmp_master:
       loc->loc_type = bp_loc_software_breakpoint;
       break;
     case bp_hardware_breakpoint:
@@ -4428,32 +4469,26 @@ make_breakpoint_permanent (struct breakp
     bl->inserted = 1;
 }
 
-static void
-create_longjmp_breakpoint (char *func_name)
-{
-  struct minimal_symbol *m;
-
-  m = lookup_minimal_symbol_text (func_name, NULL);
-  if (m == NULL)
-    return;
-  set_momentary_breakpoint_at_pc (SYMBOL_VALUE_ADDRESS (m), bp_longjmp);
-  update_global_location_list (1);
-}
-
 /* Call this routine when stepping and nexting to enable a breakpoint
-   if we do a longjmp().  When we hit that breakpoint, call
+   if we do a longjmp() in THREAD.  When we hit that breakpoint, call
    set_longjmp_resume_breakpoint() to figure out where we are going. */
 
 void
-set_longjmp_breakpoint (void)
+set_longjmp_breakpoint (int thread)
 {
-  if (gdbarch_get_longjmp_target_p (current_gdbarch))
-    {
-      create_longjmp_breakpoint ("longjmp");
-      create_longjmp_breakpoint ("_longjmp");
-      create_longjmp_breakpoint ("siglongjmp");
-      create_longjmp_breakpoint ("_siglongjmp");
-    }
+  struct breakpoint *b, *temp;
+
+  /* To avoid having to rescan all objfile symbols at every step,
+     we maintain a list of continually-inserted but always disabled
+     longjmp "master" breakpoints.  Here, we simply create momentary
+     clones of those and enable them for the requested thread.  */
+  ALL_BREAKPOINTS_SAFE (b, temp)
+    if (b->type == bp_longjmp_master)
+      {
+	struct breakpoint *clone = clone_momentary_breakpoint (b);
+	b->type = bp_longjmp;
+	b->thread = thread;
+      }
 }
 
 /* Delete all longjmp breakpoints from THREAD.  */
@@ -5164,6 +5199,7 @@ mention (struct breakpoint *b)
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
+      case bp_longjmp_master:
 	break;
       }
 
@@ -7432,6 +7468,7 @@ delete_command (char *arg, int from_tty)
 	    && b->type != bp_shlib_event
 	    && b->type != bp_thread_event
 	    && b->type != bp_overlay_event
+	    && b->type != bp_longjmp_master
 	    && b->number >= 0)
 	  {
 	    breaks_to_delete = 1;
@@ -7449,6 +7486,7 @@ delete_command (char *arg, int from_tty)
 		&& b->type != bp_shlib_event
 		&& b->type != bp_thread_event
 		&& b->type != bp_overlay_event
+		&& b->type != bp_longjmp_master
 		&& b->number >= 0)
 	      delete_breakpoint (b);
 	  }
@@ -7743,9 +7781,10 @@ breakpoint_re_set_one (void *bint)
     default:
       printf_filtered (_("Deleting unknown breakpoint type %d\n"), b->type);
       /* fall through */
-      /* Delete overlay event breakpoints; they will be reset later by
-         breakpoint_re_set.  */
+      /* Delete overlay event and longjmp master breakpoints; they will be
+	 reset later by breakpoint_re_set.  */
     case bp_overlay_event:
+    case bp_longjmp_master:
       delete_breakpoint (b);
       break;
 
@@ -7797,6 +7836,10 @@ breakpoint_re_set (void)
   input_radix = save_input_radix;
 
   create_overlay_event_breakpoint ("_ovly_debug_event");
+  create_longjmp_master_breakpoint ("longjmp");
+  create_longjmp_master_breakpoint ("_longjmp");
+  create_longjmp_master_breakpoint ("siglongjmp");
+  create_longjmp_master_breakpoint ("_siglongjmp");
 }
 \f
 /* Reset the thread number of this breakpoint:
Index: gdb-head/gdb/breakpoint.h
===================================================================
--- gdb-head.orig/gdb/breakpoint.h
+++ gdb-head/gdb/breakpoint.h
@@ -110,6 +110,13 @@ enum bptype
 
     bp_overlay_event, 
 
+    /* Master copies of longjmp breakpoints.  These are always installed
+       as soon as an objfile containing longjmp is loaded, but they are
+       always disabled.  While necessary, temporary clones of bp_longjmp
+       type will be created and enabled.  */
+
+    bp_longjmp_master,
+
     bp_catchpoint,
 
     bp_tracepoint,
@@ -765,7 +772,7 @@ extern void update_breakpoints_after_exe
    inferior_ptid.  */
 extern int detach_breakpoints (int);
 
-extern void set_longjmp_breakpoint (void);
+extern void set_longjmp_breakpoint (int thread);
 extern void delete_longjmp_breakpoint (int thread);
 
 extern void enable_overlay_breakpoints (void);
Index: gdb-head/gdb/infcmd.c
===================================================================
--- gdb-head.orig/gdb/infcmd.c
+++ gdb-head/gdb/infcmd.c
@@ -831,7 +831,7 @@ step_1 (int skip_subroutines, int single
       if (in_thread_list (inferior_ptid))
  	thread = pid_to_thread_id (inferior_ptid);
 
-      set_longjmp_breakpoint ();
+      set_longjmp_breakpoint (thread);
 
       make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
     }

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] longjmp breakpoints (Re: [00/19] Eliminate some more current_gdbarch uses)

[Pedro Alves]

On Tuesday 23 June 2009 19:04:13, Ulrich Weigand wrote:
> Pedro Alves wrote:
> > On Monday 08 June 2009 15:38:39, Ulrich Weigand wrote:
> > > and/or
> > > - keeping them always disabled, but installing momentary clones in
> > > =A0 threads that are stepping
> > 
> > Yeah, sounds sort of good too.  I've added a momentary breakpoint
> > cloning function just a few days ago.  This requires looking up which
> > threads in the same address space are stepping.  I'm not certain which
> > version would be uglier.  Currently, it's the address lookup part
> > that's ineficient.  We could tackle that with per-objfile data, without
> > making the breakpoints module much aware of stepping.
> 
> This patch implements the idea of maintaining "master copies" of the
> longjmp breakpoints that are created at the same places where overlay
> event breakpoints are created today, and then installing momentary
> clones while we want them to be active within a thread.
> 
> What do you think?

Looks good to me.  Thanks!

> 
> Tested on spu-elf.
> 
> Bye,
> Ulrich
> 
> 
> ChangeLog:
> 
> 	* breakpoint.h (set_longjmp_breakpoint): Add THREAD argument.
> 	(enum bptype): Add bp_longjmp_master.
> 
> 	* breakpoint.c (create_longjmp_master_breakpoint): New function.
> 	(update_breakpoints_after_exec): Handle bp_longjmp_master
> 	breakpoints.  Call create_longjmp_master_breakpoint.
> 	(print_it_typical, bpstat_stop_status, bpstat_what,
> 	print_one_breakpoint_location, allocate_bp_location, mention,
> 	delete_command, breakpoint_re_set_one): Handle bp_longjmp_master.
> 	(breakpoint_re_set): Call create_longjmp_master_breakpoint.
> 	(create_longjmp_breakpoint): Delete.
> 	(set_longjmp_breakpoint): Add THREAD argument.  Reimplement
> 	to install momentary clones of bp_longjmp_master breakpoints.
> 
> 	* infcmd.c (step_1): Pass thread to set_longjmp_breakpoint.
> 
> 
> Index: gdb-head/gdb/breakpoint.c
> ===================================================================
> --- gdb-head.orig/gdb/breakpoint.c
> +++ gdb-head/gdb/breakpoint.c
> @@ -1507,6 +1507,31 @@ create_overlay_event_breakpoint (char *f
>    update_global_location_list (1);
>  }
>  
> +static void
> +create_longjmp_master_breakpoint (char *func_name)
> +{
> +  struct objfile *objfile;
> +
> +  ALL_OBJFILES (objfile)
> +    {
> +      struct breakpoint *b;
> +      struct minimal_symbol *m;
> +
> +      if (!gdbarch_get_longjmp_target_p (get_objfile_arch (objfile)))
> +	continue;
> +
> +      m = lookup_minimal_symbol_text (func_name, objfile);
> +      if (m == NULL)
> +        continue;
> +
> +      b = create_internal_breakpoint (SYMBOL_VALUE_ADDRESS (m),
> +                                      bp_longjmp_master);
> +      b->addr_string = xstrdup (func_name);
> +      b->enable_state = bp_disabled;
> +    }
> +  update_global_location_list (1);
> +}
> +
>  void
>  update_breakpoints_after_exec (void)
>  {
> @@ -1535,8 +1560,9 @@ update_breakpoints_after_exec (void)
>        }
>  
>      /* Thread event breakpoints must be set anew after an exec(),
> -       as must overlay event breakpoints.  */
> -    if (b->type == bp_thread_event || b->type == bp_overlay_event)
> +       as must overlay event and longjmp master breakpoints.  */
> +    if (b->type == bp_thread_event || b->type == bp_overlay_event
> +	|| b->type == bp_longjmp_master)
>        {
>  	delete_breakpoint (b);
>  	continue;
> @@ -1608,6 +1634,10 @@ update_breakpoints_after_exec (void)
>    }
>    /* FIXME what about longjmp breakpoints?  Re-create them here?  */
>    create_overlay_event_breakpoint ("_ovly_debug_event");
> +  create_longjmp_master_breakpoint ("longjmp");
> +  create_longjmp_master_breakpoint ("_longjmp");
> +  create_longjmp_master_breakpoint ("siglongjmp");
> +  create_longjmp_master_breakpoint ("_siglongjmp");
>  }
>  
>  int
> @@ -2426,6 +2456,12 @@ print_it_typical (bpstat bs)
>        result = PRINT_NOTHING;
>        break;
>  
> +    case bp_longjmp_master:
> +      /* These should never be enabled.  */
> +      printf_filtered (_("Longjmp Master Breakpoint: gdb should not stop!\n"));
> +      result = PRINT_NOTHING;
> +      break;
> +
>      case bp_watchpoint:
>      case bp_hardware_watchpoint:
>        annotate_watchpoint (b->number);
> @@ -3119,7 +3155,8 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
>      if (!bs->stop)
>        continue;
>  
> -    if (b->type == bp_thread_event || b->type == bp_overlay_event)
> +    if (b->type == bp_thread_event || b->type == bp_overlay_event
> +	|| b->type == bp_longjmp_master)
>        /* We do not stop for these.  */
>        bs->stop = 0;
>      else
> @@ -3403,6 +3440,7 @@ bpstat_what (bpstat bs)
>  	  break;
>  	case bp_thread_event:
>  	case bp_overlay_event:
> +	case bp_longjmp_master:
>  	  bs_class = bp_nostop;
>  	  break;
>  	case bp_catchpoint:
> @@ -3529,6 +3567,7 @@ print_one_breakpoint_location (struct br
>      {bp_shlib_event, "shlib events"},
>      {bp_thread_event, "thread events"},
>      {bp_overlay_event, "overlay events"},
> +    {bp_longjmp_master, "longjmp master"},
>      {bp_catchpoint, "catchpoint"},
>      {bp_tracepoint, "tracepoint"},
>    };
> @@ -3657,6 +3696,7 @@ print_one_breakpoint_location (struct br
>        case bp_shlib_event:
>        case bp_thread_event:
>        case bp_overlay_event:
> +      case bp_longjmp_master:
>        case bp_tracepoint:
>  	if (opts.addressprint)
>  	  {
> @@ -4274,6 +4314,7 @@ allocate_bp_location (struct breakpoint 
>      case bp_shlib_event:
>      case bp_thread_event:
>      case bp_overlay_event:
> +    case bp_longjmp_master:
>        loc->loc_type = bp_loc_software_breakpoint;
>        break;
>      case bp_hardware_breakpoint:
> @@ -4428,32 +4469,26 @@ make_breakpoint_permanent (struct breakp
>      bl->inserted = 1;
>  }
>  
> -static void
> -create_longjmp_breakpoint (char *func_name)
> -{
> -  struct minimal_symbol *m;
> -
> -  m = lookup_minimal_symbol_text (func_name, NULL);
> -  if (m == NULL)
> -    return;
> -  set_momentary_breakpoint_at_pc (SYMBOL_VALUE_ADDRESS (m), bp_longjmp);
> -  update_global_location_list (1);
> -}
> -
>  /* Call this routine when stepping and nexting to enable a breakpoint
> -   if we do a longjmp().  When we hit that breakpoint, call
> +   if we do a longjmp() in THREAD.  When we hit that breakpoint, call
>     set_longjmp_resume_breakpoint() to figure out where we are going. */
>  
>  void
> -set_longjmp_breakpoint (void)
> +set_longjmp_breakpoint (int thread)
>  {
> -  if (gdbarch_get_longjmp_target_p (current_gdbarch))
> -    {
> -      create_longjmp_breakpoint ("longjmp");
> -      create_longjmp_breakpoint ("_longjmp");
> -      create_longjmp_breakpoint ("siglongjmp");
> -      create_longjmp_breakpoint ("_siglongjmp");
> -    }
> +  struct breakpoint *b, *temp;
> +
> +  /* To avoid having to rescan all objfile symbols at every step,
> +     we maintain a list of continually-inserted but always disabled
> +     longjmp "master" breakpoints.  Here, we simply create momentary
> +     clones of those and enable them for the requested thread.  */
> +  ALL_BREAKPOINTS_SAFE (b, temp)
> +    if (b->type == bp_longjmp_master)
> +      {
> +	struct breakpoint *clone = clone_momentary_breakpoint (b);
> +	b->type = bp_longjmp;
> +	b->thread = thread;
> +      }
>  }
>  
>  /* Delete all longjmp breakpoints from THREAD.  */
> @@ -5164,6 +5199,7 @@ mention (struct breakpoint *b)
>        case bp_shlib_event:
>        case bp_thread_event:
>        case bp_overlay_event:
> +      case bp_longjmp_master:
>  	break;
>        }
>  
> @@ -7432,6 +7468,7 @@ delete_command (char *arg, int from_tty)
>  	    && b->type != bp_shlib_event
>  	    && b->type != bp_thread_event
>  	    && b->type != bp_overlay_event
> +	    && b->type != bp_longjmp_master
>  	    && b->number >= 0)
>  	  {
>  	    breaks_to_delete = 1;
> @@ -7449,6 +7486,7 @@ delete_command (char *arg, int from_tty)
>  		&& b->type != bp_shlib_event
>  		&& b->type != bp_thread_event
>  		&& b->type != bp_overlay_event
> +		&& b->type != bp_longjmp_master
>  		&& b->number >= 0)
>  	      delete_breakpoint (b);
>  	  }
> @@ -7743,9 +7781,10 @@ breakpoint_re_set_one (void *bint)
>      default:
>        printf_filtered (_("Deleting unknown breakpoint type %d\n"), b->type);
>        /* fall through */
> -      /* Delete overlay event breakpoints; they will be reset later by
> -         breakpoint_re_set.  */
> +      /* Delete overlay event and longjmp master breakpoints; they will be
> +	 reset later by breakpoint_re_set.  */
>      case bp_overlay_event:
> +    case bp_longjmp_master:
>        delete_breakpoint (b);
>        break;
>  
> @@ -7797,6 +7836,10 @@ breakpoint_re_set (void)
>    input_radix = save_input_radix;
>  
>    create_overlay_event_breakpoint ("_ovly_debug_event");
> +  create_longjmp_master_breakpoint ("longjmp");
> +  create_longjmp_master_breakpoint ("_longjmp");
> +  create_longjmp_master_breakpoint ("siglongjmp");
> +  create_longjmp_master_breakpoint ("_siglongjmp");
>  }
>  \f
>  /* Reset the thread number of this breakpoint:
> Index: gdb-head/gdb/breakpoint.h
> ===================================================================
> --- gdb-head.orig/gdb/breakpoint.h
> +++ gdb-head/gdb/breakpoint.h
> @@ -110,6 +110,13 @@ enum bptype
>  
>      bp_overlay_event, 
>  
> +    /* Master copies of longjmp breakpoints.  These are always installed
> +       as soon as an objfile containing longjmp is loaded, but they are
> +       always disabled.  While necessary, temporary clones of bp_longjmp
> +       type will be created and enabled.  */
> +
> +    bp_longjmp_master,
> +
>      bp_catchpoint,
>  
>      bp_tracepoint,
> @@ -765,7 +772,7 @@ extern void update_breakpoints_after_exe
>     inferior_ptid.  */
>  extern int detach_breakpoints (int);
>  
> -extern void set_longjmp_breakpoint (void);
> +extern void set_longjmp_breakpoint (int thread);
>  extern void delete_longjmp_breakpoint (int thread);
>  
>  extern void enable_overlay_breakpoints (void);
> Index: gdb-head/gdb/infcmd.c
> ===================================================================
> --- gdb-head.orig/gdb/infcmd.c
> +++ gdb-head/gdb/infcmd.c
> @@ -831,7 +831,7 @@ step_1 (int skip_subroutines, int single
>        if (in_thread_list (inferior_ptid))
>   	thread = pid_to_thread_id (inferior_ptid);
>  
> -      set_longjmp_breakpoint ();
> +      set_longjmp_breakpoint (thread);
>  
>        make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
>      }
> 



-- 
Pedro Alves

Re: [rfc] longjmp breakpoints (Re: [00/19] Eliminate some more current_gdbarch uses)

[Ulrich Weigand]

Pedro Alves wrote:

> > This patch implements the idea of maintaining "master copies" of the
> > longjmp breakpoints that are created at the same places where overlay
> > event breakpoints are created today, and then installing momentary
> > clones while we want them to be active within a thread.
> > 
> > What do you think?
> 
> Looks good to me.  Thanks!

Thanks for the review!

I've noticed the patch as posted as a silly bug:

> > +  ALL_BREAKPOINTS_SAFE (b, temp)
> > +    if (b->type == bp_longjmp_master)
> > +      {
> > +	struct breakpoint *clone = clone_momentary_breakpoint (b);
> > +	b->type = bp_longjmp;
> > +	b->thread = thread;
> > +      }

This obviously needs to set clone->type and clone->thread instead.

I've now re-tested and checked in the fixed version.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com