Re: (gdb-6.8) Discard breakpoint address if shared library is unloaded

[Vladimir Prus <ghost@cs.msu.su>]

On Friday 15 February 2008 07:01:00 Nick Roberts wrote:
>  > Can you please resend a new complete patch, with a ChangeLog, and I will
>  > review it. It's much easier that way for me to make sure that what I
>  > think you are proposing is indeed what you are proposing to commit.
> 
> -- 
> Nick                                           http://www.inet.net.nz/~nickrob
> 
> 
> 2008-02-15  Nick Roberts  <nickrob@snap.net.nz>
> 
>         * breakpoint.c (print_one_breakpoint_location): Revert Enb field
>         to old format.  Discard breakpoint address if shared library is
>         unloaded.
>         (breakpoint_1): Adjust formatting of table header accordingly.
> 
> 
> 
> *** breakpoint.c        15 Feb 2008 16:52:37 +1300      1.302
> --- breakpoint.c        15 Feb 2008 16:55:43 +1300      
> *************** print_one_breakpoint_location (struct br
> *** 3426,3448 ****
>     /* 4 */
>     annotate_field (3);
>     if (part_of_multiple)
> !     ui_out_field_string (uiout, "enabled", 
> !                        loc->shlib_disabled 
> !                        ? (loc->enabled ? "y(p)" : "n(p)")
> !                        : (loc->enabled ? "y" : "n"));
>     else
> !     {
> !       int pending = (b->loc == NULL || b->loc->shlib_disabled);
> !       /* For header of multiple, there's no point showing pending
> !        state -- it will be apparent from the locations.  */
> !       if (header_of_multiple)
> !       pending = 0;
> !       ui_out_field_fmt (uiout, "enabled", "%c%s", 
> !                       bpenables[(int) b->enable_state],
> !                       pending ? "(p)" : "");
> !       if (!pending)
> !       ui_out_spaces (uiout, 3);
> !     }
>   
>     
>     /* 5 and 6 */
> --- 3426,3436 ----
>     /* 4 */
>     annotate_field (3);
>     if (part_of_multiple)
> !     ui_out_field_string (uiout, "enabled", loc->enabled ? "y" : "n");
>     else
> !       ui_out_field_fmt (uiout, "enabled", "%c", 
> !                       bpenables[(int) b->enable_state]);
> !   ui_out_spaces (uiout, 2);
>   
>     
>     /* 5 and 6 */
> *************** print_one_breakpoint_location (struct br
> *** 3553,3562 ****
>         if (addressprint)
>           {
>             annotate_field (4);
> !           if (b->loc == NULL)
> !             ui_out_field_string (uiout, "addr", "<PENDING>");
> !           else if (header_of_multiple)
>               ui_out_field_string (uiout, "addr", "<MULTIPLE>");
>             else
>               ui_out_field_core_addr (uiout, "addr", loc->address);
>           }
> --- 3541,3550 ----
>         if (addressprint)
>           {
>             annotate_field (4);
> !           if (header_of_multiple)
>               ui_out_field_string (uiout, "addr", "<MULTIPLE>");
> +           if (b->loc == NULL || loc->shlib_disabled)
> +             ui_out_field_string (uiout, "addr", "<PENDING>");
>             else
>               ui_out_field_core_addr (uiout, "addr", loc->address);
>           }
> *************** breakpoint_1 (int bnum, int allflag)
> *** 3781,3787 ****
>     ui_out_table_header (uiout, 4, ui_left, "disp", "Disp");            /* 3 */
>     if (nr_printable_breakpoints > 0)
>       annotate_field (3);
> !   ui_out_table_header (uiout, 4, ui_left, "enabled", "Enb");  /* 4 */
>     if (addressprint)
>         {
>           if (nr_printable_breakpoints > 0)
> --- 3769,3775 ----
>     ui_out_table_header (uiout, 4, ui_left, "disp", "Disp");            /* 3 */
>     if (nr_printable_breakpoints > 0)
>       annotate_field (3);
> !   ui_out_table_header (uiout, 3, ui_left, "enabled", "Enb");  /* 4 */
>     if (addressprint)
>         {
>           if (nr_printable_breakpoints > 0)

Did you run the testsuite with this patch? I'm getting the following test failures
if the patch is applied:


FAIL: gdb.cp/ovldbreak.exp: breakpoint info (after setting one-by-one)
FAIL: gdb.cp/ovldbreak.exp: breakpoint info (after cancel)
FAIL: gdb.cp/ovldbreak.exp: breakpoint info (after setting on all)

I did not run the entire testsuite, only gdb.cp directory, so there maybe
be more.

While looking on the patch I've noticed a pre-existing problem -- say
we create a pending breakpoint of a function called foo. Then 'info break'
will say the breakpoint is on 'foo'. After the breakpoint is resolved,
info break will mention both the function name and the source line.
If the library is unloaded, then info break will only mention source line ---
even though breakpoint's address_string actually refers to function.

I don't think we should bother to fix this right now, as the code in
question seems rather old.

Other than that, I did not noticed any issues, so I presume the patch can
go in as soon as the test failures are handled.

- Volodya