Watchpoint on an unloaded shared library(4)

Hello members,

It's the last one I've found while I am investigating problems about
watchpoint on an unloaded shared library.  

I modified the test code shown the mail below a bit, to watch a global
variable in the main program.  
  http://sourceware.org/ml/gdb-patches/2008-11/msg00538.html

And the issue 4 is: given the watchpoint refers to a variable on a
shared library in its conditional phrase, GDB doesn't resume the
inferior after the library was unloaded until the user deletes that
watchpoint.  I thought disabling the watchpoint should go, however, it
didn't either.

----------------
$ ./x86-gdb dl-test
GNU gdb (GDB) 6.8.50.20090114-cvs
Copyright (C) 2009 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) list
3       #include <stdio.h>
4       #include <stdlib.h>
5
6       static void (*sample) (void);
7       int glob = 0;
8
9       int
10      main (void)
11      {
12        void *handle = NULL;
13
14        if ((handle = dlopen("./libsample.so", RTLD_LAZY)) == NULL)
15          errx(2, "dlopen(): %s", dlerror());
16
17        if ((sample = dlsym(handle, "sample")) == NULL)
18          errx(2, "dlsym(): %s", dlerror());
19
20        sample ();
21        glob++;
22
23        if (dlclose(handle) < 0)
24          errx(2, "dlclose(): %s", dlerror());
25
26        return 0;
27      }
28
(gdb) break 17
Breakpoint 1 at 0x8048536: file dl-test.c, line 17.
(gdb) continue
The program is not being run.
(gdb) run
Starting program: /home/suzuki/test/dl-test

Breakpoint 1, main () at dl-test.c:17
17        if ((sample = dlsym(handle, "sample")) == NULL)
(gdb) watch glob if sample_glob > 1
Hardware watchpoint 2: glob
(gdb) continue
Continuing.
sample of shared library
Hardware watchpoint 2: glob

Old value = 0
New value = 1
main () at dl-test.c:23
23        if (dlclose(handle) < 0)
(gdb) continue
Continuing.
0x00bd91a1 in _dl_debug_state () from /lib/ld-linux.so.2
(gdb) continue
Continuing.
No symbol "sample_glob" in current context.
(gdb) info breakpoints
Num     Type           Disp Enb Address    What
1       breakpoint     keep y   0x08048536 in main at dl-test.c:17
        breakpoint already hit 1 time
2       hw watchpoint  keep y              glob
        stop only if sample_glob > 1
        breakpoint already hit 1 time
(gdb) disable 2
(gdb) continue
Continuing.
No symbol "sample_glob" in current context.
(gdb)
---------------

I've caught where GDB is prevented to resume:

  /* Make sure all breakpoints are inserted in inferior.
     Throws exception on any error.
     A breakpoint that is already inserted won't be inserted
     again, so calling this function twice is safe.  */
  void
  insert_breakpoints (void)
  {
    struct breakpoint *bpt;

    ALL_BREAKPOINTS (bpt)
      if (is_hardware_watchpoint (bpt))
        update_watchpoint (bpt, 0 /* don't reparse. */);

The conditional phrases always be reparsed in update_watchpoint no
matter how 'reparse' parameter is, so it would always assert after
when the symtabs for the library is vanished.  

I understand that update_watchpoint is called here for deleting
invalid symtabs.  If it is, we can ignore disabled watchpoints here,
because the expression would be reparsed when re-enabling.  (Note 
that the fix for 'info breakpoints' accessing to invalid expression
structure is being asked for revewing at
http://sourceware.org/ml/gdb-patches/2009-01/msg00111.html.)

I made a patch and tested it on x86-linux.  Is the below OK?

2009-01-16  Emi Suzuki  <emi-suzuki <at> tjsys.co.jp>

	* breakpoint.c (insert_breakpoints): Reparse the watchpoint
        expression only when it is enabled.  

diff -upBw src.orig/gdb/breakpoint.c src/gdb/breakpoint.c
--- src.orig/gdb/breakpoint.c	2009-01-09 01:32:29.000000000 +0900
+++ src/gdb/breakpoint.c	2009-01-14 14:15:13.000000000 +0900
 <at>  <at>  -1247,7 +1247,7  <at>  <at>  insert_breakpoints (void)
   struct breakpoint *bpt;

   ALL_BREAKPOINTS (bpt)
-    if (is_hardware_watchpoint (bpt))
+    if (is_hardware_watchpoint (bpt) && bpt->enable_state != bp_disabled)
       update_watchpoint (bpt, 0 /* don't reparse. */);

   update_global_location_list (1);

--

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