Watchpoint on an unloaded shared library(2)

Watchpoint on an unloaded shared library(2)

[Emi SUZUKI]

Hello members, 

Sorry for my laziness about reporting the rest of the issues I
suggested at the mail:

  http://sourceware.org/ml/gdb-patches/2008-11/msg00538.html

Now I'd like to resume.  

For this issue, I will post this to gdb@, not gdb-patches@, cause I
don't have any exact solutions for the issue below.  And note that I
use the same program code shown in the above mail to reproduce it.  

---- 
Issue 2:

$ ./gdb dl-test
GNU gdb (GDB) 6.8.50.20081216-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) break main
Breakpoint 1 at 0x80484e5: file dl-test.c, line 14.
(gdb) run
Starting program: /home/suzuki/test/dl-test

Breakpoint 1, main () at dl-test.c:14
14        if ((handle = dlopen("./libsample.so", RTLD_LAZY)) == NULL)
(gdb) next
17        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) continue
Continuing.

Program exited normally.
(gdb) info breakpoints
Num     Type           Disp Enb Address    What
1       breakpoint     keep y   0x080484e5 in main at dl-test.c:14
        breakpoint already hit 1 time
2       hw watchpoint  keep y              sample_glob
        breakpoint already hit 1 time
(gdb) run
Starting program: /home/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.

Breakpoint 1, main () at dl-test.c:14
14        if ((handle = dlopen("./libsample.so", RTLD_LAZY)) == NULL)
(gdb) info breakpoints
Num     Type           Disp Enb Address    What
1       breakpoint     keep y   0x080484e5 in main at dl-test.c:14
        breakpoint already hit 1 time
sample of shared library
Segmentation fault

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

The cause of a crash is that print_one_breakpoint_location in breakpoint.c
doesn't care about whether the expression for the watchpoint is valid:

      case bp_watchpoint:
      case bp_hardware_watchpoint:
      case bp_read_watchpoint:
      case bp_access_watchpoint:
        /* Field 4, the address, is omitted (which makes the columns
           not line up too nicely with the headers, but the effect
           is relatively readable).  */
        if (opts.addressprint)
          ui_out_field_skip (uiout, "addr");
        annotate_field (5);
        print_expression (b->exp, stb->stream);
        ui_out_field_stream (uiout, "what", stb);
        break;

Here, b->exp for the watchpoints set on an unloaded shared library can
be NULL, because breakpoint_re_set_one has done it.  However, what
should we do instead?

I have considered two solutions:

 a) Print b->exp_string and b->cond_string. 
    We might make some effort to display it like as its expression is
    valid for annotations... I have no idea whether it is worthwhile
    to try.  

 b) Don't set b->exp to NULL in update_watchpoint (called by
    breakpoint_re_set_one), do_enable_breakpoint and so on.  
    Maybe we should add some flags to `struct expression' to avoid
    passing invalid symtabs to some interacting functions.  

"Skip printing" is another possibility, but I'd ignore it: skipping
means that the user can't refer to the information about what they
were.  

Anyway, I don't like either of those ideas above much.  So I'd like to
ask you which you think it better, or any other ideas for solving the
issue.  Any comments are appreciated.  


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

Re: Watchpoint on an unloaded shared library(2)

[Joel Brobecker]

> The cause of a crash is that print_one_breakpoint_location in breakpoint.c
> doesn't care about whether the expression for the watchpoint is valid:
> 
>       case bp_watchpoint:
>       case bp_hardware_watchpoint:
>       case bp_read_watchpoint:
>       case bp_access_watchpoint:
>         /* Field 4, the address, is omitted (which makes the columns
>            not line up too nicely with the headers, but the effect
>            is relatively readable).  */
>         if (opts.addressprint)
>           ui_out_field_skip (uiout, "addr");
>         annotate_field (5);
>         print_expression (b->exp, stb->stream);
>         ui_out_field_stream (uiout, "what", stb);
>         break;
> 
> Here, b->exp for the watchpoints set on an unloaded shared library can
> be NULL, because breakpoint_re_set_one has done it.  However, what
> should we do instead?
> 
> I have considered two solutions:
> 
>  a) Print b->exp_string and b->cond_string. 
>     We might make some effort to display it like as its expression is
>     valid for annotations... I have no idea whether it is worthwhile
>     to try.  

I think that this is the best we can do (print exp_string).
I personally wouldn't worry about trying to massage the string
into something that would look like we're printing an expression.
I'm not even sure why we use "print_expression (b->exp) rather
than printing exp_string directly - perhaps someone does?

>  b) Don't set b->exp to NULL in update_watchpoint (called by
>     breakpoint_re_set_one), do_enable_breakpoint and so on.  
>     Maybe we should add some flags to `struct expression' to avoid
>     passing invalid symtabs to some interacting functions.  

I don't think we can do that. Otherwise, the expression could
reference some symbols that no longer exist.

> "Skip printing" is another possibility, but I'd ignore it: skipping
> means that the user can't refer to the information about what they
> were.  

Right, as long as the watchpoint is defined, albeit disabled,
we need to show it in the list. Another alternative of your suggestion
is to not print the "what".  But I would find this confusing, since
you would no longer know what the watchpoint entry is watching.

-- 
Joel

Re: Watchpoint on an unloaded shared library(2)

[Daniel Jacobowitz]

On Mon, Dec 29, 2008 at 09:13:17AM +0400, Joel Brobecker wrote:
> >  a) Print b->exp_string and b->cond_string. 
> >     We might make some effort to display it like as its expression is
> >     valid for annotations... I have no idea whether it is worthwhile
> >     to try.  
> 
> I think that this is the best we can do (print exp_string).
> I personally wouldn't worry about trying to massage the string
> into something that would look like we're printing an expression.
> I'm not even sure why we use "print_expression (b->exp) rather
> than printing exp_string directly - perhaps someone does?

I don't know, but it drives me nuts.  This causes:

(gdb) break *0x10000000
Breakpoint 1 at 0x10000000
(gdb) watch *0x10000000
Watchpoint 2: *268435456
(gdb) info break
Num     Type           Disp Enb Address    What
1       breakpoint     keep y   0x10000000
2       watchpoint     keep y              *268435456

I'm sure there are some cases which are more ambiguous if you print
the string, but I'd risk some ambiguity to get rid of that :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: Watchpoint on an unloaded shared library(2)

[Pedro Alves]

On Monday 29 December 2008 13:53:32, Daniel Jacobowitz wrote:
> On Mon, Dec 29, 2008 at 09:13:17AM +0400, Joel Brobecker wrote:
> > >  a) Print b->exp_string and b->cond_string. 
> > >     We might make some effort to display it like as its expression is
> > >     valid for annotations... I have no idea whether it is worthwhile
> > >     to try.  
> > 
> > I think that this is the best we can do (print exp_string).
> > I personally wouldn't worry about trying to massage the string
> > into something that would look like we're printing an expression.
> > I'm not even sure why we use "print_expression (b->exp) rather
> > than printing exp_string directly - perhaps someone does?
> 
> I don't know, but it drives me nuts.  This causes:
> 
> (gdb) break *0x10000000
> Breakpoint 1 at 0x10000000
> (gdb) watch *0x10000000
> Watchpoint 2: *268435456
> (gdb) info break
> Num     Type           Disp Enb Address    What
> 1       breakpoint     keep y   0x10000000
> 2       watchpoint     keep y              *268435456
> 
> I'm sure there are some cases which are more ambiguous if you print
> the string, but I'd risk some ambiguity to get rid of that :-)
> 

This looks like PR 8079:
http://sourceware.org/bugzilla/show_bug.cgi?id=8079

-- 
Pedro Alves

Re: Watchpoint on an unloaded shared library(2)

[Tom Tromey]

Joel> I'm not even sure why we use "print_expression (b->exp) rather
Joel> than printing exp_string directly - perhaps someone does?

Daniel> I don't know, but it drives me nuts.

Me too!  I usually recognize the expression I typed, not necessarily
the canonicalization that gdb comes up with.

Pedro> This looks like PR 8079:
Pedro> http://sourceware.org/bugzilla/show_bug.cgi?id=8079

I propose that if nobody comes up with a reason for the current
behavior, we commit that patch.  I can run it through the regression
tester and fix up the problems next week, if you like.  Let me know.

Tom

Re: Watchpoint on an unloaded shared library(2)

[Joel Brobecker]

> I propose that if nobody comes up with a reason for the current
> behavior, we commit that patch.  I can run it through the regression
> tester and fix up the problems next week, if you like.  Let me know.

That works for me :)

-- 
Joel

Re: Watchpoint on an unloaded shared library(2)

[Emi SUZUKI]

Hello Tom, 

From: Tom Tromey <tromey at redhat.com>
Subject: Re: Watchpoint on an unloaded shared library(2)
Date: Mon, 29 Dec 2008 17:00:06 -0700

> Pedro> This looks like PR 8079:
> Pedro> http://sourceware.org/bugzilla/show_bug.cgi?id=8079
> 
> I propose that if nobody comes up with a reason for the current
> behavior, we commit that patch.  I can run it through the regression
> tester and fix up the problems next week, if you like.  Let me know.

I thought using print_expression should have some reasons.  But if it
seems not, the fix would be good for me.  Thank you!

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