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

[Luis Machado <luis_gustavo@mentor.com>]

[-- Attachment #1: Type: text/plain, Size: 8580 bytes --]

Hi,

Before going through the changes, i'd like to point out that i removed 
all "stub" and "gdbserver" references that i had previously used. I'm 
using "target" instead.

As Stan suggested before, i want to make it clear that "target" does not 
mean just gdbserver.  For remote debugging, it means any remote stub 
that supports such features.

If GDB can talk to an agent, "target" also means the agent that is 
attached to the inferior we're debugging, since it will be able to do 
target-side breakpoint condition evaluation, so the term "stub" is a bit 
too narrow in scope.

At this stage and in the scope of this patch series, "target" means 
remote stub's (like gdbserver).

On 01/06/2012 06:28 AM, Eli Zaretskii wrote:
>> +* GDBServer supports evaluating breakpoint conditions.
> Suggest to rephrase
>
>    GDBServer supports evaluation of breakpoint conditions.
>

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

I added a few other entries that i missed before.

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

Fixed. How does it look now?

> 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? Is it realistic to expect users will play with all the 
required switches all the time? I've seen discussions about reducing the 
number of switches and buttons for GDB, so i'd like to hear some 
thoughts about this.
> 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.
>
Fixed.

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

The number of hits for a conditional breakpoint is the number of times 
it triggered with its condition indicating true. So we should be safe there.
>> +@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.

Fixed.

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

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

This is part of the stub/target naming confusion.  It does not need to 
be only gdbserver, but any remote stub that is capable of evaluating 
agent expressions and advertises such feature.
>> +       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?

I've updated the phrasing and hopefully it should be clearer now. Agent 
expressions have a narrower scope compared to GDB's expressions. We 
can't do evaluation on the target's side if the conditional expression 
uses values that are only available on the host's side.
>> +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.
>
Fixed.
>> +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?
Yes. We should not be overly optimistic here. I've updated the phrasing 
to present a more realistic scenario.
>>   @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}.
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. In case we have, say, 5 
conditional locations at the same address, we will send 5 conditional 
expressions.
>> +@itemx Z1,@var{addr},@var{kind};@r{[};@var{cond expr}@r{]}@dots{}
>                                           ^^^^^^^^^^^^^^^
> Likewise.
Fixed.
>> +and @var{cond expr} have the same meaning as in @samp{Z0} packets.
> Likewise.
>
> Thanks.
Fixed.

Thanks!

Luis
lgustavo@codesourcery.com

[-- Attachment #2: 0000-target-side-breaks-docs.diff --]
[-- Type: text/x-patch, Size: 9636 bytes --]

2012-01-06  Luis Machado  <lgustavo@codesourcery.com>

	* NEWS: Mention target-side conditional breakpoint support,
	new condition-evaluation breakpoint subcommand and remote
	packet extensions.

	* doc/gdb.texinfo (Setting Breakpoints): Mention and explain the
	condition-evaluation breakpoint parameter.
	Mention condition-evaluation mode being shown in "info break".
	(Break Conditions): Add description for target-side
	conditional breakpoints.
	(Remote Configuration): Mention conditional-breakpoints-packet. 
	(Packets): Add cond-expr parameter to Z0/Z1 packets and explain
	cond-expr.
	(General Query Packets): Mention new ConditionalBreakpoint feature.
	
Index: gdb/gdb/NEWS
===================================================================
--- 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.
+
 *** Changes in GDB 7.4
 
 * GDB now handles ambiguous linespecs more consistently; the existing
Index: gdb/gdb/doc/gdb.texinfo
===================================================================
--- gdb.orig/gdb/doc/gdb.texinfo	2012-01-04 14:02:33.650431998 -0200
+++ gdb/gdb/doc/gdb.texinfo	2012-01-06 11:44:54.582431998 -0200
@@ -3484,12 +3484,17 @@ the appropriate shared library is loaded
 @end table
 
 @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.
+If a breakpoint is conditional, there are two evaluation modes: "gdb" and
+"target".  If mode is "gdb", breakpoint condition evaluation is done by
+@value{GDBN} on the host's side.  If it is "target", then the condition
+is evaluated by the target.  The @code{info break} command shows
+the condition on the line following the affected breakpoint, together with
+its condition evaluation mode in between parentheses.
+
+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.
 
 @noindent
 @code{info break} with a breakpoint
@@ -3681,6 +3686,46 @@ controlling the inferior in all-stop mod
 @code{breakpoint always-inserted} mode is off.
 @end table
 
+@value{GDBN} handles conditional breakpoints by evaluating these conditions
+when a breakpoint breaks.  If the condition is true, then the process being
+debugged stops, otherwise the process is resumed.
+
+If the target supports evaluating conditions on its end, @value{GDBN} may
+download the breakpoint, together with its conditions, to it.
+
+This feature can be controlled via the following commands:
+
+@kindex set breakpoint condition-evaluation
+@kindex show breakpoint condition-evaluation
+@table @code
+@item set breakpoint condition-evaluation gdb
+This option commands @value{GDBN} to evaluate the breakpoint
+conditions on its own, that is, on the host's side.  Unconditional
+breakpoints are sent to the target which in turn receives the
+triggers and reports them back to GDB for condition evaluation.  This
+is the standard evaluation mode.
+
+@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.
+
+@item set breakpoint condition-evaluation auto
+This is the default mode.  If the target supports evaluating breakpoint
+conditions on its end, @value{GDBN} will download breakpoint conditions to
+the target (limitations mentioned previously apply).  If the target does
+not support breakpoint condition evaluation, then @value{GDBN} will fallback
+to evaluating all these conditions by itself, on the host's side.
+@end table
+
+
 @cindex negative breakpoint numbers
 @cindex internal @value{GDBN} breakpoints
 @value{GDBN} itself sometimes sets breakpoints in your program for
@@ -4340,6 +4385,19 @@ conditions for the
 purpose of performing side effects when a breakpoint is reached
 (@pxref{Break Commands, ,Breakpoint Command Lists}).
 
+Breakpoint conditions can also be evaluated on the target's side if
+the target supports it.  Instead of evaluating the conditions locally,
+@value{GDBN} encodes the expression into an agent expression
+(@pxref{Agent Expressions}) suitable for execution on the target,
+independently of @value{GDBN}.  Global variables become raw memory
+locations, locals become stack accesses, and so forth.
+
+In this case, @value{GDBN} will only be notified of a breakpoint trigger
+when its condition evaluates to true.  This mechanism may provide faster
+response times depending on the performance characteristics of the target
+since it does not need to keep @value{GDBN} informed about
+every breakpoint trigger, even those with false conditions.
+
 Break conditions can be specified when a breakpoint is set, by using
 @samp{if} in the arguments to the @code{break} command.  @xref{Set
 Breaks, ,Setting Breakpoints}.  They can also be changed at any time
@@ -17432,6 +17490,10 @@ are:
 @item @code{disable-randomization}
 @tab @code{QDisableRandomization}
 @tab @code{set disable-randomization}
+
+@item @code{conditional-breakpoints-packet}
+@tab @code{Z0 and Z1}
+@tab @code{Support for target-side breakpoint condition evaluation}
 @end multitable
 
 @node Remote Stub
@@ -34149,7 +34211,7 @@ avoid potential problems with duplicate
 be implemented in an idempotent way.}
 
 @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 +34223,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 list of conditional expressions in bytecode
+form that should be evaluated on the target's side.  These are the
+conditions that should be taken into consideration when deciding if
+the breakpoint trigger should be reported back to @var{GDBN}.
+
 see @ref{Architecture-Specific Protocol Details}.
 
 @emph{Implementation note: It is possible for a target to copy or move
@@ -34179,7 +34246,8 @@ for an error
 @end table
 
 @item z1,@var{addr},@var{kind}
-@itemx Z1,@var{addr},@var{kind}
+@itemx Z1,@var{addr},@var{kind};@r{[};@var{cond-expr}@r{]}@dots{}
+
 @cindex @samp{z1} packet
 @cindex @samp{Z1} packet
 Insert (@samp{Z1}) or remove (@samp{z1}) a hardware breakpoint at
@@ -34187,7 +34255,8 @@ address @var{addr}.
 
 A hardware breakpoint is implemented using a mechanism that is not
 dependant on being able to modify the target's memory.  @var{kind}
-has the same meaning as in @samp{Z0} packets.
+and @var{cond-expr} have the same meaning as in @samp{Z0} packets.
+
 
 @emph{Implementation note: A hardware breakpoint is not affected by code
 movement.}
@@ -34992,6 +35061,11 @@ These are the currently defined stub fea
 @tab @samp{-}
 @tab No
 
+@item @samp{ConditionalBreakpoints}
+@tab No
+@tab @samp{-}
+@tab No
+
 @item @samp{ConditionalTracepoints}
 @tab No
 @tab @samp{-}
@@ -35129,6 +35203,11 @@ indicated it supports them in its @samp{
 The remote stub understands the @samp{qXfer:osdata:read} packet
 ((@pxref{qXfer osdata read}).
 
+@item ConditionalBreakpoints
+The target accepts and implements evaluation of conditional expressions
+defined for breakpoints.  The target will only report breakpoint triggers
+when such conditions are true (@pxref{Conditions, ,Break Conditions}).
+
 @item ConditionalTracepoints
 The remote stub accepts and implements conditional expressions defined
 for tracepoints (@pxref{Tracepoint Conditions}).