Re: [RFC stub-side break conditions 1/5] Documentation bits

[Eli Zaretskii <eliz at gnu dot org>]

> Date: Fri, 06 Jan 2012 11:48:24 -0200
> From: Luis Machado <luis_gustavo@mentor.com>
> CC: gdb-patches@sourceware.org
> 
> >    ... the @dfn{condition evaluation mode}, described below, will be
> >    shown after the condition, in between parentheses
> >
> > For bonus points, have most of the evaluation mode, minus the
> > commands, described before this paragraph, in which case there's no
> > need for the "described below" part.
> 
> Fixed. How does it look now?

Looks good, thanks.  A couple of minor comments below.

> > As an aside not directly related to the documentation: why is it a
> > good idea to show the evaluation mode only when the evaluation is not
> > by GDB? why not show it always?  At the very least it would avoid
> > confusion when the user is debugging with a server that supports
> > server-side evaluation of conditions, and turns the feature on and
> > off: it's IMO confusing not to have an indication of the mode in half
> > the cases.
> I took this path for the sake of keeping the old behavior/output 
> unchanged.  But you are right, it may cause confusion, so i suppose we 
> should always display the condition evaluation mode.
> 
> Or should we just avoid displaying anything and let GDB handle 
> everything automatically, hiding all the complexity and switches from 
> the user?

That's also a possibility.  It may even be a better one, as this is an
obscure implementation detail.

> >> +@var{cond expr} is an optional conditional expression in bytecode form
> > It's a bad mojo to have in @var something that is not a single word.
> > Suggest to use @var{cond-expr}.
> Fixed.
> >> +                                                This is the list of
> >> +conditions that should be taken into consideration when deciding if
> >> +the breakpoint trigger should be reported back to @var{GDBN}.
> > A "list" of conditions?  Didn't you just say that it's a single
> > expression?
> It is a list of expressions.  If we have only a single conditional 
> location, we need to send a single expression.

I see you changed "expression" to "list of expressions"; now I'm
happy.

> --- gdb.orig/gdb/NEWS	2012-01-04 14:02:33.578431998 -0200
> +++ gdb/gdb/NEWS	2012-01-06 10:51:15.990431999 -0200
> @@ -9,6 +9,28 @@
>  * The binary "gdbtui" can no longer be built or installed.
>    Use "gdb -tui" instead.
>  
> +* GDBServer supports evaluation of breakpoint conditions.  When
> +  support is advertised by GDBServer, GDB may be told to send the
> +  breakpoint conditions in bytecode form to GDBServer.  GDBServer
> +  will only report the breakpoint trigger to GDB when its condition
> +  evaluates to true.
> +
> +* New options
> +
> +set breakpoint condition-evaluation
> +show breakpoint condition-evaluation
> +  Controls whether breakpoint conditions are evaluated by GDB ("gdb") or by
> +  GDBServer ("target").
> +  This option can improve debugger efficiency depending on the speed of the
> +  target.
> +
> +* New remote packets
> +
> +  The z0/z1 breakpoint insertion packets have been extended to carry
> +  a list of conditional expressions over to GDBServer depending on the
> +  condition evaluation mode.  The use of this extension can be controlled
> +  via the "set remote conditional-breakpoints-packet" command.
> +

This is okay.

> +If a breakpoint is conditional, there are two evaluation modes: "gdb" and
> +"target".  If mode is "gdb", breakpoint condition evaluation is done by

Please use ``..'' style of quoting, not "..".  In the Info output,
makeinfo will produce ".." regardless, but in the printed version the
former style produces prettier results.

> +@item set breakpoint condition-evaluation target
> +This option commands @value{GDBN} to download breakpoint conditions
> +to the target at the moment of their insertion.  The target
> +is responsible for evaluating the conditional expression and reporting
> +breakpoint stop events back to @value{GDBN} whenever the condition
> +is true.  Limitations apply, and conditions that are not
> +recognized as valid or depend on local data that lives in the host will be
> +evaluated by @value{GDBN}. Examples are conditional expressions involving
> +convenience variables, complex types that cannot be handled by the agent
> +expression parser and expressions that are too long to be sent over to
> +the target, specially when the target is a remote system.

This is clear, but I suggest a slight rewording:

  @item set breakpoint condition-evaluation target
  This option commands @value{GDBN} to download breakpoint conditions to
  the target at the moment of their insertion.  The target is
  responsible for evaluating the conditional expression and reporting
  breakpoint stop events back to @value{GDBN} whenever the condition is
  true.  Due to limitations of target-side evaluation, some conditions
  cannot be evaluated there, e.g., conditions that are not valid or
  depend on local data that is only known to the host.  Examples include
  conditional expressions involving convenience variables, complex types
  that cannot be handled by the agent expression parser and expressions
  that are too long to be sent over to the target, specially when the
  target is a remote system.  In these cases, the conditions will be
  evaluated by @value{GDBN}.

One question: what does "conditions that are not valid" refer to?  Are
these the cases where some of the sub-expressions can only be
evaluated when the breakpoint actually breaks, because they are valid
only in the breakpoint's scope?  Or are there other situations where
the expression can be invalid?  I think we should give the reader some
hint about when this can happen.

OK with these changes.

Thanks.