Re: Watchpoint on an unloaded shared library(1)

[Joel Brobecker <brobecker at adacore dot com>]

> > 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