Watchpoint on an unloaded shared library(1)

Watchpoint on an unloaded shared library(1)

[Emi SUZUKI]

Hello members,

I've now faced three issues related to a watchpoint on an unloaded
shared library: a segfault causes on GDB when referring to a
watchpoint which is invalid at that time.  Now I will report them
separately.  

To begin with, I will provide a sample to reproduce all the issues: 

-----------------------------
dl-main.c:

#include <dlfcn.h>
#include <err.h>
#include <stdio.h>
#include <stdlib.h>

static void (*sample) (void);

int
main (void)
{
  void *handle;

  if ((handle = dlopen("./libsample.so", RTLD_LAZY)) == NULL)
    errx(2, "dlopen(): %s", dlerror());

  if ((sample = dlsym(handle, "sample")) == NULL)
    errx(2, "dlsym(): %s", dlerror());

  sample ();

  if (dlclose(handle) < 0)
    errx(2, "dlclose(): %s", dlerror());

  return 0;
}

-----------------------------
sample.c:

#include <stdio.h>

int sample_glob = 1;

void
sample (void)
{
  puts ("sample of shared library");
  ++sample_glob;
}

-----------------------------
Build:

$ gcc -c -g -Wall sample.c
$ gcc -o libsample.so -shared sample.o
$ gcc -c -g -Wall dl-test.c
$ gcc -o dl-test dl-test.o -ldl

-----------------------------

And the first issue:

-----------------------------
$ gdb ./dl-test
GNU gdb (GDB) 6.8.50.20081114-cvs
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-pc-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
(gdb) start
Temporary breakpoint 1 at 0x80484e5: file dl-test.c, line 13.
Starting program: /home/suzuki/test/dl-test

Temporary breakpoint 1, main () at dl-test.c:14
13        if ((handle = dlopen("./libsample.so", RTLD_LAZY)) == NULL)
(gdb) next
16        if ((sample = dlsym(handle, "sample")) == NULL)
(gdb) watch sample_glob
Hardware watchpoint 2: sample_glob
(gdb) continue
Continuing.
sample of shared library
Hardware watchpoint 2: sample_glob

Old value = 1
New value = 2
sample () at sample.c:10
10      }
(gdb) disable 2
(gdb) c
Continuing.

Program exited normally.
(gdb) start
Temporary breakpoint 3 at 0x80484e5: file dl-test.c, line 13.
Starting program: /homer/suzuki/test/dl-test
Error in re-setting breakpoint 2: No symbol "sample_glob" in current context.
Error in re-setting breakpoint 2: No symbol "sample_glob" in current context.
Error in re-setting breakpoint 2: No symbol "sample_glob" in current context.
Error in re-setting breakpoint 2: No symbol "sample_glob" in current context.

Temporary breakpoint 3, main () at dl-test.c:13
13        if ((handle = dlopen("./libsample.so", RTLD_LAZY)) == NULL)
(gdb) enable 2
sample of shared library
Segmentation fault

$ 
-----------------------------

The cause is rather simple: the pointer to struct expression in struct
breakpoint (`bpt->exp') is set to NULL in breakpoint_re_set, when the
program was restarted and the shared library in which the watchpoint
expression is valid has not been loaded yet.  However,
do_enable_breakpoint does not care about it.  

The patch below addresses to the issue.  Is that OK?


2008-11-20  Emi Suzuki	<emi-suzuki@tjsys.co.jp>

	* breakpoint.c (do_enable_breakpoint): Inform the user and
	return from the function if the expression of a watchpoint is
	invalid and cannot be updated.  


diff src/gdb/breakpoint.c.orig src/gdb/breakpoint.c
--- src/gdb/breakpoint.c.orig   2008-11-20 18:52:13.000000000 +0900
+++ src/gdb/breakpoint.c        2008-11-20 18:52:56.000000000 +0900
@@ -7756,6 +7756,18 @@ is valid is not currently in scope.\n"),
            }
          select_frame (fr);
        }
+
+      if (bpt->exp == NULL)
+       {
+         char *s = bpt->exp_string;
+         if (!gdb_parse_exp_1 (&s, bpt->exp_valid_block, 0, &bpt->exp))
+           {
+             printf_filtered (_("\
+Cannot enable watchpoint %d because the block in which its expression\n\
+is valid is not exist.\n"), bpt->number);
+             return;
+           }
+       }

       if (bpt->val)
        value_free (bpt->val);


My best regards,
-- 
Emi SUZUKI / emi-suzuki at tjsys.co.jp

Re: Watchpoint on an unloaded shared library(1)

[Joel Brobecker]

Emi,

Thanks for sending this patch.

> 2008-11-20  Emi Suzuki	<emi-suzuki@tjsys.co.jp>
> 
> 	* breakpoint.c (do_enable_breakpoint): Inform the user and
> 	return from the function if the expression of a watchpoint is
> 	invalid and cannot be updated.  

How about replace all the code that's inside

  if (bpt->type == bp_watchpoint || 
      bpt->type == bp_hardware_watchpoint ||
      bpt->type == bp_read_watchpoint ||
      bpt->type == bp_access_watchpoint)
    {
      [re-create a lot of stuff for our watchpoint...]

by a call to update_watchpoint (bpt, 1)?  Would that work in your case? 

When I looked at what update_watchpoint does and what do_enable_breakpoint
does for watchpoints, it seemed to me that do_enable_breakpoint was trying
to do the same, except that it was missing a few pieces. Does anybody see
a reason for the code almost-duplication here?

-- 
Joel

Re: Watchpoint on an unloaded shared library(1)

[Emi SUZUKI]

Hello Joel,

Thank you for reviewing!

From: Joel Brobecker <brobecker at adacore.com>
Subject: Re: Watchpoint on an unloaded shared library(1)
Date: Sat, 13 Dec 2008 19:05:17 +0400

> How about replace all the code that's inside
> 
>   if (bpt->type == bp_watchpoint || 
>       bpt->type == bp_hardware_watchpoint ||
>       bpt->type == bp_read_watchpoint ||
>       bpt->type == bp_access_watchpoint)
>     {
>       [re-create a lot of stuff for our watchpoint...]
> 
> by a call to update_watchpoint (bpt, 1)?  Would that work in your case? 

It works without merging the missing code each other, as long as we
don't have to care the hardware watchpoint resource count here: if the
user sets other watchpoints while the disabled hardware watchpoints
exist, re-enabling the disabled ones might fail out of the shortage of
resources.  

Actually, calling update_watchpoint without resource checking works on
almost all the target which has the hardware watchpoint resources,
since TARGET_CAN_USE_HARDWARE_WATCHPOINT always returns 1 on those
targets.  Instead of stop enabling watchpoints in do_enable_breakpoint, 
out of the watchpoint resources would be reported to the user when GDB
actually address to them by target_insert_watchpoint.  

This mechanism does not work on ppc-linux native target which has
DABR: it would just overwrite the register value each time when
target_insert_watchpoint is called.  However, I've found that the
watchpoint resource count checking feature itself does not work at all
on that target...  ppc_linux_check_watch_resources, which replaces
TARGET_CAN_USE_HARDWARE_WATCHPOINT on a ppc-linux target build, never
return the negative value.  

Maybe we should revise them separately...

> When I looked at what update_watchpoint does and what do_enable_breakpoint
> does for watchpoints, it seemed to me that do_enable_breakpoint was trying
> to do the same, except that it was missing a few pieces. Does anybody see
> a reason for the code almost-duplication here?

I know at least when `update_watchpoint' was introduced:

  http://sourceware.org/ml/gdb-patches/2008-01/msg00543.html

In the thread includes the above mail, Jim Blandy and Vladmir Plus
discussed about the timing of refreshing watchpoint expressions and
changed some of them.  I guess that those codes have got more smilar
along with they rebuild the logic.  


My best regards, 
-- 
Emi SUZUKI / emi-suzuki at tjsys.co.jp

Re: Watchpoint on an unloaded shared library(1)

[Joel Brobecker]

> > How about replace all the code that's inside
> > 
> >   if (bpt->type == bp_watchpoint || 
> >       bpt->type == bp_hardware_watchpoint ||
> >       bpt->type == bp_read_watchpoint ||
> >       bpt->type == bp_access_watchpoint)
> >     {
> >       [re-create a lot of stuff for our watchpoint...]
> > 
> > by a call to update_watchpoint (bpt, 1)?  Would that work in your case? 
> 
> It works without merging the missing code each other, as long as we
> don't have to care the hardware watchpoint resource count here: if the
> user sets other watchpoints while the disabled hardware watchpoints
> exist, re-enabling the disabled ones might fail out of the shortage of
> resources.  

That's a good point.

I still think this might work. I am hanging on to the idea because
I'm seeing a few cases in this file where we have two pieces of code
that are almost the same and yet not quite the same. I really dislike
this code semi-duplication. I'm also seeing potential disasters such
as a reference to a free'ed value, which makes me want to share the
(correct!) code more.

So, what I think we should do, in this case, is call update_watchpoint
within an exception wrapper. If we fail to enable the watchpoint,
I think we want to print a message that's similar to the message
printed when re-setting it, something like this:

   (gdb) enable 2
   Cannot enable watchpoint 2: No symbol "sample_glob" in current context.

Rather than just printing:

   (gdb) enable 2
   No symbol "sample_glob" in current context.

If the update_watchpoint completes without exception, then there
are two cases:

    1. The scope associated to the watchpoint no longer exists.
       update_watchpoint has therefore "deleted" it, which actually
       means has scheduled it for deletion at the next stop
       (by setting b->disposition to disp_del_at_next_stop).
       update_watchpoint has already printed an error message
       about it, so there isn't much else to do.

    2. Everything went fine in terms of updating the watchpoint.
       We need to confirm that we still have enough hardware
       resources to effectively insert the watchpoint.

So, in the end, the code should look like this:

   if (bpt->type == bp_watchpoint || 
       bpt->type == bp_hardware_watchpoint ||
       bpt->type == bp_read_watchpoint ||
       bpt->type == bp_access_watchpoint)
     {
       struct gdb_exception e;

       TRY_CATCH (e, RETURN_MASK_ALL)
         {
           update_watchpoint (...);
         }
       if (e.reason < 0)
         {
           exception_fprintf (gdb_strerr, e, "Cannot enable watchpoint %d:",
                              b->number);
           return;
         }
       else
         {
           [check hardware support here]
           if (target_resources_ok < 0)
             {
               printf_filtered (error_message);
               return;
             }
         }
     }

Could you see if that works?

-- 
Joel

Re: Watchpoint on an unloaded shared library(1)

[Joel Brobecker]

> > > How about replace all the code that's inside
> > > 
> > >   if (bpt->type == bp_watchpoint || 
> > >       bpt->type == bp_hardware_watchpoint ||
> > >       bpt->type == bp_read_watchpoint ||
> > >       bpt->type == bp_access_watchpoint)
> > >     {
> > >       [re-create a lot of stuff for our watchpoint...]
> > > 
> > > by a call to update_watchpoint (bpt, 1)?  Would that work in your case? 
> > 
> > It works without merging the missing code each other, as long as we
> > don't have to care the hardware watchpoint resource count here: if the
> > user sets other watchpoints while the disabled hardware watchpoints
> > exist, re-enabling the disabled ones might fail out of the shortage of
> > resources.  
> 
> That's a good point.

As it turns out, I think we just got lucky thanks to Jan:
http://www.sourceware.org/ml/gdb-patches/2008-12/msg00363.html.

update_watchpoint now automagically downgrades hardware watchpoints
into a software watchpoints if needed. So we can get rid of that part.
So all we have to do, now is:

>    if (bpt->type == bp_watchpoint || 
>        bpt->type == bp_hardware_watchpoint ||
>        bpt->type == bp_read_watchpoint ||
>        bpt->type == bp_access_watchpoint)
>      {
>        struct gdb_exception e;
> 
>        TRY_CATCH (e, RETURN_MASK_ALL)
>          {
>            update_watchpoint (...);
>          }
>        if (e.reason < 0)
>          {
>            exception_fprintf (gdb_strerr, e, "Cannot enable watchpoint %d:",
>                               b->number);
>            return;
>          }

-- 
Joel

Re: Watchpoint on an unloaded shared library(1)

[Emi SUZUKI]

Hello Joel, 

From: Joel Brobecker <brobecker at adacore.com>
Subject: Re: Watchpoint on an unloaded shared library(1)
Date: Mon, 22 Dec 2008 07:27:58 +0400

>> > > How about replace all the code that's inside
>> > > 
>> > >   if (bpt->type == bp_watchpoint || 
>> > >       bpt->type == bp_hardware_watchpoint ||
>> > >       bpt->type == bp_read_watchpoint ||
>> > >       bpt->type == bp_access_watchpoint)
>> > >     {
>> > >       [re-create a lot of stuff for our watchpoint...]
>> > > 
>> > > by a call to update_watchpoint (bpt, 1)?  Would that work in your case? 
>> > 
>> > It works without merging the missing code each other, as long as we
>> > don't have to care the hardware watchpoint resource count here: if the
>> > user sets other watchpoints while the disabled hardware watchpoints
>> > exist, re-enabling the disabled ones might fail out of the shortage of
>> > resources.  
>> 
>> That's a good point.
> 
> As it turns out, I think we just got lucky thanks to Jan:
> http://www.sourceware.org/ml/gdb-patches/2008-12/msg00363.html.
> 
> update_watchpoint now automagically downgrades hardware watchpoints
> into a software watchpoints if needed. So we can get rid of that part.
> So all we have to do, now is:
> 
>>    if (bpt->type == bp_watchpoint || 
>>        bpt->type == bp_hardware_watchpoint ||
>>        bpt->type == bp_read_watchpoint ||
>>        bpt->type == bp_access_watchpoint)
>>      {
>>        struct gdb_exception e;
>> 
>>        TRY_CATCH (e, RETURN_MASK_ALL)
>>          {
>>            update_watchpoint (...);
>>          }
>>        if (e.reason < 0)
>>          {
>>            exception_fprintf (gdb_strerr, e, "Cannot enable watchpoint %d:",
>>                               b->number);
>>            return;
>>          }

This does work for me, except one duplication of the codes (that you
may have noticed already):

  static void
  do_enable_breakpoint (struct breakpoint *bpt, enum bpdisp disposition)
  {
    int target_resources_ok, other_type_used;
    struct value *mark;
  
    if (bpt->type == bp_hardware_breakpoint)
      {
        int i;
        i = hw_breakpoint_used_count ();
        target_resources_ok = 
          TARGET_CAN_USE_HARDWARE_WATCHPOINT (bp_hardware_breakpoint, 
                                              i + 1, 0);
        if (target_resources_ok == 0)
          error (_("No hardware breakpoint support in the target."));
        else if (target_resources_ok < 0)
          error (_("Hardware breakpoints used exceeds limit."));
      }

That should be done in update_watchpoint.  

I would try to address incapabilities on PPC after those codes are
committed.  Thank you for taking care of it!


I wonder if I should greet for Christmas here...
-- 
Emi SUZUKI / emi-suzuki at tjsys.co.jp

Re: Watchpoint on an unloaded shared library(1)

[Joel Brobecker]

> This does work for me, except one duplication of the codes (that you
> may have noticed already):
> 
>   static void
>   do_enable_breakpoint (struct breakpoint *bpt, enum bpdisp disposition)
>   {
>     int target_resources_ok, other_type_used;
>     struct value *mark;
>   
>     if (bpt->type == bp_hardware_breakpoint)

Not quite: As you can see, this part of the code deals with hardware
*breakpoints*, not watchpoints. The code is slightly different because:

>         i = hw_breakpoint_used_count ();

We get the current count of hw breakpoints currently used.

> That should be done in update_watchpoint.  

Except that update_watchpoint only deals with watchpoints :)

> I would try to address incapabilities on PPC after those codes are
> committed.  Thank you for taking care of it!

Er, ... Can you send a patch? I was merely suggesting a different
approach that I hoped would work.

Thank you!
-- 
Joel

Re: Watchpoint on an unloaded shared library(1)

[Emi SUZUKI]

Hello Joel, 

From: Joel Brobecker <brobecker at adacore.com>
Subject: Re: Watchpoint on an unloaded shared library(1)
Date: Fri, 26 Dec 2008 10:10:55 +0400

>> This does work for me, except one duplication of the codes (that you
>> may have noticed already):
>> 
>>   static void
>>   do_enable_breakpoint (struct breakpoint *bpt, enum bpdisp disposition)
>>   {
>>     int target_resources_ok, other_type_used;
>>     struct value *mark;
>>   
>>     if (bpt->type == bp_hardware_breakpoint)
> 
> Not quite: As you can see, this part of the code deals with hardware
> *breakpoints*, not watchpoints. 

Shame on me... I've totally missed that :-(

>> I would try to address incapabilities on PPC after those codes are
>> committed.  Thank you for taking care of it!
> 
> Er, ... Can you send a patch? I was merely suggesting a different
> approach that I hoped would work.

Certainly; is it like below then?


2008-12-26  Joel Brobecker  <brobecker@adacore.com>
	    Emi Suzuki  <emi-suzuki@tjsys.co.jp>

	* breakpoint.c (do_enable_breakpoint): Use update_watchpoint for
	watchpoints.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.367
diff -u -r1.367 breakpoint.c
--- breakpoint.c	22 Dec 2008 04:37:37 -0000	1.367
+++ breakpoint.c	26 Dec 2008 06:55:49 -0000
@@ -7765,56 +7765,17 @@
       bpt->type == bp_read_watchpoint || 
       bpt->type == bp_access_watchpoint)
     {
-      struct frame_id saved_frame_id;
-      
-      saved_frame_id = get_frame_id (get_selected_frame (NULL));
-      if (bpt->exp_valid_block != NULL)
+      struct gdb_exception e;
+      TRY_CATCH (e, RETURN_MASK_ALL)
 	{
-	  struct frame_info *fr =
-	    fr = frame_find_by_id (bpt->watchpoint_frame);
-	  if (fr == NULL)
-	    {
-	      printf_filtered (_("\
-Cannot enable watchpoint %d because the block in which its expression\n\
-is valid is not currently in scope.\n"), bpt->number);
-	      return;
-	    }
-	  select_frame (fr);
+	  update_watchpoint (bpt, 1 /* reparse */);
 	}
-      
-      if (bpt->val)
-	value_free (bpt->val);
-      mark = value_mark ();
-      fetch_watchpoint_value (bpt->exp, &bpt->val, NULL, NULL);
-      if (bpt->val)
-	release_value (bpt->val);
-      bpt->val_valid = 1;
-
-      if (bpt->type == bp_hardware_watchpoint ||
-	  bpt->type == bp_read_watchpoint ||
-	  bpt->type == bp_access_watchpoint)
+      if (e.reason < 0)
 	{
-	  int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
-	  int mem_cnt = can_use_hardware_watchpoint (bpt->val);
-	  
-	  /* Hack around 'unused var' error for some targets here */
-	  (void) mem_cnt, (void) i;
-	  target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
-								    bpt->type, i + mem_cnt, other_type_used);
-	  /* we can consider of type is bp_hardware_watchpoint, convert to 
-	     bp_watchpoint in the following condition */
-	  if (target_resources_ok < 0)
-	    {
-	      printf_filtered (_("\
-Cannot enable watchpoint %d because target watch resources\n\
-have been allocated for other watchpoints.\n"), bpt->number);
-	      value_free_to_mark (mark);
-	      return;
-	    }
+	  exception_fprintf (gdb_stderr, e, _("Cannot enable watchpoint %d: "),
+			     bpt->number);
+	  return;
 	}
-      
-      select_frame (frame_find_by_id (saved_frame_id));
-      value_free_to_mark (mark);
     }
 
   if (bpt->enable_state != bp_permanent)


Anyway, thank you for your help!

-- 
Emi SUZUKI / emi-suzuki at tjsys.co.jp

Re: Watchpoint on an unloaded shared library(1)

[Joel Brobecker]

> > Not quite: As you can see, this part of the code deals with hardware
> > *breakpoints*, not watchpoints. 
> 
> Shame on me... I've totally missed that :-(

Believe me, I've done way worse and I am pretty sure it will happen
to me again (we're only human after all).

> 2008-12-26  Joel Brobecker  <brobecker@adacore.com>
> 	    Emi Suzuki  <emi-suzuki@tjsys.co.jp>
> 
> 	* breakpoint.c (do_enable_breakpoint): Use update_watchpoint for
> 	watchpoints.

Looks good (just a tiny tiny formatting adjustment - see below).
However, you did not say whether you tested the change (meaning that
you verified that it does not introduce any new failure in the GDB
testsuite), and if you did, on which system. I just want to make
sure the change was properly tested before it is checked in.

> +      struct gdb_exception e;
> +      TRY_CATCH (e, RETURN_MASK_ALL)

Can you add an empty line between these two line. The style in GDB
is to separate the local variable declarations from the rest of
the code by an empty line.

Thanks,
-- 
Joel

Re: Watchpoint on an unloaded shared library(1)

[Emi SUZUKI]

Hello Joel, 

From: Joel Brobecker <brobecker at adacore.com>
Subject: Re: Watchpoint on an unloaded shared library(1)
Date: Sun, 28 Dec 2008 15:17:19 +0400

> However, you did not say whether you tested the change (meaning that
> you verified that it does not introduce any new failure in the GDB
> testsuite), and if you did, on which system. I just want to make
> sure the change was properly tested before it is checked in.

OK, I would mention it from now on.  

>> +      struct gdb_exception e;
>> +      TRY_CATCH (e, RETURN_MASK_ALL)
> 
> Can you add an empty line between these two line. The style in GDB
> is to separate the local variable declarations from the rest of
> the code by an empty line.

The below is the reviced one.  I've tested it on x86-linux (F9) and
seen no regression.  

Thank you for all of your valuable advices for this issue!


2008-12-26  Joel Brobecker  <brobecker@adacore.com>
	    Emi Suzuki  <emi-suzuki@tjsys.co.jp>
 
	* breakpoint.c (do_enable_breakpoint): Use update_watchpoint for
	watchpoints.


--- breakpoint.c.orig	2009-01-03 14:57:50.000000000 +0900
+++ breakpoint.c	2009-01-05 14:34:01.000000000 +0900
@@ -7789,56 +7789,18 @@ do_enable_breakpoint (struct breakpoint 
       bpt->type == bp_read_watchpoint || 
       bpt->type == bp_access_watchpoint)
     {
-      struct frame_id saved_frame_id;
-      
-      saved_frame_id = get_frame_id (get_selected_frame (NULL));
-      if (bpt->exp_valid_block != NULL)
+      struct gdb_exception e;
+
+      TRY_CATCH (e, RETURN_MASK_ALL)
 	{
-	  struct frame_info *fr =
-	    fr = frame_find_by_id (bpt->watchpoint_frame);
-	  if (fr == NULL)
-	    {
-	      printf_filtered (_("\
-Cannot enable watchpoint %d because the block in which its expression\n\
-is valid is not currently in scope.\n"), bpt->number);
-	      return;
-	    }
-	  select_frame (fr);
+	  update_watchpoint (bpt, 1 /* reparse */);
 	}
-      
-      if (bpt->val)
-	value_free (bpt->val);
-      mark = value_mark ();
-      fetch_watchpoint_value (bpt->exp, &bpt->val, NULL, NULL);
-      if (bpt->val)
-	release_value (bpt->val);
-      bpt->val_valid = 1;
-
-      if (bpt->type == bp_hardware_watchpoint ||
-	  bpt->type == bp_read_watchpoint ||
-	  bpt->type == bp_access_watchpoint)
+      if (e.reason < 0)
 	{
-	  int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
-	  int mem_cnt = can_use_hardware_watchpoint (bpt->val);
-	  
-	  /* Hack around 'unused var' error for some targets here */
-	  (void) mem_cnt, (void) i;
-	  target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
-								    bpt->type, i + mem_cnt, other_type_used);
-	  /* we can consider of type is bp_hardware_watchpoint, convert to 
-	     bp_watchpoint in the following condition */
-	  if (target_resources_ok < 0)
-	    {
-	      printf_filtered (_("\
-Cannot enable watchpoint %d because target watch resources\n\
-have been allocated for other watchpoints.\n"), bpt->number);
-	      value_free_to_mark (mark);
-	      return;
-	    }
+	  exception_fprintf (gdb_stderr, e, _("Cannot enable watchpoint %d: "),
+			     bpt->number);
+	  return;
 	}
-      
-      select_frame (frame_find_by_id (saved_frame_id));
-      value_free_to_mark (mark);
     }
 
   if (bpt->enable_state != bp_permanent)


My best regards, 
-- 
Emi SUZUKI / emi-suzuki at tjsys.co.jp

Re: Watchpoint on an unloaded shared library(1)

[Joel Brobecker]

> The below is the reviced one.  I've tested it on x86-linux (F9) and
> seen no regression.  
[...]
> 2008-12-26  Joel Brobecker  <brobecker@adacore.com>
> 	    Emi Suzuki  <emi-suzuki@tjsys.co.jp>
>  
> 	* breakpoint.c (do_enable_breakpoint): Use update_watchpoint for
> 	watchpoints.

Thanks. The change itself is OK to commit, now.

I did forget something important, however, which is copyright
assignment. Do you or your employer have a copyright assignment
on file with the FSF? I searched the FSF files, and I couldn't
locate you.

-- 
Joel

Re: Watchpoint on an unloaded shared library(1)

[Emi SUZUKI]

Hello Joel, 

From: Joel Brobecker <brobecker at adacore.com>
Subject: Re: Watchpoint on an unloaded shared library(1)
Date: Tue, 06 Jan 2009 08:28:19 +0400

> I did forget something important, however, which is copyright
> assignment. Do you or your employer have a copyright assignment
> on file with the FSF? I searched the FSF files, and I couldn't
> locate you.

Oh, I should have mentioned it...
I've done all the work with GDB for Toshiba Corporation, and got an
assignment as Toshiba.  


My best regards, 
-- 
Emi SUZUKI / emi-suzuki at tjsys.co.jp

[commit] Watchpoint on an unloaded shared library(1)

[Emi SUZUKI]

Hello Joel and all, 

From: Joel Brobecker <brobecker at adacore.com>
Subject: Re: Watchpoint on an unloaded shared library(1)
Date: Tue, 06 Jan 2009 08:28:19 +0400

>> 2008-12-26  Joel Brobecker  <brobecker@adacore.com>
>> 	    Emi Suzuki  <emi-suzuki@tjsys.co.jp>
>>  
>> 	* breakpoint.c (do_enable_breakpoint): Use update_watchpoint for
>> 	watchpoints.
> 
> Thanks. The change itself is OK to commit, now.

This is now checked in.  Thank you for all of your support!

My best regards, 
-- 
Emi SUZUKI / emi-suzuki at tjsys.co.jp