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

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

[Luis Machado]

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

Hi,

This patch handles the documentation changes.

- Explain new condition-evaluation breakpoint subcommand.
- Explain info break changes.
- Describe support for stub-side breakpoint conditions.
- Explain new conditional-breakpoints-packet.
- Explain z0/z1 extensions.
- Mention new ConditionalBreakpoint feature.

Luis
lgustavo@codesourcery.com

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

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

	* NEWS: Mention target-side conditional breakpoint support.

	* 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-04 14:02:53.066431999 -0200
@@ -9,6 +9,12 @@
 * The binary "gdbtui" can no longer be built or installed.
   Use "gdb -tui" instead.
 
+* GDBServer supports evaluating breakpoint conditions.  When
+  support is advertised by GDBServer, GDB will send the breakpoint
+  conditions in bytecode form to GDBServer.  GDBServer
+  will only report the breakpoint trigger to GDB when its condition
+  evaluates to true.
+
 *** 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-04 14:02:53.082431999 -0200
@@ -3485,11 +3485,13 @@ the appropriate shared library is loaded
 
 @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.  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 +3683,45 @@ 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 trigger happens.  If the condition is true, then the trigger
+results in a stop, otherwise the process is resumed.
+
+If a remote stub supports evaluating conditions on its end, @value{GDBN} can
+download the conditional breakpoint, together with its conditions, to
+the target.
+
+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 remote stub which in turn receives the
+triggers and reports them back to GDB for condition evaluation.  This
+method is potentially slow for debugging remote targets through a
+network connection.
+
+@item set breakpoint condition-evaluation stub
+This option commands @value{GDBN} to download breakpoint conditions
+to the remote stub at the moment of their insertion.  The remote stub
+is responsible for evaluating the conditional expression and reporting
+triggers back to @value{GDBN} whenever the result of the evaluation
+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}.
+
+@item set breakpoint condition-evaluation auto
+This is the default mode.  If the remote stub supports evaluating breakpoint
+conditionals on its end, @value{GDBN} will download breakpoint conditions to
+the target (limitations mentioned previously apply).  If the remote stub does
+not support evaluating breakpoint conditionals, then @value{GDBN} assumes
+all breakpoint conditions will be evaluated 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 +4381,18 @@ 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 remote stub 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 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.
+
 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 +17485,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 stub-side breakpoint condition evaluation}
 @end multitable
 
 @node Remote Stub
@@ -34149,7 +34206,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 +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
+that should be evaluated on the target's side.  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}.
+
 see @ref{Architecture-Specific Protocol Details}.
 
 @emph{Implementation note: It is possible for a target to copy or move
@@ -34179,7 +34241,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 +34250,7 @@ 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 +35055,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 +35197,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 remote stub accepts and implements evaluation of conditional expressions
+defined for breakpoints.  The remote stub 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}).

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

[Yao Qi]

On 01/05/2012 10:56 PM, Luis Machado wrote:
> 	* NEWS: Mention target-side conditional breakpoint support.
                        ^^^^^^^^^^^
To align with subject, I doubt it should be "stub-side" rather than
"target-side".

-- 
Yao (齐尧)

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

[Stan Shebs]

On 1/5/12 6:19 PM, Yao Qi wrote:
> On 01/05/2012 10:56 PM, Luis Machado wrote:
>> 	* NEWS: Mention target-side conditional breakpoint support.
>                          ^^^^^^^^^^^
> To align with subject, I doubt it should be "stub-side" rather than
> "target-side".
>

I would prefer to say "target-side" everywhere actually - no one really 
knows what we mean by "stub", and if they try to understand it by 
looking at, say, our foo-stub.c files, they'll get really confused. :-)  
"Target-side" has the right connotation of "outside GDB" but without 
committing to a specific implementation strategy - like conditional 
breakpoint tests in h/w debug registers, which would be the most awesome...

Stan

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

[Joel Brobecker]

> >To align with subject, I doubt it should be "stub-side" rather than
> >"target-side".
> >
> 
> I would prefer to say "target-side" everywhere actually - no one
> really knows what we mean by "stub", and if they try to understand
> it by looking at, say, our foo-stub.c files, they'll get really
> confused. :-)  "Target-side" has the right connotation of "outside
> GDB" but without committing to a specific implementation strategy -
> like conditional breakpoint tests in h/w debug registers, which
> would be the most awesome...

Indeed. Just to be sure: we should also change the name of the setting
from "stub" to "target".

-- 
Joel

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

[Eli Zaretskii]

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

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

[Luis Machado]

[-- 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}).

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

[Eli Zaretskii]

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

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

[Pedro Alves]

On 01/05/2012 02:56 PM, Luis Machado wrote:
> +@item @code{conditional-breakpoints-packet}
> +@tab @code{Z0 and Z1}
> +@tab @code{Support for stub-side breakpoint condition evaluation}
>   @end multitable

What about watchpoints, and other Z packets?

>   @node Remote Stub
> @@ -34149,7 +34206,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

That seems to renders as ‘Z0,addr,kind;[;cond expr]...’

Hmm.  If we're reusing/extending Z packets, then I'd rather this doesn't
make it hard to support future extensions to the packet (e.g., thread specific
breakpoints), while at the same time _not_ supporting stub-side
condition evaluation.  So imagine that I'll add a new thread-id (pPID.TID)
field to this packet, and I will specify an empty condition.   How will the
result look like?

   Z0,addr,kind;;pPID.TID

?

Wouldn't it be better something like:

  Z0,addr,kind,cond_expr_list,other_future_extensionsÂ’

and cond_expr_list being a ;-separated list?

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

[Luis Machado]

On 01/06/2012 02:22 PM, Pedro Alves wrote:
> On 01/05/2012 02:56 PM, Luis Machado wrote:
>> +@item @code{conditional-breakpoints-packet}
>> +@tab @code{Z0 and Z1}
>> +@tab @code{Support for stub-side breakpoint condition evaluation}
>>   @end multitable
>
> What about watchpoints, and other Z packets?
I wasn't going to touch them for now, but i suppose they can be easily 
extended later on.

>
>>   @node Remote Stub
>> @@ -34149,7 +34206,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
>
> That seems to renders as ‘Z0,addr,kind;[;cond expr]...’
Not right, i'll fix it in the next iteration after clearing the packet 
format issue.

>
> Hmm.  If we're reusing/extending Z packets, then I'd rather this doesn't
> make it hard to support future extensions to the packet (e.g., thread 
> specific
> breakpoints), while at the same time _not_ supporting stub-side
> condition evaluation.  So imagine that I'll add a new thread-id 
> (pPID.TID)
> field to this packet, and I will specify an empty condition.   How 
> will the
> result look like?
>
>   Z0,addr,kind;;pPID.TID
>
> ?
>
> Wouldn't it be better something like:
>
>  Z0,addr,kind,cond_expr_list,other_future_extensionsÂ’
>
> and cond_expr_list being a ;-separated list?

Makes sense. The problem is that each conditional expression is of the 
form "size,expr". That's why i used a different marker to signal the 
beginning of an extension area.

We could name each extension with a marker (cond, thread_id, itset 
etc...). Would it be too much overhead in the packet? Like the following.

Z0,addr,kind,cond=size0,expr0;size1,expr1;...;sizen,exprn,itset=<itset>,foo=<foo>...

Something that comes to mind regarding adding multiple extensions to 
these packets is the packet size limitation. We may have to add 
"continuation" packets if the extensions get too numerous. If that 
happens, it probably wouldn't be wise to rely on specific field ordering.

-- 
Luis
lgustavo@codesourcery.com

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

[Pedro Alves]

On 01/06/2012 05:07 PM, Luis Machado wrote:
> On 01/06/2012 02:22 PM, Pedro Alves wrote:
>> On 01/05/2012 02:56 PM, Luis Machado wrote:
>>> +@item @code{conditional-breakpoints-packet}
>>> +@tab @code{Z0 and Z1}
>>> +@tab @code{Support for stub-side breakpoint condition evaluation}
>>>   @end multitable
>>
>> What about watchpoints, and other Z packets?
> I wasn't going to touch them for now, but i suppose they can be easily extended later on.

If you handled them now, we'd avoid a new qSupported feature later on.  :-)

>>>   @node Remote Stub
>>> @@ -34149,7 +34206,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
>>
>> That seems to renders as ‘Z0,addr,kind;[;cond expr]...’
> Not right, i'll fix it in the next iteration after clearing the packet format issue.
> 
>>
>> Hmm.  If we're reusing/extending Z packets, then I'd rather this doesn't
>> make it hard to support future extensions to the packet (e.g., thread specific
>> breakpoints), while at the same time _not_ supporting stub-side
>> condition evaluation.  So imagine that I'll add a new thread-id (pPID.TID)
>> field to this packet, and I will specify an empty condition.   How will the
>> result look like?
>>
>>   Z0,addr,kind;;pPID.TID
>>
>> ?
>>
>> Wouldn't it be better something like:
>>
>>  Z0,addr,kind,cond_expr_list,other_future_extensionsÂ’
>>
>> and cond_expr_list being a ;-separated list?
> 
> Makes sense. The problem is that each conditional expression is of the form "size,expr". 

Where does the ',' requisite come from?  Can't we use something else (e.g. ':' or '.')?
What happens if we have a field in the future that wants a ';' ?  We run out of
separator characters.  :-)

> That's why i used a different marker to signal the beginning of an extension area.
> 
> We could name each extension with a marker (cond, thread_id, itset etc...). Would it be too much overhead in the packet? Like the following.

Might make sense, no strong opinion.

> Z0,addr,kind,cond=size0,expr0;size1,expr1;...;sizen,exprn,itset=<itset>,foo=<foo>...
> 
> Something that comes to mind regarding adding multiple extensions to these packets is the packet size limitation. We may have to add "continuation" packets if the extensions get too numerous. If that happens, it probably wouldn't be wise to rely on specific field ordering.
>