Nick Roberts | 1 Jun 2008 01:40
Picon

Re: [patch/MI] Implementation for break-catch command

 > >  static void enable_delete_breakpoint (struct breakpoint *);
 > >  <at>  <at>  -6549,13 +6547,16  <at>  <at>  print_mention_exception_catchpoint (stru
 > >    int bp_temp;
 > >    int bp_throw;
 > >  
 > > -  bp_temp = b->loc->owner->disposition == disp_del;
 > > -  bp_throw = strstr (b->addr_string, "throw") != NULL;
 > > -  ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
 > > -                             : _("Catchpoint "));
 > > -  ui_out_field_int (uiout, "bkptno", b->number);
 > > -  ui_out_text (uiout, bp_throw ? _(" (throw)")
 > > -                              : _(" (catch)"));
 > > +  if (!ui_out_is_mi_like_p (uiout))
 > > +    {
 > > +      bp_temp = b->loc->owner->disposition == disp_del;
 > > +      bp_throw = strstr (b->addr_string, "throw") != NULL;
 > > +      ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
 > > +                                 : _("Catchpoint "));
 > > +      ui_out_field_int (uiout, "bkptno", b->number);
 > > +      ui_out_text (uiout, bp_throw ? _(" (throw)")
 > > +                                  : _(" (catch)"));
 > > +    }
 > 
 > Uh, seems like there's some messiness in MI here. For ordinary breakpoints,
 > MI installs the 'breakpoint_notify' hook that prints all the information,
 > and 'mention' specifically avoids printing anything for MI. For watchpoints,
 > however, no hooks are installed, and 'mention' does print something --
 > basically just id and expression.
 > 
 > For -break-catch, you choose to install hooks, and disable printing anything
(Continue reading)

Nick Roberts | 1 Jun 2008 04:46
Picon

[PATCH:MI] Use observers for breakpoints

Following Aleksandar Ristovski's patch for catchpoints in GDB/MI, here's a
patch to demonstrate the use of observers for breakpoints.  This relates to
earlier patches I've submitted which uses event notification to communicate a
change in state rather than using command output directly.  Among other things
this allows the use of CLI commands with MI.

It's not a patch for immediate inclusion as it changes existing behaviour.
However I think it's something we should be aiming towards and would be
suitable for inclusion next time the MI level gets bumped.

Here's some sample output:

b main
&"b main\n"
=breakpoints-changed,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x080486d6",func="main",file="myprog.c",fullname="/home/nickrob/myprog.c",line="140",times="0",original-location="main"}
~"Breakpoint 1 at 0x80486d6: file myprog.c, line 140.\n"
^done
(gdb) 
dis 1
&"dis 1\n"
=breakpoints-changed,bkpt={number="1",type="breakpoint",disp="keep",enabled="n",addr="0x080486d6",func="main",file="myprog.c",fullname="/home/nickrob/myprog.c",line="140",times="0",original-location="main"}
^done
(gdb) 
cond 1 1==1
&"cond 1 1==1\n"
=breakpoints-changed,bkpt={number="1",type="breakpoint",disp="keep",enabled="n",addr="0x080486d6",func="main",file="myprog.c",fullname="/home/nickrob/myprog.c",line="140",cond="1==1",times="0",original-location="main"}
^done
(gdb) 
-break-insert main
=breakpoints-changed,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0x080486d6",func="main",file="myprog.c",fullname="/home/nickrob/myprog.c",line="140",times="0",original-location="main"}
(Continue reading)

Eli Zaretskii | 1 Jun 2008 05:20
Picon

Re: [PATCH:MI] Use observers for breakpoints

> From: Nick Roberts <nickrob <at> snap.net.nz>
> Date: Sun, 1 Jun 2008 14:46:04 +1200
> Cc: ghost <at> cs.msu.su
> 
> + <at> deftypefun void breakpoints_changed (int  <at> var{bnum})
> +The thread specified by  <at> var{t} has exited.

There's no argument  <at> var{t} in the function's signature.

Nick Roberts | 1 Jun 2008 05:30
Picon

Re: [PATCH:MI] Use observers for breakpoints

 > > + <at> deftypefun void breakpoints_changed (int  <at> var{bnum})
 > > +The thread specified by  <at> var{t} has exited.
 > 
 > There's no argument  <at> var{t} in the function's signature.

OK, thanks.  Too much cut and paste.  That should be:

+ <at> deftypefun void breakpoints_changed (int  <at> var{bnum})
+The breakpoint specified by  <at> var{bnum} has changed or is a new breakpoint.

--

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Vladimir Prus | 1 Jun 2008 07:14

Re: [patch/MI] Implementation for break-catch command

On Sunday 01 June 2008 03:40:06 Nick Roberts wrote:
>  > >  static void enable_delete_breakpoint (struct breakpoint *);
>  > >  <at>  <at>  -6549,13 +6547,16  <at>  <at>  print_mention_exception_catchpoint (stru
>  > >    int bp_temp;
>  > >    int bp_throw;
>  > >  
>  > > -  bp_temp = b->loc->owner->disposition == disp_del;
>  > > -  bp_throw = strstr (b->addr_string, "throw") != NULL;
>  > > -  ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
>  > > -                             : _("Catchpoint "));
>  > > -  ui_out_field_int (uiout, "bkptno", b->number);
>  > > -  ui_out_text (uiout, bp_throw ? _(" (throw)")
>  > > -                              : _(" (catch)"));
>  > > +  if (!ui_out_is_mi_like_p (uiout))
>  > > +    {
>  > > +      bp_temp = b->loc->owner->disposition == disp_del;
>  > > +      bp_throw = strstr (b->addr_string, "throw") != NULL;
>  > > +      ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
>  > > +                                 : _("Catchpoint "));
>  > > +      ui_out_field_int (uiout, "bkptno", b->number);
>  > > +      ui_out_text (uiout, bp_throw ? _(" (throw)")
>  > > +                                  : _(" (catch)"));
>  > > +    }
>  > 
>  > Uh, seems like there's some messiness in MI here. For ordinary breakpoints,
>  > MI installs the 'breakpoint_notify' hook that prints all the information,
>  > and 'mention' specifically avoids printing anything for MI. For watchpoints,
>  > however, no hooks are installed, and 'mention' does print something --
>  > basically just id and expression.
>  > 
(Continue reading)

Joel Brobecker | 2 Jun 2008 05:27
Favicon

[commit] print minimal symbol's address correctly...

As it turns out, AIX was using the same technique as ia64 to identify
function descriptors (namely: If the address doesn't point to an
executable section, then it must be a function descriptor). See:

   http://www.sourceware.org/ml/gdb-patches/2008-05/msg00740.html

for a description of why this heuristic needs to be improved a bit.

For now, I applied the same change as on ia64. The comment above
the function doing the conversion mentions OPD sections, but it
doesn't apply at all to the actual implementation, and I couldn't
find confirmation of such sections in the few AIX documents that
I have. Perhaps Ulrich could confirm or infirm whether the comment
is accurate? Depending on the outcome, we might want to further
enhance the function.

2008-06-01  Joel Brobecker  <brobecker <at> adacore.com>

        * rs6000-aix-tdep.c (rs6000_convert_from_func_ptr_addr): Do not
        treat pointers in data space as function descriptors if the
        target address is also in the data space.

Tested on ppc-aix. One test actually transitions from PASS to FAIL,
but I think the PASS was an accident: It's ptype.exp:ptype int.
For some reason, when doing "ptype v_int" (v_int is a global variable
defined as an int), GDB doesn't seem to find any debugging info for
this variable (it should because the file was compiled with -g!).
Because of that, and because rs6000_convert_from_func_ptr_addr used
to treat the address as a function descriptor, GDB would end up
thinking that this is a function pointer, and thus return "type =
(Continue reading)

Aleksandar Ristovski | 2 Jun 2008 15:22

Re: [patch/MI] Implementation for break-catch command

Vladimir Prus wrote:
> On Sunday 01 June 2008 03:40:06 Nick Roberts wrote:
>>
>> I think all these "breakpoints" should be changed to use observers.  This
>> seems to just add to the ugliness.
> 
> They definitely should.

Well then, what is the preferred way? I can tweak my patch to take your comments into consideration, but if a
switch to observers is going to happen soon, then I guess the break-catch should get this new stuff right
away? Implementing observer_notify for all the breakpoints would break the compatibility and require
quite a bit of work on the testsuite side.

Either way, Vladimir, it's your call - let me know.

Vladimir Prus | 2 Jun 2008 16:39

Re: [patch/MI] Implementation for break-catch command

On Monday 02 June 2008 17:22:45 you wrote:
> Vladimir Prus wrote:
> > On Sunday 01 June 2008 03:40:06 Nick Roberts wrote:
> >>
> >> I think all these "breakpoints" should be changed to use observers.  This
> >> seems to just add to the ugliness.
> > 
> > They definitely should.
> 
> Well then, what is the preferred way? I can tweak my patch to 
> take your comments into consideration, but if a switch to observers is 
> going to happen soon, then I guess the break-catch should get 
> this new stuff right away? Implementing observer_notify for all the breakpoints
> would break the compatibility and require quite a bit of work on the testsuite side.
> 
> Either way, Vladimir, it's your call - let me know.

I don't think you should worry about observers -- your patch makes a nice self-contained
improvement, and it does not make sense to mix it with any other cleanups. For now, using
hooks is fine, and my comment about observers does not call for any changes in this
patch.
I don't think any of my comments will be invalidated when we switch to observers.

Thanks,
Volodya

Aleksandar Ristovski | 2 Jun 2008 18:06

Re: [patch/MI] Implementation for break-catch command

Vladimir Prus wrote:
> On Thursday 29 May 2008 18:48:36 Aleksandar Ristovski wrote:
>> Index: gdb/breakpoint.c
> 
> First of all, I would like a some testcases to be added with this patch.
> 
> I have some questions about behaviour, too:
> 
> 1. -break-catch throw
>    ^done,bkpt={number="3",type="breakpoint",disp="keep",enabled="y",
>                addr="0xb7ef6e05",what="exception throw",times="0",
>                original-location="__cxa_throw"}
>    (gdb)
> 
> The way this breakpoint is printed seems not sufficiently precise. Suppose
> such a breakpoint was added via CLI. GUI would like to detect this, and
> add a breakpoint to the breakpoint list as "catch" breakpoint. How can we
> detect it is a catch breakpoint? The "what" field appears to be of a fairly
> ad-hoc use in MI. 

A few questions:

This is the output from -break-list:

^done,BreakpointTable={nr_rows="3",nr_cols="6",hdr=[
{width="7",alignment="-1",col_name="number",colhdr="Num"},
{width="14",alignment="-1",col_name="type",colhdr="Type"},
{width="4",alignment="-1",col_name="disp",colhdr="Disp"},
{width="3",alignment="-1",col_name="enabled",colhdr="Enb"},
{width="10",alignment="-1",col_name="addr",colhdr="Address"},
(Continue reading)

Joel Brobecker | 2 Jun 2008 18:08
Favicon

Re: [PATCH/m68k] Fix length of lea insn.

Roman,

> > > 	* m68k-tdep.c (m68k_analyze_prologue): Fix length of lea insn.
> > 
> > OK, thanks.  Do you have write access and/or a copyright assignment?
> 
> Only the latter.

I checked both patches in for you. If you think you might be submitting
additional patches and would like to have write access to the repository,
let us know.

--

-- 
Joel


Gmane