remove global stop_bpstat dependency from breakpoints module

remove global stop_bpstat dependency from breakpoints module

[Pedro Alves]

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

Hi,

Currently, GDB assumes there can only be one stop_bpstat.  This is OK in 
all-stop mode, as there can only be one even that the user has to care for.

There are two uses of stop_bpstat outside of infrun.c, that is, outside
of the path that handles an event.  

The first, is in infcmd.c:continue_command, where we
implement setting a proceed count on the breakpoint we're stopped at:

 (gdb) help continue
 Continue program being debugged, after signal or breakpoint.
 If proceeding from breakpoint, a number N may be used as an argument,
 which means to set the ignore count of that breakpoint to N - 1 (so that
 the breakpoint won't break until the Nth time it is reached).

  /* If have argument (besides '&'), set proceed count of breakpoint
     we stopped at.  */
  if (proc_count_exp != NULL)
    {
      bpstat bs = stop_bpstat;
      int num, stat;
      int stopped = 0;

      while ((stat = bpstat_num (&bs, &num)) != 0)
	if (stat > 0)
	  {
	    set_ignore_count (num,
			      parse_and_eval_long (proc_count_exp) - 1,
			      from_tty);
	    /* set_ignore_count prints a message ending with a period.
	       So print two spaces before "Continuing.".  */
	    if (from_tty)
	      printf_filtered ("  ");
	    stopped = 1;
	  }

      if (!stopped && from_tty)
	{
	  printf_filtered
	    ("Not stopped at any breakpoint; argument ignored.\n");
	}
    }

I had missed this on the non-stop series.  I'll need to context switch
stop_bpstat in non-stop mode, because there, we'll have simultaneous
independant stop events, one per thread, and each should have its own
stop_bpstat.  That's a small change to patch 4 in series I posted.

The second place is in breakpoints.c:delete_breakpoint.  When we delete
a breakpoint, we're looking through the stop_bpstat chain for a matching
breakpoint, so we can clear the location from it:

void
delete_breakpoint (struct breakpoint *bpt)
{
...
 /* Be sure no bpstat's are pointing at it after it's been freed.  */
  /* FIXME, how can we find all bpstat's?
     We just check stop_bpstat for now.  Note that we cannot just
     remove bpstats pointing at bpt from the stop_bpstat list
     entirely, as breakpoint commands are associated with the bpstat;
     if we remove it here, then the later call to
         bpstat_do_actions (&stop_bpstat);
     in event-top.c won't do anything, and temporary breakpoints
     with commands won't work.  */
  for (bs = stop_bpstat; bs; bs = bs->next)
    if (bs->breakpoint_at && bs->breakpoint_at->owner == bpt)
      {
	bs->breakpoint_at = NULL;
	bs->old_val = NULL;
	/* bs->commands will be freed later.  */
      }
...
}

(Nevermind that bs->old_val is leaking.)

Now, if we make a stop_bpstat per-thread, due to context-switching
this will only update the stop_stat that corresponds to the
current thread, leaving the stop_bpstats of the other threads,
if stopped at the breakpoint we're deleting, with dangling pointers.

One way to fix it, would be to also loop through all threads to update
their version of stop_bpstat, but I'd like better.

Another form, is to remove the dependency on knowing about stop_bpstat
at all.  That is what this patch implements.  To do that, I added
reference counting to bp_locations, and made sure the counts are updated
at the appropriate places.  To mark the location as ready for garbage
collection, I've added a new bp_loc_dead location type.

Does this look sane?  I've tested this on x86_64-unknown-linux
with and without breakpoints always-inserted on, without
any regressions.

-- 
Pedro Alves

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

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

	* breakpoint.h (enum bp_loc_type): Add bp_loc_dead.
	(struct bp_location): Add refcount.
	(struct bpstats): Remove const qualifier from breakpoint_at.

	* breakpoint.c (invalid_bp_location): New global.
	(bp_location_is_dead, bp_location_ref, bp_location_unref): New
	functions.
	(bpstat_free): Decrement breakpoint_at's reference count.
	(bpstat_copy): Increment breakpoint_at's reference count.
	(bpstat_find_breakpoint): Check for dead bp_location instead of
	checking for valid pointer.
	(bpstat_find_step_resume_breakpoint, bpstat_num)
	(print_it_typical): Ditto.
	(print_bp_stop_message): If location is dead, return
	PRINT_UNKNOWN.
	(bpstat_alloc): Remove const qualifier from BL parameter.
	Increment breakpoint_at's reference count.
	(bpstat_stop_status): When updating watchpoints locations, don't
	check bs->stop.  Decrement breakpoint_at's reference count, and
	point bpstat at invalid_bp_location.
	(bpstat_what): Check for dead bp_location instead of checking for
	NULL pointer.
	(allocate_bp_location): Set refcount to 1.
	(free_bp_location): Decrement location's reference count and mark
	it as dead instead of freeing it.
	(breakpoint_auto_delete): Check for dead bp_location instead of
	checking for NULL pointer.
	(delete_breakpoint): Remove redundant checks.  Don't look over the
	bsstats for a matching breakpoint to clear it.

---
 gdb/breakpoint.c |  177 +++++++++++++++++++++++++++++--------------------------
 gdb/breakpoint.h |   17 ++++-
 2 files changed, 109 insertions(+), 85 deletions(-)

Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h	2008-05-08 01:32:05.000000000 +0100
+++ src/gdb/breakpoint.h	2008-05-08 01:55:30.000000000 +0100
@@ -215,6 +215,10 @@ struct bp_target_info
 
 enum bp_loc_type
 {
+  /* This bp_location is now dead, but there are still references to
+     it somewhere.  A dead location will never appear in the global
+     location list.  */
+  bp_loc_dead,
   bp_loc_software_breakpoint,
   bp_loc_hardware_breakpoint,
   bp_loc_hardware_watchpoint,
@@ -230,7 +234,14 @@ struct bp_location
   /* Pointer to the next breakpoint location, in a global
      list of all breakpoint locations.  */
   struct bp_location *global_next;
- 
+
+  /* The lifetime of a bp_location object is managed with refcounting.
+     This is because pointers to the same bp_location object may be
+     stored in several places (e.g., struct breakpoint and bpstats).
+     A bp_location that is not useful anymore is identified by having
+     bp_loc_dead type.  */
+  int refcount;
+
   /* Type of this breakpoint location.  */
   enum bp_loc_type loc_type;
 
@@ -273,7 +284,7 @@ struct bp_location
      bp_loc_other.  */
   CORE_ADDR address;
 
-  /* For hardware watchpoints, the size of data ad ADDRESS being watches.  */
+  /* For hardware watchpoints, the size of data at ADDRESS being watched.  */
   int length;
 
   /* Type of hardware watchpoint. */
@@ -632,7 +643,7 @@ struct bpstats
        place, and a bpstat reflects the fact that both have been hit.  */
     bpstat next;
     /* Breakpoint that we are at.  */
-    const struct bp_location *breakpoint_at;
+    struct bp_location *breakpoint_at;
     /* Commands left to be done.  */
     struct command_line *commands;
     /* Old value associated with a watchpoint.  */
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2008-05-08 01:32:05.000000000 +0100
+++ src/gdb/breakpoint.c	2008-05-08 01:38:11.000000000 +0100
@@ -114,7 +114,7 @@ static void breakpoints_info (char *, in
 
 static void breakpoint_1 (int, int);
 
-static bpstat bpstat_alloc (const struct bp_location *, bpstat);
+static bpstat bpstat_alloc (struct bp_location *, bpstat);
 
 static int breakpoint_cond_eval (void *);
 
@@ -203,6 +203,36 @@ static int is_hardware_watchpoint (struc
 
 static void insert_breakpoint_locations (void);
 
+/* This exists so we can make bpstat->breakpoint_at point somewhere in
+   bpstat_stop_status, when rebuilding the locations of
+   watchpoints.  */
+static struct bp_location invalid_bp_location = { NULL, NULL, 0, bp_loc_dead };
+
+static int
+bp_location_is_dead (struct bp_location *bl)
+{
+  return bl->loc_type == bp_loc_dead;
+}
+
+static void
+bp_location_ref (struct bp_location *bl)
+{
+  if (bl == &invalid_bp_location)
+    return;
+
+  bl->refcount++;
+}
+
+static void
+bp_location_unref (struct bp_location *bl)
+{
+  if (bl == &invalid_bp_location)
+    return;
+
+  if (--bl->refcount == 0)
+    xfree (bl);
+}
+
 static const char *
 bpdisp_text (enum bpdisp disp)
 {
@@ -863,7 +893,7 @@ fetch_watchpoint_value (struct expressio
 }
 
 /* Assuming that B is a hardware watchpoint:
-   - Reparse watchpoint expression, is REPARSE is non-zero
+   - Reparse watchpoint expression, if REPARSE is non-zero
    - Evaluate expression and store the result in B->val
    - Update the list of values that must be watched in B->loc.
 
@@ -1956,6 +1986,7 @@ bpstat_free (bpstat bs)
   if (bs->old_val != NULL)
     value_free (bs->old_val);
   free_command_lines (&bs->commands);
+  bp_location_unref (bs->breakpoint_at);
   xfree (bs);
 }
 
@@ -1997,6 +2028,7 @@ bpstat_copy (bpstat bs)
     {
       tmp = (bpstat) xmalloc (sizeof (*tmp));
       memcpy (tmp, bs, sizeof (*tmp));
+      bp_location_ref (tmp->breakpoint_at);
       if (bs->commands != NULL)
 	tmp->commands = copy_command_lines (bs->commands);
       if (bs->old_val != NULL)
@@ -2023,7 +2055,8 @@ bpstat_find_breakpoint (bpstat bsp, stru
 
   for (; bsp != NULL; bsp = bsp->next)
     {
-      if (bsp->breakpoint_at && bsp->breakpoint_at->owner == breakpoint)
+      if (!bp_location_is_dead (bsp->breakpoint_at)
+	  && bsp->breakpoint_at->owner == breakpoint)
 	return bsp;
     }
   return NULL;
@@ -2048,10 +2081,10 @@ bpstat_find_step_resume_breakpoint (bpst
 
   for (; bsp != NULL; bsp = bsp->next)
     {
-      if ((bsp->breakpoint_at != NULL) &&
-	  (bsp->breakpoint_at->owner->type == bp_step_resume) &&
-	  (bsp->breakpoint_at->owner->thread == current_thread || 
-	   bsp->breakpoint_at->owner->thread == -1))
+      if (!bp_location_is_dead (bsp->breakpoint_at)
+	  && bsp->breakpoint_at->owner->type == bp_step_resume
+	  && (bsp->breakpoint_at->owner->thread == current_thread
+	      || bsp->breakpoint_at->owner->thread == -1))
 	return bsp->breakpoint_at->owner;
     }
 
@@ -2072,17 +2105,19 @@ int
 bpstat_num (bpstat *bsp, int *num)
 {
   struct breakpoint *b;
+  bpstat s = *bsp;
 
-  if ((*bsp) == NULL)
+  if (s == NULL)
     return 0;			/* No more breakpoint values */
 
-  /* We assume we'll never have several bpstats that
-     correspond to a single breakpoint -- otherwise, 
-     this function might return the same number more
-     than once and this will look ugly.  */
-  b = (*bsp)->breakpoint_at ? (*bsp)->breakpoint_at->owner : NULL;
   *bsp = (*bsp)->next;
-  if (b == NULL)
+
+  /* We assume we'll never have several bpstats that correspond to a
+     single breakpoint -- otherwise, this function might return the
+     same number more than once and this will look ugly.  */
+  if (!bp_location_is_dead (s->breakpoint_at))
+    b = s->breakpoint_at->owner;
+  else
     return -1;			/* breakpoint that's been deleted since */
 
   *num = b->number;		/* We have its number */
@@ -2247,9 +2282,8 @@ print_it_typical (bpstat bs)
   int bp_temp = 0;  
   stb = ui_out_stream_new (uiout);
   old_chain = make_cleanup_ui_out_stream_delete (stb);
-  /* bs->breakpoint_at can be NULL if it was a momentary breakpoint
-     which has since been deleted.  */
-  if (bs->breakpoint_at == NULL)
+  if (bp_location_is_dead (bs->breakpoint_at))
+    /* A momentary breakpoint which has since been deleted.  */
     return PRINT_UNKNOWN;
   bl = bs->breakpoint_at;
   b = bl->owner;
@@ -2462,13 +2496,16 @@ print_bp_stop_message (bpstat bs)
 
     case print_it_normal:
       {
-	const struct bp_location *bl = bs->breakpoint_at;
-	struct breakpoint *b = bl ? bl->owner : NULL;
-	
+	struct breakpoint *b;
+
+	if (bp_location_is_dead (bs->breakpoint_at))
+	  return PRINT_UNKNOWN;
+
+	b = bs->breakpoint_at->owner;
+
 	/* Normal case.  Call the breakpoint's print_it method, or
 	   print_it_typical.  */
-	/* FIXME: how breakpoint can ever be NULL here?  */
-	if (b != NULL && b->ops != NULL && b->ops->print_it != NULL)
+	if (b->ops != NULL && b->ops->print_it != NULL)
 	  return b->ops->print_it (b);
 	else
 	  return print_it_typical (bs);
@@ -2542,13 +2579,14 @@ breakpoint_cond_eval (void *exp)
 /* Allocate a new bpstat and chain it to the current one.  */
 
 static bpstat
-bpstat_alloc (const struct bp_location *bl, bpstat cbs /* Current "bs" value */ )
+bpstat_alloc (struct bp_location *bl, bpstat cbs /* Current "bs" value */ )
 {
   bpstat bs;
 
   bs = (bpstat) xmalloc (sizeof (*bs));
   cbs->next = bs;
   bs->breakpoint_at = bl;
+  bp_location_ref (bs->breakpoint_at);
   /* If the condition is false, etc., don't do the commands.  */
   bs->commands = NULL;
   bs->old_val = NULL;
@@ -3011,7 +3049,7 @@ bpstat
 bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid)
 {
   struct breakpoint *b = NULL;
-  const struct bp_location *bl;
+  struct bp_location *bl;
   /* Root of the chain of bpstat's */
   struct bpstats root_bs[1];
   /* Pointer to the last thing in the chain currently.  */
@@ -3026,10 +3064,9 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
 
     /* For hardware watchpoints, we look only at the first location.
        The watchpoint_check function will work on entire expression,
-       not the individual locations.  For read watchopints, the
-       watchpoints_triggered function have checked all locations
-       alrea
-     */
+       not the individual locations.  For read watchpoints, the
+       watchpoints_triggered function has checked all locations
+       already.  */
     if (b->type == bp_hardware_watchpoint && bl != b->loc)
       continue;
 
@@ -3097,15 +3134,13 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
 
   if (bs == NULL)
     for (bs = root_bs->next; bs != NULL; bs = bs->next)
-      if (!bs->stop
-	  && (bs->breakpoint_at->owner->type == bp_hardware_watchpoint
-	      || bs->breakpoint_at->owner->type == bp_read_watchpoint
-	      || bs->breakpoint_at->owner->type == bp_access_watchpoint))
+      if (is_hardware_watchpoint (bs->breakpoint_at->owner))
 	{
 	  /* remove/insert can invalidate bs->breakpoint_at, if this
 	     location is no longer used by the watchpoint.  Prevent
 	     further code from trying to use it.  */
-	  bs->breakpoint_at = NULL;
+	  bp_location_unref (bs->breakpoint_at);
+	  bs->breakpoint_at = &invalid_bp_location;
 	  remove_breakpoints ();
 	  insert_breakpoints ();
 	  break;
@@ -3258,7 +3293,7 @@ bpstat_what (bpstat bs)
   for (; bs != NULL; bs = bs->next)
     {
       enum class bs_class = no_effect;
-      if (bs->breakpoint_at == NULL)
+      if (bp_location_is_dead (bs->breakpoint_at))
 	/* I suspect this can happen if it was a momentary breakpoint
 	   which has since been deleted.  */
 	continue;
@@ -4257,6 +4292,7 @@ allocate_bp_location (struct breakpoint 
   loc->cond = NULL;
   loc->shlib_disabled = 0;
   loc->enabled = 1;
+  loc->refcount = 1;
 
   switch (bp_type)
     {
@@ -4296,15 +4332,19 @@ allocate_bp_location (struct breakpoint 
   return loc;
 }
 
-static void free_bp_location (struct bp_location *loc)
+static void
+free_bp_location (struct bp_location *loc)
 {
   if (loc->cond)
     xfree (loc->cond);
 
   if (loc->function_name)
     xfree (loc->function_name);
-  
-  xfree (loc);
+
+  /* Mark as dead, so bpstats still pointing into it know it was
+     deleted.  */
+  loc->loc_type = bp_loc_dead;
+  bp_location_unref (loc);
 }
 
 /* Helper to set_raw_breakpoint below.  Creates a breakpoint
@@ -6943,7 +6983,8 @@ breakpoint_auto_delete (bpstat bs)
   struct breakpoint *b, *temp;
 
   for (; bs; bs = bs->next)
-    if (bs->breakpoint_at && bs->breakpoint_at->owner->disposition == disp_del
+    if (!bp_location_is_dead (bs->breakpoint_at)
+	&& bs->breakpoint_at->owner->disposition == disp_del
 	&& bs->stop)
       delete_breakpoint (bs->breakpoint_at->owner);
 
@@ -7117,50 +7158,22 @@ delete_breakpoint (struct breakpoint *bp
     }
 
   free_command_lines (&bpt->commands);
-  if (bpt->cond_string != NULL)
-    xfree (bpt->cond_string);
-  if (bpt->addr_string != NULL)
-    xfree (bpt->addr_string);
-  if (bpt->exp != NULL)
-    xfree (bpt->exp);
-  if (bpt->exp_string != NULL)
-    xfree (bpt->exp_string);
-  if (bpt->val != NULL)
-    value_free (bpt->val);
-  if (bpt->source_file != NULL)
-    xfree (bpt->source_file);
-  if (bpt->dll_pathname != NULL)
-    xfree (bpt->dll_pathname);
-  if (bpt->triggered_dll_pathname != NULL)
-    xfree (bpt->triggered_dll_pathname);
-  if (bpt->exec_pathname != NULL)
-    xfree (bpt->exec_pathname);
-
-  /* Be sure no bpstat's are pointing at it after it's been freed.  */
-  /* FIXME, how can we find all bpstat's?
-     We just check stop_bpstat for now.  Note that we cannot just
-     remove bpstats pointing at bpt from the stop_bpstat list
-     entirely, as breakpoint commands are associated with the bpstat;
-     if we remove it here, then the later call to
-         bpstat_do_actions (&stop_bpstat);
-     in event-top.c won't do anything, and temporary breakpoints
-     with commands won't work.  */
-  for (bs = stop_bpstat; bs; bs = bs->next)
-    if (bs->breakpoint_at && bs->breakpoint_at->owner == bpt)
-      {
-	bs->breakpoint_at = NULL;
-	bs->old_val = NULL;
-	/* bs->commands will be freed later.  */
-      }
-
-  /* Now that breakpoint is removed from breakpoint
-     list, update the global location list.  This
-     will remove locations that used to belong to
-     this breakpoint.  Do this before freeing
-     the breakpoint itself, since remove_breakpoint
-     looks at location's owner.  It might be better
-     design to have location completely self-contained,
-     but it's not the case now.  */
+  xfree (bpt->cond_string);
+  xfree (bpt->addr_string);
+  xfree (bpt->exp);
+  xfree (bpt->exp_string);
+  value_free (bpt->val);
+  xfree (bpt->source_file);
+  xfree (bpt->dll_pathname);
+  xfree (bpt->triggered_dll_pathname);
+  xfree (bpt->exec_pathname);
+
+  /* Now that breakpoint is removed from breakpoint list, update the
+     global location list.  This will remove locations that used to
+     belong to this breakpoint.  Do this before freeing the breakpoint
+     itself, since remove_breakpoint looks at location's owner.  It
+     might be better design to have location completely
+     self-contained, but it's not the case now.  */
   update_global_location_list ();
 
 

Re: remove global stop_bpstat dependency from breakpoints module

[Daniel Jacobowitz]

On Thu, May 08, 2008 at 02:22:04AM +0100, Pedro Alves wrote:
> I had missed this on the non-stop series.  I'll need to context switch
> stop_bpstat in non-stop mode, because there, we'll have simultaneous
> independant stop events, one per thread, and each should have its own
> stop_bpstat.  That's a small change to patch 4 in series I posted.

The implementation makes it look like this will work for watchpoints
too.  If we want to preserve that - which is not the documented
behavior - then I agree we should context-switch stop_bpstat.  But
I'm thinking it makes more sense to restrict this to breakpoints,
and search for a breakpoint at the current PC.

What do you think?  Also, does that remove the only reason to
context-switch stop_bpstat?  If so, we don't need this patch at all.

> One way to fix it, would be to also loop through all threads to update
> their version of stop_bpstat, but I'd like better.

If we do need to context-switch stop_bpstat, it seems like checking
all of those would be simpler than this patch as posted.


-- 
Daniel Jacobowitz
CodeSourcery

Re: remove global stop_bpstat dependency from breakpoints module

[Pedro Alves]

A Thursday 05 June 2008 19:03:31, Daniel Jacobowitz wrote:
> On Thu, May 08, 2008 at 02:22:04AM +0100, Pedro Alves wrote:
> > I had missed this on the non-stop series.  I'll need to context switch
> > stop_bpstat in non-stop mode, because there, we'll have simultaneous
> > independant stop events, one per thread, and each should have its own
> > stop_bpstat.  That's a small change to patch 4 in series I posted.
>
> The implementation makes it look like this will work for watchpoints
> too.  If we want to preserve that - which is not the documented
> behavior - then I agree we should context-switch stop_bpstat.  But
> I'm thinking it makes more sense to restrict this to breakpoints,
> and search for a breakpoint at the current PC.

Well, I can't say if we want to preserve that.  I never use
this functionality, so I wouldn't missed it.  The
"ignore" command gets me the same functionality anyway.

Notice that I was proposing context switching it in non-stop
only.  The current implementation sets the ignore count
on the breakpoint that we stopped last.  If we just read current
PC, we may be reading it from the wrong thread:

 Breakpoint 1, at xxx
 thread 2
 c 10

we could use get_last_target_status to get at the
correct PC if we do want to preserve this behaviour in
all-stop.  In non-stop, I'd read from the current thread always.

But, the help doc:

"Continue program being debugged, after signal or breakpoint.
If proceeding from breakpoint, a number N may be used as an argument,
which means to set the ignore count of that breakpoint to N - 1 (so that
the breakpoint won't break until the Nth time it is reached)"

... doesn't make it clear, if "proceeding" was meant to apply
to the last breakpoint, or to the breakpoint under PC.

As I said, I would mind at all to change this to the read
the PC of the current thread.  Just pointing out the current
behaviour.

>
> What do you think?  Also, does that remove the only reason to
> context-switch stop_bpstat?  If so, we don't need this patch at all.
>

Yes.

> > One way to fix it, would be to also loop through all threads to update
> > their version of stop_bpstat, but I'd like better.
>
> If we do need to context-switch stop_bpstat, it seems like checking
> all of those would be simpler than this patch as posted.

Well, simpler yes, but adds even more coupling.  I thought this was
a nice cleanup.  It's mentioned a few times in comments
throughout breakpoints.c.

-- 
Pedro Alves

Re: remove global stop_bpstat dependency from breakpoints module

[Daniel Jacobowitz]

On Thu, Jun 05, 2008 at 07:27:07PM +0100, Pedro Alves wrote:
> As I said, I would mind at all to change this to the read
> the PC of the current thread.  Just pointing out the current
> behaviour.

I think that would be correcting it to the right behavior.  In 99% of
the cases where this is used at all, they mean the same thing.

> > > One way to fix it, would be to also loop through all threads to update
> > > their version of stop_bpstat, but I'd like better.
> >
> > If we do need to context-switch stop_bpstat, it seems like checking
> > all of those would be simpler than this patch as posted.
> 
> Well, simpler yes, but adds even more coupling.  I thought this was
> a nice cleanup.  It's mentioned a few times in comments
> throughout breakpoints.c.

I don't like reference counting as a solution to clear memory
management.  Sometimes it's necessary or helpful, but here I think it
just gives us room to be careless (and have to worry more about
leaks).

-- 
Daniel Jacobowitz
CodeSourcery

Re: remove global stop_bpstat dependency from breakpoints module

[Pedro Alves]

A Thursday 05 June 2008 19:33:47, Daniel Jacobowitz wrote:
> On Thu, Jun 05, 2008 at 07:27:07PM +0100, Pedro Alves wrote:
> > As I said, I would mind at all to change this to the read
> > the PC of the current thread.  Just pointing out the current
> > behaviour.
>
> I think that would be correcting it to the right behavior.  In 99% of
> the cases where this is used at all, they mean the same thing.

I'll just post a patch to do that.

> I don't like reference counting as a solution to clear memory
> management.  Sometimes it's necessary or helpful, but here I think it
> just gives us room to be careless (and have to worry more about
> leaks).

Ok.  Obviously, I disagree.  :-)

-- 
Pedro Alves