[Bug python/19808] New: python unwinder documentation could use some love

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=19808

Bug ID: 19808
Summary: python unwinder documentation could use some love
Product: gdb
Version: unknown
Status: NEW
Severity: normal
Priority: P2
Component: python
Assignee: unassigned at sourceware dot org
Reporter: tromey at sourceware dot org
Target Milestone: ---

The python unwinder documentation is a bit unclear, IMO, and could use
some updates.

The documentation for create_unwind_info says:

'sp, pc, special'
'frame_id_build_special (FRAME_ID.sp, FRAME_ID.pc,
FRAME_ID.special)'

'sp, pc'
'frame_id_build (FRAME_ID.sp, FRAME_ID.pc)'

This is the most common case.

'sp'
'frame_id_build_wild (FRAME_ID.sp)'


This doesn't really mean anything unless you already know quite a bit
about the gdb frame internals. I think that rather than refer to internal
functions, this text should explain the meaning of these different calls.
(Especially since I think two of these are generally uninteresting.)


Nothing in the docs explains the meaning of the parameters to the frame id.
It should at least mention that these are intended to be the "unchanging"
versions, say the CFA and the start PC of the function.

(Also, gdb should just export a generic frame id object so that all users
don't have to invent their own. This could be put in unwinder.py.)

[palves at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=19808

Pedro Alves <palves at redhat dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Blocks| |19923


Referenced Bugs:

https://sourceware.org/bugzilla/show_bug.cgi?id=19923
[Bug 19923] [tracker] Stack unwinder for OpenJDK

[adinn at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=19808

Andrew Dinn <adinn at redhat dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |adinn at redhat dot com

--- Comment #1 from Andrew Dinn <adinn at redhat dot com> ---
I found a few other things undocumented in the unwinder API that would be worth
mentioning.

1) values returned by pending_frame.read_register

passing an illegal register

pending_frame.read_register throws an Exception if you pass an illegal register
name for the pending frame's architecture.


retrieving an undefined register

if you pass a legal register name pending_frame.read_register always returns a
lazy gdb.Value even when the register's value is not available for retrieval.
in the latter case when you try to use the returned gdb.Value it throws an
Exception.


what is the type of a returned gdb.Value

the type of the gdb.Value returned by pending_frame.read_register is normally
the gdb.Type with name 'long long'. However, for a program counter register
(e.g. rip on x86) it is the type with name 'void (*) (void)' and for a stack
pointer or frame pointer register (rsp or rbp on x86) it is the type with name
'void *'. this is significant because the type determines the way that
automatic conversions to python ints/longs and comparisons with python
ints/longs are performed.

[n.b. I am not sure whether/how this generalises to other architectures e.g.
whether for, say, AArch64 values for register rfp have type 'void *']

2) safety of gdb operations invoked within the unwinder __call__ method

[this is probably a bug that should be fixed rather than a 'feature' to be
documented but documentation might help implementors to diagnose and deal with
errors]

unwinder implementations are advised to do as little work as possible inside
the unwinder __call__ method. a registered unwinder may be called in
circumstances where the entry frame to the stack frame hierarchy is not yet
established. in such circumstances invocations of certain gdb-supplied python
methods below __call__ may lead gdb to attempt to re-establish the stack frame
hierarchy. this can have two fatal consequences:

firstly, it may cause repeated recursive entry into the unwinder until python
detects a potential stack overflow and abandons the original __call__.

secondly, and much worse, gdb may reinitialise it's current stack hierarchy,
freeing the memory used to store the base frame that underlies the
pending_frame argument. as a consequence gdb will most likely crash with an
internal error when __call__ returns.

[The following may or may not be a useful addition to the documentation
depending upon whether the bug is fixed or not]

for example, in the OpenJDK unwinder calling

gdb.parse_and_eval("CodeCache::_heap")

falls foul of both the recursion and the frame freeing issues when the entry
frame has not been established.

similarly, executing

ip = pending_frame.read_register('rip')
if ip < hi_water:
. . .

falls foul of the frame freeing issue. confusingly, this only happens because
the gdb.Value appears first on the left. reversing the comparison and the
operands to

if hi_water >= ip:
. . .

bypasses the problem.

the OpenJDK unwinder avoids the recursion problem by detecting recursive entry
into the unwinder's __call__ method and backing out to return None from the top
level entry. this allows the standard gdb unwinders to establish a proper frame
stack.

the OpenJDK unwinder avoids the comparison problem by converting register
values to python longs (thereby also forcing early rather than lazy evaluation)
before performing comparisons.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=19808

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at sourceware dot org |tromey at sourceware dot org

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=19808

--- Comment #2 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <***@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e7b5068cc22f1a5707a9fd0310234b8d7300f227

commit e7b5068cc22f1a5707a9fd0310234b8d7300f227
Author: Tom Tromey <***@tromey.com>
Date: Sat Sep 8 13:25:34 2018 -0600

Update Python unwinder documentation

PR python/19808 points out a few issues in the Python unwinder
documentation. This patch update the documentation for
create_unwind_info and read_register to address the issues noted, and
adds a cautionary note about writing an unwinder.

gdb/doc/ChangeLog
2018-09-10 Tom Tromey <***@tromey.com>

PR python/19808:
* python.texi (Unwinding Frames in Python): Rewrite
create_unwind_info documentation. Update read_register
documentation and add a note about unwinder caution.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=19808

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
Target Milestone|--- |8.3

--- Comment #3 from Tom Tromey <tromey at sourceware dot org> ---
I think this is fixed enough; but if not, please open a new
bug with more details.