Re: Watchpoint on an unloaded shared library(1)

[Emi SUZUKI <emi-suzuki@tjsys.co.jp>]

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