[PATCH 1/2] gdb/python doc: Add register_unwinder to code example [PATCH 2/2] gdb/python doc: Add enable property to the unwinder Minor documentation changes to the unwinder example code with the goal of having all the features described in the prose also reflect on the code.
It is now clear in the example code that an unwinder needs to be registered to be used. --- gdb/doc/python.texi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index cb5283e03c0..5b7fba798a9 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder): # Return the result: return unwind_info +# To use an unwinder it needs to be registered and enabled. +gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>) @end smallexample @subheading Registering a Unwinder -- 2.25.1
The enable property is now also in the example code making it clear how it is used and how it influences the operation of an unwinder. --- gdb/doc/python.texi | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index 5b7fba798a9..cb148d9b0ea 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2796,6 +2796,10 @@ class FrameId(object): class MyUnwinder(Unwinder): def __init__(....): super(MyUnwinder, self).__init___(<expects unwinder name argument>) + # If set to True the unwinder will be enabled. + # If upon registration the enable property is True, the unwinder will be usable immediately. + # gdb or code may change value. + self.enabled = <True|False> def __call__(pending_frame): if not <we recognize frame>: -- 2.25.1
> Date: Sat, 28 May 2022 13:55:08 +0200
> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Paulo Neves <ptsneves@gmail.com>
>
> It is now clear in the example code that an unwinder needs to be
> registered to be used.
> ---
> gdb/doc/python.texi | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index cb5283e03c0..5b7fba798a9 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder):
> # Return the result:
> return unwind_info
>
> +# To use an unwinder it needs to be registered and enabled.
> +gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
> @end smallexample
Thanks, I think this needs to be reviewed by someone who knows how to
program the Python unwinders. As far as Texinfo is concerned, this is
fine.
> Date: Sat, 28 May 2022 13:55:09 +0200 > From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org> > Cc: Paulo Neves <ptsneves@gmail.com> > > The enable property is now also in the example code making it clear how > it is used and how it influences the operation of an unwinder. > --- > gdb/doc/python.texi | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi > index 5b7fba798a9..cb148d9b0ea 100644 > --- a/gdb/doc/python.texi > +++ b/gdb/doc/python.texi > @@ -2796,6 +2796,10 @@ class FrameId(object): > class MyUnwinder(Unwinder): > def __init__(....): > super(MyUnwinder, self).__init___(<expects unwinder name argument>) > + # If set to True the unwinder will be enabled. > + # If upon registration the enable property is True, the unwinder will be usable immediately. This last line is too long for an @example, please break it into two. > + # gdb or code may change value. > + self.enabled = <True|False> > > def __call__(pending_frame): > if not <we recognize frame>: Other than that, this needs to be reviewed by someone who knows about programming unwinders in Python. Thanks.
It is now clear in the example code that an unwinder needs to be registered to be used. --- gdb/doc/python.texi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index cb5283e03c0..5b7fba798a9 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder): # Return the result: return unwind_info +# To use an unwinder it needs to be registered and enabled. +gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>) @end smallexample @subheading Registering a Unwinder -- 2.25.1
The enable property is now also in the example code making it clear how it is used and how it influences the operation of an unwinder. --- gdb/doc/python.texi | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index 5b7fba798a9..444e332d752 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2796,6 +2796,11 @@ class FrameId(object): class MyUnwinder(Unwinder): def __init__(....): super(MyUnwinder, self).__init___(<expects unwinder name argument>) + # If set to True the unwinder will be enabled. + # If upon registration the enable property is True, the unwinder + # will be usable immediately. + # gdb or code may change value. + self.enabled = <True|False> def __call__(pending_frame): if not <we recognize frame>: -- 2.25.1
Replace gdb.unwinders with gdb.unwinder. The gdb.unwinders module does not and trying to import it leads to errors. Other instances of the manual correctly refer to gdb.unwinder. --- gdb/doc/python.texi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index 444e332d752..59232750099 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2785,7 +2785,7 @@ class. Derive your unwinder class from it and structure the code as follows: @smallexample -from gdb.unwinders import Unwinder +from gdb.unwinder import Unwinder class FrameId(object): def __init__(self, sp, pc): @@ -2828,7 +2828,7 @@ gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>) An object file, a program space, and the @value{GDBN} proper can have unwinders registered with it. -The @code{gdb.unwinders} module provides the function to register a +The @code{gdb.unwinder} module provides the function to register a unwinder: @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False) -- 2.25.1
On 5/28/22 14:22, Eli Zaretskii wrote: >> Date: Sat, 28 May 2022 13:55:09 +0200 >> From: Paulo Neves via Gdb-patches<gdb-patches@sourceware.org> >> Cc: Paulo Neves<ptsneves@gmail.com> >> >> The enable property is now also in the example code making it clear how >> it is used and how it influences the operation of an unwinder. >> --- >> gdb/doc/python.texi | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi >> index 5b7fba798a9..cb148d9b0ea 100644 >> --- a/gdb/doc/python.texi >> +++ b/gdb/doc/python.texi >> @@ -2796,6 +2796,10 @@ class FrameId(object): >> class MyUnwinder(Unwinder): >> def __init__(....): >> super(MyUnwinder, self).__init___(<expects unwinder name argument>) >> + # If set to True the unwinder will be enabled. >> + # If upon registration the enable property is True, the unwinder will be usable immediately. > This last line is too long for an @example, please break it into two. Done > >> + # gdb or code may change value. >> + self.enabled = <True|False> >> >> def __call__(pending_frame): >> if not <we recognize frame>: > Other than that, this needs to be reviewed by someone who knows about > programming unwinders in Python. > > Thanks. Ok, I will be waiting then and ping according to the guidelines. I added a 3rd patch that also corrects a mistake in the module name mentioned in the example and doc.
Paulo Neves via Gdb-patches <gdb-patches@sourceware.org> writes: > Replace gdb.unwinders with gdb.unwinder. The gdb.unwinders module does > not and trying to import it leads to errors. Other instances of the > manual correctly refer to gdb.unwinder. This patch is fine. Thanks, Andrew > --- > gdb/doc/python.texi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi > index 444e332d752..59232750099 100644 > --- a/gdb/doc/python.texi > +++ b/gdb/doc/python.texi > @@ -2785,7 +2785,7 @@ class. Derive your unwinder class from it and structure the code as > follows: > > @smallexample > -from gdb.unwinders import Unwinder > +from gdb.unwinder import Unwinder > > class FrameId(object): > def __init__(self, sp, pc): > @@ -2828,7 +2828,7 @@ gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>) > An object file, a program space, and the @value{GDBN} proper can have > unwinders registered with it. > > -The @code{gdb.unwinders} module provides the function to register a > +The @code{gdb.unwinder} module provides the function to register a > unwinder: > > @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False) > -- > 2.25.1
Paulo Neves via Gdb-patches <gdb-patches@sourceware.org> writes: > It is now clear in the example code that an unwinder needs to be > registered to be used. > --- > gdb/doc/python.texi | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi > index cb5283e03c0..5b7fba798a9 100644 > --- a/gdb/doc/python.texi > +++ b/gdb/doc/python.texi > @@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder): > # Return the result: > return unwind_info > > +# To use an unwinder it needs to be registered and enabled. > +gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>) > @end smallexample I've had reason to read the unwinder example before, and I think it's probably my most hated example in our manual. I absolutely hate that the example is not real, executable, EXAMPLE, code, it's just seems like documentation in another form. That said, I think its not fair for me to require that your additions to the existing example be executable, when the rest of the example is not. But, if you _did_ want to improve this whole example, then I'd certainly be in favour! I do however, this that with this addition, the example code is now in the wrong place. Notice your example now makes use of `register_unwinder`, but this method is not discussed in detail until after the example code. I think that the whole subsection titled `Unwinder Skeleton Code` should be moved later, probably to just before `@node Xmethods In Python`, that is, right at the end of the section on unwinders. Thanks, Andrew > > @subheading Registering a Unwinder > -- > 2.25.1
Paulo Neves via Gdb-patches <gdb-patches@sourceware.org> writes: > The enable property is now also in the example code making it clear how > it is used and how it influences the operation of an unwinder. > --- > gdb/doc/python.texi | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi > index 5b7fba798a9..444e332d752 100644 > --- a/gdb/doc/python.texi > +++ b/gdb/doc/python.texi > @@ -2796,6 +2796,11 @@ class FrameId(object): > class MyUnwinder(Unwinder): > def __init__(....): > super(MyUnwinder, self).__init___(<expects unwinder name argument>) > + # If set to True the unwinder will be enabled. > + # If upon registration the enable property is True, the unwinder > + # will be usable immediately. > + # gdb or code may change value. > + self.enabled = <True|False> Everything I said about patch #1 applies here too. I think your comment could be shortened to just: # If set to True the unwinder will be enabled and available for use. Given that text I think most users will assume that setting the field to `True` in the constructor will result in the unwinder being available upon registration. Thanks, Andrew > > def __call__(pending_frame): > if not <we recognize frame>: > -- > 2.25.1
Hello Andrew On 5/30/22 11:55, Andrew Burgess wrote: > Paulo Neves via Gdb-patches<gdb-patches@sourceware.org> writes: > >> It is now clear in the example code that an unwinder needs to be >> registered to be used. >> --- >> gdb/doc/python.texi | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi >> index cb5283e03c0..5b7fba798a9 100644 >> --- a/gdb/doc/python.texi >> +++ b/gdb/doc/python.texi >> @@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder): >> # Return the result: >> return unwind_info >> >> +# To use an unwinder it needs to be registered and enabled. >> +gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>) >> @end smallexample > I've had reason to read the unwinder example before, and I think it's > probably my most hated example in our manual. > > I absolutely hate that the example is not real, executable, EXAMPLE, > code, it's just seems like documentation in another form. I think the fact it was not executable was the reason that nobody noticed the import was wrong. > > That said, I think its not fair for me to require that your additions to > the existing example be executable, when the rest of the example is > not. But, if you _did_ want to improve this whole example, then I'd > certainly be in favour! When you mean you want it to be executable, does this mean i should to remove the meta-values like "<SP number>" sprinkled in the code and replace them with comments? Also by real example do you want it to be functional? I am concerned an example that does any meaningful thing would be confusing. Something i looked at when trying to get my unwinder working was the gdb/testsuite/gdb.python/py-unwind.py test. It could be used but adds unrelated logic. > > I do however, this that with this addition, the example code is now in > the wrong place. Notice your example now makes use of > `register_unwinder`, but this method is not discussed in detail until > after the example code. > > I think that the whole subsection titled `Unwinder Skeleton Code` should > be moved later, probably to just before `@node Xmethods In Python`, that > is, right at the end of the section on unwinders. I can move the example to the last subsection. > > Thanks, > Andrew > >> >> @subheading Registering a Unwinder >> -- >> 2.25.1 I am interested in getting this section improved. Just give me the guidelines to the questions above and I will prepare the patches. Paulo Neves
On 5/30/22 11:57, Andrew Burgess wrote: > Paulo Neves via Gdb-patches <gdb-patches@sourceware.org> writes: > >> The enable property is now also in the example code making it clear how >> it is used and how it influences the operation of an unwinder. >> --- >> gdb/doc/python.texi | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi >> index 5b7fba798a9..444e332d752 100644 >> --- a/gdb/doc/python.texi >> +++ b/gdb/doc/python.texi >> @@ -2796,6 +2796,11 @@ class FrameId(object): >> class MyUnwinder(Unwinder): >> def __init__(....): >> super(MyUnwinder, self).__init___(<expects unwinder name argument>) >> + # If set to True the unwinder will be enabled. >> + # If upon registration the enable property is True, the unwinder >> + # will be usable immediately. >> + # gdb or code may change value. >> + self.enabled = <True|False> > Everything I said about patch #1 applies here too. > > I think your comment could be shortened to just: > > # If set to True the unwinder will be enabled and available for use. > > Given that text I think most users will assume that setting the field to > `True` in the constructor will result in the unwinder being available > upon registration. I think that part can be indeed removed but I think the hint that gdb may change values of your unwinder is important. I wrote these changes though the eyes of the difficulties I had as an absolute GDB extension newbie. > Thanks, > Andrew > > >> >> def __call__(pending_frame): >> if not <we recognize frame>: >> -- >> 2.25.1
It is now clear in the example code that an unwinder needs to be registered to be used. --- gdb/doc/python.texi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index ba5a9b315e1..228d515a817 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2823,6 +2823,8 @@ class MyUnwinder(Unwinder): # Return the result: return unwind_info +# To use an unwinder it needs to be registered and enabled. +gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>) @end smallexample @subheading Registering a Unwinder -- 2.25.1
The enable property is now also in the example code making it clear how it is used and how it influences the operation of an unwinder. --- gdb/doc/python.texi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index 228d515a817..3e4833a4a41 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2805,6 +2805,9 @@ class FrameId(object): class MyUnwinder(Unwinder): def __init__(....): super(MyUnwinder, self).__init___(<expects unwinder name argument>) + # If set to True the unwinder will be enabled. + # gdb or code may change this value. + self.enabled = <True|False> def __call__(pending_frame): if not <we recognize frame>: -- 2.25.1
Replace gdb.unwinders with gdb.unwinder. The gdb.unwinders module does not and trying to import it leads to errors. Other instances of the manual correctly refer to gdb.unwinder. --- gdb/doc/python.texi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index 3e4833a4a41..e2821755851 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2794,7 +2794,7 @@ class. Derive your unwinder class from it and structure the code as follows: @smallexample -from gdb.unwinders import Unwinder +from gdb.unwinder import Unwinder class FrameId(object): def __init__(self, sp, pc): @@ -2835,7 +2835,7 @@ gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>) An object file, a program space, and the @value{GDBN} proper can have unwinders registered with it. -The @code{gdb.unwinders} module provides the function to register a +The @code{gdb.unwinder} module provides the function to register a unwinder: @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False) -- 2.25.1
This way all the concepts mentioned in the skeleton code are previously detailed. --- gdb/doc/python.texi | 50 ++++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index e2821755851..1deeae82b9b 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2787,6 +2787,31 @@ values see @ref{gdbpy_frame_read_register,,Frame.read_register}. @var{value} is a register value (a @code{gdb.Value} object). @end defun +@subheading Registering a Unwinder + +An object file, a program space, and the @value{GDBN} proper can have +unwinders registered with it. + +The @code{gdb.unwinder} module provides the function to register a +unwinder: + +@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False) +@var{locus} is specifies an object file or a program space to which +@var{unwinder} is added. Passing @code{None} or @code{gdb} adds +@var{unwinder} to the @value{GDBN}'s global unwinder list. 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 a unwinder with 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}. + @subheading Unwinder Skeleton Code @value{GDBN} comes with the module containing the base @code{Unwinder} @@ -2830,31 +2855,6 @@ class MyUnwinder(Unwinder): gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>) @end smallexample -@subheading Registering a Unwinder - -An object file, a program space, and the @value{GDBN} proper can have -unwinders registered with it. - -The @code{gdb.unwinder} module provides the function to register a -unwinder: - -@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False) -@var{locus} is specifies an object file or a program space to which -@var{unwinder} is added. Passing @code{None} or @code{gdb} adds -@var{unwinder} to the @value{GDBN}'s global unwinder list. 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 a unwinder with 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 -- 2.25.1
> Date: Fri, 3 Jun 2022 18:01:36 +0200
> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Paulo Neves <ptsneves@gmail.com>
>
> It is now clear in the example code that an unwinder needs to be
> registered to be used.
> ---
> gdb/doc/python.texi | 2 ++
> 1 file changed, 2 insertions(+)
OK, thanks.
> Date: Fri, 3 Jun 2022 18:01:37 +0200
> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Paulo Neves <ptsneves@gmail.com>
>
> The enable property is now also in the example code making it clear how
> it is used and how it influences the operation of an unwinder.
> ---
> gdb/doc/python.texi | 3 +++
> 1 file changed, 3 insertions(+)
This is okay, thanks.
> Date: Fri, 3 Jun 2022 18:01:38 +0200
> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Paulo Neves <ptsneves@gmail.com>
>
> Replace gdb.unwinders with gdb.unwinder. The gdb.unwinders module does
> not and trying to import it leads to errors. Other instances of the
> manual correctly refer to gdb.unwinder.
> ---
> gdb/doc/python.texi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
OK, thanks.
> Date: Fri, 3 Jun 2022 18:01:39 +0200 > From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org> > Cc: Paulo Neves <ptsneves@gmail.com> > > +@subheading Registering a Unwinder > + > +An object file, a program space, and the @value{GDBN} proper can have > +unwinders registered with it. > + > +The @code{gdb.unwinder} module provides the function to register a > +unwinder: > + > +@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False) > +@var{locus} is specifies an object file or a program space to which ^^ The "is" part should be deleted. > +@var{unwinder} is added. Passing @code{None} or @code{gdb} adds > +@var{unwinder} to the @value{GDBN}'s global unwinder list. First, @value{GDBN}, not @code{gdb}. And also, this sentence doesn't read well ("passing None or gdb adds..."), I guess it's some typo or missing or redundant word. Thanks.
On 6/3/22 18:20, Eli Zaretskii wrote:
>> Date: Fri, 3 Jun 2022 18:01:39 +0200
>> From: Paulo Neves via Gdb-patches<gdb-patches@sourceware.org>
>> Cc: Paulo Neves<ptsneves@gmail.com>
>>
>> +@subheading Registering a Unwinder
>> +
>> +An object file, a program space, and the @value{GDBN} proper can have
>> +unwinders registered with it.
>> +
>> +The @code{gdb.unwinder} module provides the function to register a
>> +unwinder:
>> +
>> +@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
>> +@var{locus} is specifies an object file or a program space to which
> ^^
> The "is" part should be deleted.
>
>> +@var{unwinder} is added. Passing @code{None} or @code{gdb} adds
>> +@var{unwinder} to the @value{GDBN}'s global unwinder list.
> First, @value{GDBN}, not @code{gdb}.
>
> And also, this sentence doesn't read well ("passing None or gdb
> adds..."), I guess it's some typo or missing or redundant word.
>
> Thanks.
This patch is purely a move from one place to another, as suggested by Andrew.
The issues you mention are not introduced in this patch, but i will try to
fix the issues you mention on another commit and then rebase this patch on top of it.
Paulo Neves
Hello all,
Any idea when this can be merged or if there anything missing?
Paulo Neves
On 5/28/22 14:20, Eli Zaretskii wrote:
>> Date: Sat, 28 May 2022 13:55:08 +0200
>> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Paulo Neves <ptsneves@gmail.com>
>>
>> It is now clear in the example code that an unwinder needs to be
>> registered to be used.
>> ---
>> gdb/doc/python.texi | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
>> index cb5283e03c0..5b7fba798a9 100644
>> --- a/gdb/doc/python.texi
>> +++ b/gdb/doc/python.texi
>> @@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder):
>> # Return the result:
>> return unwind_info
>>
>> +# To use an unwinder it needs to be registered and enabled.
>> +gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
>> @end smallexample
> Thanks, I think this needs to be reviewed by someone who knows how to
> program the Python unwinders. As far as Texinfo is concerned, this is
> fine.
> Date: Tue, 21 Jun 2022 13:17:32 +0200
> Cc: gdb-patches@sourceware.org
> From: Paulo Neves <ptsneves@gmail.com>
>
> Hello all,
>
> Any idea when this can be merged or if there anything missing?
Like I said before: this needs to be approved by someone who knows
about Python unwinder programming. If the text is correct, the
Texinfo use was already approved by me.
On 6/21/22 14:19, Eli Zaretskii wrote:
>> Date: Tue, 21 Jun 2022 13:17:32 +0200
>> Cc: gdb-patches@sourceware.org
>> From: Paulo Neves <ptsneves@gmail.com>
>>
>> Hello all,
>>
>> Any idea when this can be merged or if there anything missing?
> Like I said before: this needs to be approved by someone who knows
> about Python unwinder programming. If the text is correct, the
> Texinfo use was already approved by me.
Hello Eli,
Could you point me out somebody who knows the python unwinder part that
i can ping?
Paulo Neves
> Date: Fri, 30 Sep 2022 13:59:06 +0200
> Cc: gdb-patches@sourceware.org
> From: Paulo Neves <ptsneves@gmail.com>
>
> On 6/21/22 14:19, Eli Zaretskii wrote:
> >> Date: Tue, 21 Jun 2022 13:17:32 +0200
> >> Cc: gdb-patches@sourceware.org
> >> From: Paulo Neves <ptsneves@gmail.com>
> >>
> >> Hello all,
> >>
> >> Any idea when this can be merged or if there anything missing?
> > Like I said before: this needs to be approved by someone who knows
> > about Python unwinder programming. If the text is correct, the
> > Texinfo use was already approved by me.
> Hello Eli,
> Could you point me out somebody who knows the python unwinder part that
> i can ping?
Any other of the global maintainers, really: Joel, Pedro, Simon, and
many others.
On 2022-06-03 5:20 p.m., Eli Zaretskii via Gdb-patches wrote:
>> Date: Fri, 3 Jun 2022 18:01:39 +0200
>> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Paulo Neves <ptsneves@gmail.com>
>>
>> +@subheading Registering a Unwinder
>> +
>> +An object file, a program space, and the @value{GDBN} proper can have
>> +unwinders registered with it.
>> +
>> +The @code{gdb.unwinder} module provides the function to register a
>> +unwinder:
>> +
>> +@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
>> +@var{locus} is specifies an object file or a program space to which
> ^^
> The "is" part should be deleted.
>
>> +@var{unwinder} is added. Passing @code{None} or @code{gdb} adds
>> +@var{unwinder} to the @value{GDBN}'s global unwinder list.
>
> First, @value{GDBN}, not @code{gdb}.
>
> And also, this sentence doesn't read well ("passing None or gdb
> adds..."), I guess it's some typo or missing or redundant word.
>
I think the intent is to say that you can pass literal "None" or "gdb"
to gdb.unwinder.register_unwinder, for "locus" argument. Reading like that,
it makes sense to me, and replacing with @value{GDBN} would be incorrect.
However, looking at the code itself, I don't see "gdb" being accepted as input:
From gdb/python/lib/gdb/unwinder.py :
def register_unwinder(locus, unwinder, replace=False):
"""Register unwinder in given locus.
The unwinder is prepended to the locus's unwinders list. Unwinder
name should be unique.
Arguments:
locus: Either an objfile, progspace, or None (in which case
the unwinder is registered globally).
unwinder: An object of a gdb.Unwinder subclass
replace: If True, replaces existing unwinder with the same name.
Otherwise, raises exception if unwinder with the same
name already exists.
Returns:
Nothing.
Raises:
RuntimeError: Unwinder name is not unique
TypeError: Bad locus type
"""
if locus is None:
if gdb.parameter("verbose"):
gdb.write("Registering global %s unwinder ...\n" % unwinder.name)
locus = gdb
elif isinstance(locus, gdb.Objfile) or isinstance(locus, gdb.Progspace):
if gdb.parameter("verbose"):
gdb.write(
"Registering %s unwinder for %s ...\n" % (unwinder.name, locus.filename)
)
else:
raise TypeError("locus should be gdb.Objfile or gdb.Progspace or None")
i = 0
for needle in locus.frame_unwinders:
if needle.name == unwinder.name:
if replace:
del locus.frame_unwinders[i]
else:
raise RuntimeError("Unwinder %s already exists." % unwinder.name)
i += 1
locus.frame_unwinders.insert(0, unwinder)
gdb.invalidate_cached_frames()
Above, when you pass "None", locus does end up being "gdb",
if locus is None:
if gdb.parameter("verbose"):
gdb.write("Registering global %s unwinder ...\n" % unwinder.name)
locus = gdb
^^^^^^^^^^^
but, if you pass locus=gdb as argument, seems to me you'll reach the else branch:
else:
raise TypeError("locus should be gdb.Objfile or gdb.Progspace or None")
And note that error string does not mention the "gdb" possibility.
So I think the manual should be fixed by dropping the "or @code{gdb}", resulting in:
Passing @code{None} adds @var{unwinder} to the @value{GDBN}'s global unwinder list.
Actually, the "the" in "to the @value{GDBN}'s" looks wrong to me. So I think we
should really say:
Passing @code{None} adds @var{unwinder} to @value{GDBN}'s global unwinder list.
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> Date: Fri, 30 Sep 2022 15:50:20 +0100
>
> So I think the manual should be fixed by dropping the "or @code{gdb}", resulting in:
>
> Passing @code{None} adds @var{unwinder} to the @value{GDBN}'s global unwinder list.
>
> Actually, the "the" in "to the @value{GDBN}'s" looks wrong to me. So I think we
> should really say:
>
> Passing @code{None} adds @var{unwinder} to @value{GDBN}'s global unwinder list.
How about
If @var{locus} is @code{None}, that stands for @value{GDBN}'s global
unwinder list.
I think this is clearer and less likely to cause misinterpretation.
On 2022-09-30 4:53 p.m., Eli Zaretskii wrote: >> Cc: gdb-patches@sourceware.org >> From: Pedro Alves <pedro@palves.net> >> Date: Fri, 30 Sep 2022 15:50:20 +0100 >> >> So I think the manual should be fixed by dropping the "or @code{gdb}", resulting in: >> >> Passing @code{None} adds @var{unwinder} to the @value{GDBN}'s global unwinder list. >> >> Actually, the "the" in "to the @value{GDBN}'s" looks wrong to me. So I think we >> should really say: >> >> Passing @code{None} adds @var{unwinder} to @value{GDBN}'s global unwinder list. > > How about > > If @var{locus} is @code{None}, that stands for @value{GDBN}'s global > unwinder list. > > I think this is clearer and less likely to cause misinterpretation. > Seems fine in isolation, but looking at the whole paragraph, how about similar text to what's in gdb/python/lib/gdb/unwinder.py? Seems clear enough to me. I also noticed a couple instances of "a unwinder", also fixed. While at it, "GDB proper" seems like a weird conception. How about this? From 7c5e7875bed7f4316823db67d95821c88c679daf Mon Sep 17 00:00:00 2001 From: Pedro Alves <pedro@palves.net> Date: Fri, 30 Sep 2022 17:23:03 +0100 Subject: [PATCH] Python Change-Id: I98c1b1000e1063815238e945ca71ec6f37b5702e --- gdb/doc/python.texi | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index 2692211f388..4a7b047c3e0 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2861,21 +2861,21 @@ class MyUnwinder(Unwinder): @end smallexample -@subheading Registering a Unwinder +@subheading Registering an Unwinder -An object file, a program space, and the @value{GDBN} proper can have -unwinders registered with it. +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 a +The @code{gdb.unwinders} module provides the function to register an unwinder: @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False) -@var{locus} is specifies an object file or a program space to which -@var{unwinder} is added. Passing @code{None} or @code{gdb} adds -@var{unwinder} to the @value{GDBN}'s global unwinder list. The newly -added @var{unwinder} will be called before any other unwinder from the +@var{locus} specifies which unwinder list the @var{unwinder} is +prepended to. 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 a unwinder with already existing name raises +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 base-commit: cfc0ffd31e90972296b81d362a7d83eb60eb21c0 -- 2.36.0
> Cc: ptsneves@gmail.com, gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> Date: Fri, 30 Sep 2022 17:41:50 +0100
>
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 2692211f388..4a7b047c3e0 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -2861,21 +2861,21 @@ class MyUnwinder(Unwinder):
>
> @end smallexample
>
> -@subheading Registering a Unwinder
> +@subheading Registering an Unwinder
>
> -An object file, a program space, and the @value{GDBN} proper can have
> -unwinders registered with it.
> +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 a
> +The @code{gdb.unwinders} module provides the function to register an
> unwinder:
>
> @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
> -@var{locus} is specifies an object file or a program space to which
> -@var{unwinder} is added. Passing @code{None} or @code{gdb} adds
> -@var{unwinder} to the @value{GDBN}'s global unwinder list. The newly
> -added @var{unwinder} will be called before any other unwinder from the
> +@var{locus} specifies which unwinder list the @var{unwinder} is
> +prepended to. 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 a unwinder with already existing name raises
> +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
That's fine, but let's avoid passive tense where we can (and make the
sentence easier to grasp while at that). So:
@var{locus} specifies to which unwinder list to prepend the
@var{unwinder}.
Hi,
On 2022-09-30 6:37 p.m., Eli Zaretskii wrote:
>> Cc: ptsneves@gmail.com, gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>> Date: Fri, 30 Sep 2022 17:41:50 +0100
>>
>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
>> index 2692211f388..4a7b047c3e0 100644
>> --- a/gdb/doc/python.texi
>> +++ b/gdb/doc/python.texi
>> @@ -2861,21 +2861,21 @@ class MyUnwinder(Unwinder):
>>
>> @end smallexample
>>
>> -@subheading Registering a Unwinder
>> +@subheading Registering an Unwinder
>>
>> -An object file, a program space, and the @value{GDBN} proper can have
>> -unwinders registered with it.
>> +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 a
>> +The @code{gdb.unwinders} module provides the function to register an
>> unwinder:
>>
>> @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
>> -@var{locus} is specifies an object file or a program space to which
>> -@var{unwinder} is added. Passing @code{None} or @code{gdb} adds
>> -@var{unwinder} to the @value{GDBN}'s global unwinder list. The newly
>> -added @var{unwinder} will be called before any other unwinder from the
>> +@var{locus} specifies which unwinder list the @var{unwinder} is
>> +prepended to. 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 a unwinder with already existing name raises
>> +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
>
> That's fine, but let's avoid passive tense where we can (and make the
> sentence easier to grasp while at that). So:
>
> @var{locus} specifies to which unwinder list to prepend the
> @var{unwinder}.
>
Finally done, and merged.
Thanks,
Pedro Alves
Thank you very much
Paulo Neves
On Fri, 21 Oct 2022 at 23:07, Pedro Alves <pedro@palves.net> wrote:
> Hi,
>
> On 2022-09-30 6:37 p.m., Eli Zaretskii wrote:
> >> Cc: ptsneves@gmail.com, gdb-patches@sourceware.org
> >> From: Pedro Alves <pedro@palves.net>
> >> Date: Fri, 30 Sep 2022 17:41:50 +0100
> >>
> >> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> >> index 2692211f388..4a7b047c3e0 100644
> >> --- a/gdb/doc/python.texi
> >> +++ b/gdb/doc/python.texi
> >> @@ -2861,21 +2861,21 @@ class MyUnwinder(Unwinder):
> >>
> >> @end smallexample
> >>
> >> -@subheading Registering a Unwinder
> >> +@subheading Registering an Unwinder
> >>
> >> -An object file, a program space, and the @value{GDBN} proper can have
> >> -unwinders registered with it.
> >> +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 a
> >> +The @code{gdb.unwinders} module provides the function to register an
> >> unwinder:
> >>
> >> @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
> >> -@var{locus} is specifies an object file or a program space to which
> >> -@var{unwinder} is added. Passing @code{None} or @code{gdb} adds
> >> -@var{unwinder} to the @value{GDBN}'s global unwinder list. The newly
> >> -added @var{unwinder} will be called before any other unwinder from the
> >> +@var{locus} specifies which unwinder list the @var{unwinder} is
> >> +prepended to. 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 a unwinder with already existing name raises
> >> +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
> >
> > That's fine, but let's avoid passive tense where we can (and make the
> > sentence easier to grasp while at that). So:
> >
> > @var{locus} specifies to which unwinder list to prepend the
> > @var{unwinder}.
> >
>
> Finally done, and merged.
>
> Thanks,
> Pedro Alves
>
>
[-- Attachment #1: Type: text/plain, Size: 2390 bytes --] Thank you very much Paulo Neves On Fri, 21 Oct 2022 at 23:07, Pedro Alves <pedro@palves.net> wrote: > Hi, > > On 2022-09-30 6:37 p.m., Eli Zaretskii wrote: > >> Cc: ptsneves@gmail.com, gdb-patches@sourceware.org > >> From: Pedro Alves <pedro@palves.net> > >> Date: Fri, 30 Sep 2022 17:41:50 +0100 > >> > >> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi > >> index 2692211f388..4a7b047c3e0 100644 > >> --- a/gdb/doc/python.texi > >> +++ b/gdb/doc/python.texi > >> @@ -2861,21 +2861,21 @@ class MyUnwinder(Unwinder): > >> > >> @end smallexample > >> > >> -@subheading Registering a Unwinder > >> +@subheading Registering an Unwinder > >> > >> -An object file, a program space, and the @value{GDBN} proper can have > >> -unwinders registered with it. > >> +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 a > >> +The @code{gdb.unwinders} module provides the function to register an > >> unwinder: > >> > >> @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False) > >> -@var{locus} is specifies an object file or a program space to which > >> -@var{unwinder} is added. Passing @code{None} or @code{gdb} adds > >> -@var{unwinder} to the @value{GDBN}'s global unwinder list. The newly > >> -added @var{unwinder} will be called before any other unwinder from the > >> +@var{locus} specifies which unwinder list the @var{unwinder} is > >> +prepended to. 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 a unwinder with already existing name raises > >> +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 > > > > That's fine, but let's avoid passive tense where we can (and make the > > sentence easier to grasp while at that). So: > > > > @var{locus} specifies to which unwinder list to prepend the > > @var{unwinder}. > > > > Finally done, and merged. > > Thanks, > Pedro Alves > >