Re: [PATCH] Program Breakpoints

[Eli Zaretskii <eliz at gnu dot org>]

> Date: Fri, 17 Apr 2009 18:32:59 -0700
> From: Ross Morley <ross@tensilica.com>
> CC: Maxim Grigoriev <maxim@tensilica.com>
> 
> The user manual has been updated. I will update the internals
> manual after this patch has been (possibly changed and) accepted.

Thank you.  I have a few comments for the documentation patch:

> +Some programs may contain embedded break or trap instructions that are
> +unknown to GDB. These are called @dfn{program breakpoints} because they 
              ^^^
"@value{GDBN}".

> +belong to the program itself. If encountered, a target may stop execution 
> +and report this to GDB along with how much to increment the PC to step 
> +over it (a target is not required to do this).

Please don't talk about a "target" in the user's manual: this
terminology is unknown to them.  Please talk about the "inferior" or
the "program being debugged".

> +@item program-breakpoint
> +A program breakpoint (trap instruction unknown to GDB) was reached.
                                                     ^^^
"@value{GDBN}"

> @@ -26723,6 +26731,12 @@
>  @value{GDBN} should use @samp{qXfer:libraries:read} to fetch a new
>  list of loaded libraries.  @var{r} is ignored.
>  
> +@item trap
> +The packet indicates that a target-specific break or trap instruction 
> +was hit. @var{r} is the size of the instruction if the PC is pointing
> +to it, else 0 (for example if the hardware already incremented the PC).
> +@var{r} is ignored if the instruction was planted by @value{GDBN}.
> +
>  The packet indicates that the target cannot continue replaying 

Something strange happened with this hunk.  In the current CVS, the
manual in this part looks like this:

    @cindex shared library events, remote reply
    @item library
    The packet indicates that the loaded libraries have changed.
    @value{GDBN} should use @samp{qXfer:libraries:read} to fetch a new
    list of loaded libraries.  @var{r} is ignored.

    @cindex replay log events, remote reply
    @item replaylog
    The packet indicates that the target cannot continue replaying
    logged execution events, because it has reached the end (or the
    beginning when executing backward) of the log.  The value of @var{r}
    will be either @samp{begin} or @samp{end}.  @xref{Reverse Execution},
    for more information.

So it seems to me that you are plugging your part in the middle of
another @item, but then where's the "@item replaylog" line and the one
preceding it, with @cindex"?  What am I missing?

Finally, please make sure every period that ends a sentence has 2
spaces, not 1, after it.

> --- gdb/NEWS	31 Mar 2009 20:21:06 -0000	1.305
> +++ gdb/NEWS	17 Apr 2009 18:55:25 -0000
> @@ -3,6 +3,10 @@
>  
>  *** Changes since GDB 6.8
>  
> +* GDB now supports handling (embedded) program break or trap instructions
> +that are unknown to GDB. These are called program breakpoints because they
> +belong to the program itself.

This is good, but I suggest to add a sentence telling how would GDB
manifest these program breakpoints.  Also, "program breakpoints"
should be in quotes, as you are introducing a new term.

Thanks.