Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code

Pedro Alves
2022-09-30T14:50:42Z

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.