[01/10] gdb/doc: spring clean the Python unwinders documentation

  The documentation for the Python Unwinders API could do with some
improvement.  The 'Unwinder Skeleton Code' has an error: it says
'unwinders' when it should say 'unwinder' in one case.

Additionally, by placing the 'Unwinder Skeleton Code' before the
section 'Registering an Unwinder' we have skipping including the
registration line in the skeleton code.  But this is confusion for
users (I think) as the skeleton code is almost complete, except for
one missing line which the user has to figure out for themselves.  By
reordering the sections, it is now obvious that the registration
should be included in the skeleton code, and the example is therefore
almost complete.

Additionally, in the example skeleton code the way in which the
frame-id was being built (using the current stack point and program
counter is (a) not correct, and (b) counter to what is laid out in the
'Unwinder Input' section when describing building a frame-id.

I've removed the incorrect code and replaced it with more generic
comments indicating what needs to be done.  As the actual actions that
need to be performed are both architecture specific, and dependent on
the function being unwound, it's almost impossible to include more
exact code here, but I think what I'm proposing is less misleading
than what we had before.

I've also added more cross references.
---
 gdb/doc/python.texi | 82 ++++++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 38 deletions(-)
  

> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Fri, 10 Mar 2023 14:55:18 +0000
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> 
> The documentation for the Python Unwinders API could do with some
> improvement.  The 'Unwinder Skeleton Code' has an error: it says
> 'unwinders' when it should say 'unwinder' in one case.
> 
> Additionally, by placing the 'Unwinder Skeleton Code' before the
> section 'Registering an Unwinder' we have skipping including the
> registration line in the skeleton code.  But this is confusion for
> users (I think) as the skeleton code is almost complete, except for
> one missing line which the user has to figure out for themselves.  By
> reordering the sections, it is now obvious that the registration
> should be included in the skeleton code, and the example is therefore
> almost complete.
> 
> Additionally, in the example skeleton code the way in which the
> frame-id was being built (using the current stack point and program
> counter is (a) not correct, and (b) counter to what is laid out in the
> 'Unwinder Input' section when describing building a frame-id.
> 
> I've removed the incorrect code and replaced it with more generic
> comments indicating what needs to be done.  As the actual actions that
> need to be performed are both architecture specific, and dependent on
> the function being unwound, it's almost impossible to include more
> exact code here, but I think what I'm proposing is less misleading
> than what we had before.
> 
> I've also added more cross references.
> ---
>  gdb/doc/python.texi | 82 ++++++++++++++++++++++++---------------------
>  1 file changed, 44 insertions(+), 38 deletions(-)

Thanks.

>  @defun PendingFrame.create_unwind_info (frame_id)
>  Returns a new @code{gdb.UnwindInfo} instance identified by given
> -@var{frame_id}.  The argument is used to build @value{GDBN}'s frame ID
> -using one of functions provided by @value{GDBN}.  @var{frame_id}'s attributes
> -determine which function will be used, as follows:
> +@var{frame_id}.  The @var{frame_id} is used internally by @value{GDBN}
> +to identify the frames within the current thread's stack.  The
> +attributes of @var{frame_id} determine what type of frame ID is
> +created within @value{GDBN}:                        ^^^^^^^^

I think the "ID" part there should be dropped (you are talking about
creating frames, not IDs, right?).

> +Object files and program spaces can have unwinders registered with
> +them.  In addition, you can also register unwinders globally.
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Either "in addition" or "also", but not both.

> +same name.  An attempt to add an unwinder with an already existing
> +name raises an exception unless @var{replace} is @code{True}, in which
> +case the old unwinder is deleted and the new unwinder is registered in
> +its place.                                                          ^^
   ^^^^^^^^^
I believe "in its stead" is better English-wise.

> +@subheading Unwinder Precedence
> +
> +@value{GDBN} first calls the unwinders from all the object files in no
> +particular order, then the unwinders from the current program space,
> +then the globally registered unwinders, and finally the unwinders
> +builtin to @value{GDBN}.

This text is too short to justify a separate @subheading, IMO.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: Andrew Burgess <aburgess@redhat.com>
>> Date: Fri, 10 Mar 2023 14:55:18 +0000
>> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
>> 
>> The documentation for the Python Unwinders API could do with some
>> improvement.  The 'Unwinder Skeleton Code' has an error: it says
>> 'unwinders' when it should say 'unwinder' in one case.
>> 
>> Additionally, by placing the 'Unwinder Skeleton Code' before the
>> section 'Registering an Unwinder' we have skipping including the
>> registration line in the skeleton code.  But this is confusion for
>> users (I think) as the skeleton code is almost complete, except for
>> one missing line which the user has to figure out for themselves.  By
>> reordering the sections, it is now obvious that the registration
>> should be included in the skeleton code, and the example is therefore
>> almost complete.
>> 
>> Additionally, in the example skeleton code the way in which the
>> frame-id was being built (using the current stack point and program
>> counter is (a) not correct, and (b) counter to what is laid out in the
>> 'Unwinder Input' section when describing building a frame-id.
>> 
>> I've removed the incorrect code and replaced it with more generic
>> comments indicating what needs to be done.  As the actual actions that
>> need to be performed are both architecture specific, and dependent on
>> the function being unwound, it's almost impossible to include more
>> exact code here, but I think what I'm proposing is less misleading
>> than what we had before.
>> 
>> I've also added more cross references.
>> ---
>>  gdb/doc/python.texi | 82 ++++++++++++++++++++++++---------------------
>>  1 file changed, 44 insertions(+), 38 deletions(-)
>
> Thanks.
>
>>  @defun PendingFrame.create_unwind_info (frame_id)
>>  Returns a new @code{gdb.UnwindInfo} instance identified by given
>> -@var{frame_id}.  The argument is used to build @value{GDBN}'s frame ID
>> -using one of functions provided by @value{GDBN}.  @var{frame_id}'s attributes
>> -determine which function will be used, as follows:
>> +@var{frame_id}.  The @var{frame_id} is used internally by @value{GDBN}
>> +to identify the frames within the current thread's stack.  The
>> +attributes of @var{frame_id} determine what type of frame ID is
>> +created within @value{GDBN}:                        ^^^^^^^^
>
> I think the "ID" part there should be dropped (you are talking about
> creating frames, not IDs, right?).

Good spot.  I dropped the 'ID'.

>
>> +Object files and program spaces can have unwinders registered with
>> +them.  In addition, you can also register unwinders globally.
>           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Either "in addition" or "also", but not both.

This sentence has become:

  In addition, you can register unwinders globally.

>
>> +same name.  An attempt to add an unwinder with an already existing
>> +name raises an exception unless @var{replace} is @code{True}, in which
>> +case the old unwinder is deleted and the new unwinder is registered in
>> +its place.                                                          ^^
>    ^^^^^^^^^
> I believe "in its stead" is better English-wise.

Changed to '... this command installed in its stead.'

>
>> +@subheading Unwinder Precedence
>> +
>> +@value{GDBN} first calls the unwinders from all the object files in no
>> +particular order, then the unwinders from the current program space,
>> +then the globally registered unwinders, and finally the unwinders
>> +builtin to @value{GDBN}.
>
> This text is too short to justify a separate @subheading, IMO.

I folded this section into the previous function description.  The
previous function is how to register unwinders, so adding information
about the order in which registered unwinders are called feels like a
good fit.

>
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>

I added your tag, but the updated patch is below if you want to
take another pass.

Thank,
Andrew

---

commit 861c24ec4d306d7604818cff0cd5c6063292376f
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Wed Mar 8 15:26:32 2023 +0000

    gdb/doc: spring clean the Python unwinders documentation
    
    The documentation for the Python Unwinders API could do with some
    improvement.  The 'Unwinder Skeleton Code' has an error: it says
    'unwinders' when it should say 'unwinder' in one case.
    
    Additionally, by placing the 'Unwinder Skeleton Code' before the
    section 'Registering an Unwinder' we have skipping including the
    registration line in the skeleton code.  But this is confusion for
    users (I think) as the skeleton code is almost complete, except for
    one missing line which the user has to figure out for themselves.  By
    reordering the sections, it is now obvious that the registration
    should be included in the skeleton code, and the example is therefore
    almost complete.
    
    Additionally, in the example skeleton code the way in which the
    frame-id was being built (using the current stack point and program
    counter is (a) not correct, and (b) counter to what is laid out in the
    'Unwinder Input' section when describing building a frame-id.
    
    I've removed the incorrect code and replaced it with more generic
    comments indicating what needs to be done.  As the actual actions that
    need to be performed are both architecture specific, and dependent on
    the function being unwound, it's almost impossible to include more
    exact code here, but I think what I'm proposing is less misleading
    than what we had before.
    
    I've also added more cross references.
    
    Reviewed-By: Eli Zaretskii <eliz@gnu.org>

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index b04f1de2ddf..54d5660543a 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2784,9 +2784,10 @@
 
 @defun PendingFrame.create_unwind_info (frame_id)
 Returns a new @code{gdb.UnwindInfo} instance identified by given
-@var{frame_id}.  The argument is used to build @value{GDBN}'s frame ID
-using one of functions provided by @value{GDBN}.  @var{frame_id}'s attributes
-determine which function will be used, as follows:
+@var{frame_id}.  The @var{frame_id} is used internally by @value{GDBN}
+to identify the frames within the current thread's stack.  The
+attributes of @var{frame_id} determine what type of frame is
+created within @value{GDBN}:
 
 @table @code
 @item sp, pc
@@ -2841,6 +2842,32 @@
 @var{value} is a register value (a @code{gdb.Value} object).
 @end defun
 
+@subheading Registering an Unwinder
+
+Object files and program spaces can have unwinders registered with
+them.  In addition, you can register unwinders globally.
+
+The @code{gdb.unwinders} module provides the function to register an
+unwinder:
+
+@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
+@var{locus} specifies to which unwinder list to prepend the
+@var{unwinder}.  It can be either an object file (@pxref{Objfiles In
+Python}), a program space (@pxref{Progspaces In Python}), or
+@code{None}, in which case the unwinder is registered globally.  The
+newly added @var{unwinder} will be called before any other unwinder
+from the same locus.  Two unwinders in the same locus cannot have the
+same name.  An attempt to add an unwinder with an already existing
+name raises an exception unless @var{replace} is @code{True}, in which
+case the old unwinder is deleted and the new unwinder is registered in
+its place.
+
+@value{GDBN} first calls the unwinders from all the object files in no
+particular order, then the unwinders from the current program space,
+then the globally registered unwinders, and finally the unwinders
+builtin to @value{GDBN}.
+@end defun
+
 @subheading Unwinder Skeleton Code
 
 @value{GDBN} comes with the module containing the base @code{Unwinder}
@@ -2848,7 +2875,7 @@
 follows:
 
 @smallexample
-from gdb.unwinders import Unwinder
+from gdb.unwinder import Unwinder
 
 class FrameId(object):
     def __init__(self, sp, pc):
@@ -2857,53 +2884,30 @@
 
 
 class MyUnwinder(Unwinder):
-    def __init__(....):
-        super(MyUnwinder, self).__init___(<expects unwinder name argument>)
+    def __init__(self):
+        super().__init___("MyUnwinder_Name")
 
-    def __call__(pending_frame):
+    def __call__(self, pending_frame):
         if not <we recognize frame>:
             return None
-        # Create UnwindInfo.  Usually the frame is identified by the stack 
-        # pointer and the program counter.
-        sp = pending_frame.read_register(<SP number>)
-        pc = pending_frame.read_register(<PC number>)
+
+        # Create a FrameID.  Usually the frame is identified by a
+        # stack pointer and the function address.
+        sp = ... compute a stack address ...
+        pc = ... compute function address ...
         unwind_info = pending_frame.create_unwind_info(FrameId(sp, pc))
 
-        # Find the values of the registers in the caller's frame and 
+        # Find the values of the registers in the caller's frame and
         # save them in the result:
-        unwind_info.add_saved_register(<register>, <value>)
+        unwind_info.add_saved_register(<register-number>, <register-value>)
         ....
 
         # Return the result:
         return unwind_info
 
+gdb.unwinder.register_unwinder(<locus>, MyUnwinder(), <replace>)
 @end smallexample
 
-@subheading Registering an Unwinder
-
-Object files and program spaces can have unwinders registered with
-them.  In addition, you can also register unwinders globally.
-
-The @code{gdb.unwinders} module provides the function to register an
-unwinder:
-
-@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
-@var{locus} specifies to which unwinder list to prepend the
-@var{unwinder}.  It can be either an object file, a program space, or
-@code{None}, in which case the unwinder is registered globally.  The
-newly added @var{unwinder} will be called before any other unwinder from the
-same locus.  Two unwinders in the same locus cannot have the same
-name.  An attempt to add an unwinder with an already existing name raises
-an exception unless @var{replace} is @code{True}, in which case the
-old unwinder is deleted.
-@end defun
-
-@subheading Unwinder Precedence
-
-@value{GDBN} first calls the unwinders from all the object files in no
-particular order, then the unwinders from the current program space,
-and finally the unwinders from @value{GDBN}.
-
 @node Xmethods In Python
 @subsubsection Xmethods In Python
 @cindex xmethods in Python
@@ -4398,7 +4402,7 @@
 commands.  Setting this attribute to @code{True} will install the
 command for use.  If there is already a Python command with this name
 installed, the currently installed command will be uninstalled, and
-this command installed in its place.
+this command installed in its stead.
 @end defvar
 
 The following code snippet shows how a two trivial MI command can be
  

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Tue, 14 Mar 2023 09:27:29 +0000
> 
> > Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> 
> I added your tag, but the updated patch is below if you want to
> take another pass.

You are a GO.

Thanks.
  

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrew Burgess <aburgess@redhat.com>
>> Cc: gdb-patches@sourceware.org
>> Date: Tue, 14 Mar 2023 09:27:29 +0000
>> 
>> > Reviewed-By: Eli Zaretskii <eliz@gnu.org>
>> 
>> I added your tag, but the updated patch is below if you want to
>> take another pass.
>
> You are a GO.

I pushed this patch.

Thanks,
Andrew