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

Eli Zaretskii eliz@gnu.org
Fri Jan 6 08:30:00 GMT 2012


> Date: Thu, 05 Jan 2012 12:56:03 -0200
> From: Luis Machado <luis_gustavo@mentor.com>
> 
> This patch handles the documentation changes.

Thanks.

> +* GDBServer supports evaluating breakpoint conditions.

Suggest to rephrase

  GDBServer supports evaluation of breakpoint conditions.

>                                                            When
> +  support is advertised by GDBServer, GDB will send the breakpoint
                                             ^^^^^^^^^
Probably "may be told to send", since what you describe may or may not
be the default, and in any case, other modes are available.

The NEWS entry is okay with these changes.

>  @noindent
>  If a breakpoint is conditional, @code{info break} shows the condition on
> -the line following the affected breakpoint; breakpoint commands, if any,
> -are listed after that.  A pending breakpoint is allowed to have a condition
> -specified for it.  The condition is not parsed for validity until a shared
> -library is loaded that allows the pending breakpoint to resolve to a
> -valid location.
> +the line following the affected breakpoint.  If a breakpoint condition is
> +not supposed to be evaluated by @value{GDBN}, the condition evaluation mode will
> +be shown after the condition, in between parentheses; breakpoint
> +commands, if any, are listed after that.

The original text had only one sentence about breakpoint conditions,
and used a semi-colon to separate its two distinct parts.  You made
that single sentence into two, so it no longer makes sense to have the
part after the semi-colon be part of the second sentence, because it
is not related to the evaluation mode in any way.  Just make it a
separate sentence.

More importantly, the "condition evaluation mode" was never explained
before, so at the very least you should say something like

  ... 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.

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.

In any case, "is not supposed to be evaluated by GDB" is unnecessarily
complicated (supposed by whom?); "is evaluated by the GDB server"
sounds much more clear to me.

A question: in the current GDB, when we display the number of the
times a breakpoint was hit, do we count only the times when the
condition was true, or do we count all of them?  If the latter, having
the condition evaluated on the stub side will cause a different number
to be displayed.

> +@value{GDBN} handles conditional breakpoints by evaluating these conditions
> +when a breakpoint trigger happens.

I think we generally say "when a breakpoint breaks".  It is also
shorter.

>                                     If the condition is true, then the trigger
> +results in a stop, otherwise the process is resumed.

 If the condition is true, the process being debugged stops, otherwise ...

> +If a remote stub supports evaluating conditions on its end, @value{GDBN} can
> +download the conditional breakpoint, together with its conditions, to
> +the target.

I have a question: is it the stub who needs to support this, or is it
gdbserver?  You sometimes say this and sometimes that, but AFAIK the
server and the stub are two different pieces of software.  It could be
a source of confusion.

> +       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}.

What exactly is behind this somewhat mysterious sentence?  Which
"limitations apply"?  Does GDB provide some feedback or indication, at
the time of the "break" or "commands" commands, that this condition
will be evaluated by GDB rather than the stub?

> +Breakpoint conditions can also be evaluated on the target's side if
> +the remote stub supports it. Instead of evaluating the conditions locally,
                              ^^
Two spaces, please.

> +In this case, @value{GDBN} will only be notified of a breakpoint trigger
> +when its condition evaluates to true.  This mechanism provides a much faster
> +response time since the remote stub does not need to keep @value{GDBN}
> +informed about every breakpoint trigger, even those with false conditions.

I presume the "much faster response time" should be qualified by the
CPU speed of the target.  If that CPU is much slower than the one on
the host, it could be the other way around, no?

>  @item z0,@var{addr},@var{kind}
> -@itemx Z0,@var{addr},@var{kind}
> +@itemx Z0,@var{addr},@var{kind};@r{[};@var{cond expr}@r{]}@dots{}
>  @cindex @samp{z0} packet
>  @cindex @samp{Z0} packet
>  Insert (@samp{Z0}) or remove (@samp{z0}) a memory breakpoint at address
> @@ -34161,6 +34218,11 @@ A memory breakpoint is implemented by re
>  the breakpoint in bytes that should be inserted.  E.g., the @sc{arm}
>  and @sc{mips} can insert either a 2 or 4 byte breakpoint.  Some
>  architectures have additional meanings for @var{kind};
> +@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}.

> +                                                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?

> +@itemx Z1,@var{addr},@var{kind};@r{[};@var{cond expr}@r{]}@dots{}
                                         ^^^^^^^^^^^^^^^
Likewise.

> +and @var{cond expr} have the same meaning as in @samp{Z0} packets.

Likewise.

Thanks.



More information about the Gdb-patches mailing list