Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gdb7 hooks to make it easier to debug Python #52280

Closed
davidmalcolm opened this issue Feb 28, 2010 · 22 comments
Closed

Add gdb7 hooks to make it easier to debug Python #52280

davidmalcolm opened this issue Feb 28, 2010 · 22 comments
Assignees

Comments

@davidmalcolm
Copy link
Member

BPO 8032
Nosy @loewis, @doko42, @gpshead, @jcea, @mdickinson, @benjaminp, @davidmalcolm
Files
  • add-gdb7-python-hooks-to-trunk.patch: Patch against trunk (r78517)
  • add-gdb7-python-hooks-to-trunk-v2.patch
  • diff-of-gdb7-hooks-v2-relative-to-v1.diff: What's changed in v2 compared to v1, for reference
  • add-gdb7-python-hooks-to-trunk-v3.patch: Version 3 of patch, against trunk (r78840)
  • diff-of-gdb7-hooks-v3-relative-to-v2.diff: What's changed in v3 compared to v2, for reference
  • add-gdb7-python-hooks-to-trunk-v4.patch: Version 4 of patch, against trunk (79422)
  • diff-of-gdb7-hooks-v4-relative-to-v3.diff: What's changed in v4 compared to v3, for reference
  • add-gdb7-python-hooks-to-trunk-v5.patch: Version 5 of patch, against trunk (r79517)
  • diff-of-gdb7-hooks-v5-relative-to-v4.diff: What's changed in v5 compared to v4, for reference
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/loewis'
    closed_at = <Date 2010-04-01.07:44:22.981>
    created_at = <Date 2010-02-28.23:40:31.163>
    labels = []
    title = 'Add gdb7 hooks to make it easier to debug Python'
    updated_at = <Date 2014-02-14.22:52:30.768>
    user = 'https://github.com/davidmalcolm'

    bugs.python.org fields:

    activity = <Date 2014-02-14.22:52:30.768>
    actor = 'jcea'
    assignee = 'loewis'
    closed = True
    closed_date = <Date 2010-04-01.07:44:22.981>
    closer = 'loewis'
    components = ['Demos and Tools']
    creation = <Date 2010-02-28.23:40:31.163>
    creator = 'dmalcolm'
    dependencies = []
    files = ['16403', '16496', '16497', '16525', '16526', '16656', '16657', '16715', '16716']
    hgrepos = []
    issue_num = 8032
    keywords = ['patch']
    message_count = 22.0
    messages = ['100226', '100491', '100537', '100538', '100662', '100882', '100885', '100886', '101720', '101721', '102058', '102059', '102067', '102190', '102193', '102195', '102197', '102283', '102284', '102285', '102286', '102288']
    nosy_count = 10.0
    nosy_names = ['loewis', 'doko', 'gregory.p.smith', 'jcea', 'mark.dickinson', 'davidfraser', 'jyasskin', 'scott.tsai', 'benjamin.peterson', 'dmalcolm']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue8032'
    versions = ['Python 2.7']

    @davidmalcolm
    Copy link
    Member Author

    gdb 7 can be extended with Python code, allowing the writing of domain-specific pretty-printers and commands.

    I've been working on gdb 7 hooks to make it easier to debug python itself, as mentioned here:
    https://fedoraproject.org/wiki/Features/EasierPythonDebugging

    I'm attaching a patch for merger with "trunk". My hope is to be the maintainer of this work, although I do not yet have SVN commit rights (see http://mail.python.org/pipermail/python-committers/2010-February/000711.html )

    The code is fully compatible with the existing "Misc/gdbinit" macros - you can freely use both as needed.

    = Two versions of Python =
    This code is intended to run inside gdb. There are potentially two Python versions involved here: that of the "inferior process" (the one being debugged), and that of the debugger.

    As I understand things, gdb only supports embedding Python 2 at the moment. This code is thus targeting that version of Python.

    So far, I've attempted to keep this code so that it will run when the "inferior process" is either Python 2 or Python 3. I could vary this code in the py3k branch if desired. The code would then track the "inferior process" version of Python, so that code to debug Python 3 would live in the py3k branch. That code would still be written for Python 2, though.

    = Makefile changes =

    The Makefile installs the gdb hooks to -gdb.py relative to the built python (even if you're not using them), which some may find irritating. It needs to do this for the test_gdb.py selftest to work (and for the gdb hooks to be usable if you're actually using them to debug your build of Python).

    Should I write a gdb configuration test to check for the availability of gdb built with python?

    I've added the file to .hgignore and .bzrignore. IIRC, a similar thing can be done to the SVN metadata (I don't think this is expressable as a patch, though). Alternatively, I could wire up the gdb tests to load the file from its location in the source tree.

    However, I intend for this code to be installed to a location alongside the build Python, so that it can be automatically detected and used by gdb. Typically this means copying it to the path of the ELF file with a -gdb.py file. In my RPM builds I add an extra copy, locating it relative to the location of the stripped DWARF data (e.g. /usr/lib/debug/usr/lib64/libpython26.so.1.0-gdb.py)

    = Selftests =
    The selftest runs whatever version of "gdb" is in the path, which then invokes the built version of python, running simple "print" commands and verifying that gdb is corrrectly representing the results in backtraces (even in the face of corrupt data). I haven't fully tested the error cases yet (e.g. for when gdb is not installed).

    The tests take about 14 seconds to run on my box:
    [david@brick trunk-gdb]$ time ./python Lib/test/regrtest.py -v -s test_gdb
    The CWD is now /tmp/test_python_19369
    test_gdb
    test_NULL_ob_type (test.test_gdb.DebuggerTests) ... ok
    test_NULL_ptr (test.test_gdb.DebuggerTests)
    Ensure that a NULL PyObject* is handled gracefully ... ok
    test_classic_class (test.test_gdb.DebuggerTests) ... ok
    test_corrupt_ob_type (test.test_gdb.DebuggerTests) ... ok
    test_corrupt_tp_flags (test.test_gdb.DebuggerTests) ... ok
    test_corrupt_tp_name (test.test_gdb.DebuggerTests) ... ok
    test_dicts (test.test_gdb.DebuggerTests) ... ok
    test_getting_backtrace (test.test_gdb.DebuggerTests) ... ok
    test_int (test.test_gdb.DebuggerTests) ... ok
    test_lists (test.test_gdb.DebuggerTests) ... ok
    test_long (test.test_gdb.DebuggerTests) ... ok
    test_modern_class (test.test_gdb.DebuggerTests) ... ok
    test_singletons (test.test_gdb.DebuggerTests) ... ok
    test_strings (test.test_gdb.DebuggerTests) ... ok
    test_subclassing_list (test.test_gdb.DebuggerTests) ... ok
    test_subclassing_tuple (test.test_gdb.DebuggerTests)
    This should exercise the negative tp_dictoffset code in the ... ok
    test_tuples (test.test_gdb.DebuggerTests) ... ok
    test_unicode (test.test_gdb.DebuggerTests) ... ok

    ----------------------------------------------------------------------
    Ran 18 tests in 13.233s

    OK
    1 test OK.
    [36833 refs]

    real 0m13.599s
    user 0m11.771s
    sys 0m1.384s

    = Platform support =
    I don't have access to anything other than Linux, so I've no idea how well this stuff works on other platforms. My testing so far has been on Fedora, though I've heard of successful usage of this on Debian.

    = Legal stuff =

    Earlier versions of this code were licensed under the LGPL 2.1
    I'm relicensing the code to be "under the same license as Python itself", assuming that's legally OK. Do I need to state that in the file header, or is that redundant?

    I'm in the process of doing the PSF Contributor Agreement paperwork (as an individual); waiting to get my hands on a fax machine. My employer, Red Hat, has agreed for me to retain copyright on all contributions I make to Python.

    @davidmalcolm
    Copy link
    Member Author

    (I faxed in my contributor agreement to the PSF on 2010-03-03)

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Mar 6, 2010

    Maybe I'm using it incorrectly, but my first attempt to use it failed:

    gdb) b PyEval_EvalCodeEx
    Breakpoint 1 at 0x80fc51d: file Python/ceval.c, line 3020.
    (gdb) c
    Continuing.
    1+2

    Breakpoint 1, PyEval_EvalCodeEx (co=0xb7d86928, globals=Traceback (most recent call last):
      File "/home/martin/work/27/python-gdb.py", line 574, in to_string
        return stringify(proxyval)
      File "/home/martin/work/27/python-gdb.py", line 526, in stringify
        return repr(val)
      File "/home/martin/work/27/python-gdb.py", line 271, in __repr__
        for arg, val in self.attrdict.iteritems()])
    AttributeError: 'FakeRepr' object has no attribute 'iteritems'
    , locals=<unknown at remote 0x0>, args=0xb7d7eb00, argcount=1, kws=0x0, kwcount=0, defs=0x827c9b0, defcount=1, closure=<unknown at remote 0x0>)
        at Python/ceval.c:3020
    3020            register PyObject *retval = NULL;

    This was with the Python 2.7 trunk.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Mar 6, 2010

    Attribution question: would it be ok if all mentioning of your name in the patch was removed accept for the one in Misc/ACKS? I'm concerned that statements of authorship may become incorrect over time (if people start contributing to it), yet nobody will dare to remove your copyright notice (for example).

    @davidmalcolm
    Copy link
    Member Author

    Martin: thanks for reviewing this.

    Re msg100537: sorry about the inauspicious start. I've added some bulletproofing for the case you discovered, and added two new unit tests.

    Re msg100538: OK. I've removed my name and the copyright notice in all places apart from the Misc/ACKS file. There are still some comments in the code where I use the first person singular.

    I'm attaching an updated patch against trunk

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Mar 11, 2010

    I find the printing of frame objects confusing:

    3033 f = PyFrame_New(tstate, co, globals, locals);
    (gdb)
    3034 if (f == NULL)
    (gdb) p f
    $3 = File /home/martin/work/27/Lib/encodings/utf_8.py, line 15, in decode ()

    I didn't recognize that this is actually the output of the Frame; I recommend something like

    Frame %x, for file ....

    Also, it prints NULL PyObject* as "<unknown at remote 0x0>". I think null pointers should be special cased, and just be printed as 0x0. Also, what is the "remote" keyword? Aren't all pointers remote in this application? I'd hope that local Python objects never show up.

    @davidmalcolm
    Copy link
    Member Author

    Thanks for reviewing the patch.

    I've changed the pretty-printing of NULL pointers to "0x0" as suggested, and I've updated frame printing. Frames are now printed like this
    (gdb) p f
    $1 = Frame 0x827e544, for file /home/david/coding/python-gdb/crasher.py, line 19, in <module> ()

    ...so that in a gdb backtrace you might see:
    #18 0x080f9aab in PyEval_EvalFrameEx (f=Frame 0x827e544, for file /home/david/coding/python-gdb/crasher.py, line 19, in <module> (), throwflag=0) at Python/ceval.c:2665

    Is this what you had in mind?

    As for the use of the word "remote",this is mostly for my own sanity, but I suspect it is needed. My rationale for this is that there are two address spaces at play: the local address space with the gdb process, and the remote one within the inferior process. It's possible for addresses with the gdb process to be printed for any object with a NULL tp_repr in its class, and if it were, it would be confusing which address space is it.

    For example, if within gdb I run this command:
    (gdb) python print repr(gdb.selected_frame())
    <gdb.Frame object at 0xb753ef98>

    the hexadecimal value that's printed is an address within gdb's address space, that of the python object wrapping the gdb frame information.

    I did deliberately model the FakeRepr representation on the output of PyObject_Repr when tp_repr is NULL:
       PyString_FromFormat("<%s object at %p>", Py_TYPE(v)->tp_name, v);
    adding the "remote" word to disambiguate things.

    Is that OK? It seemed to me like the best compromise of readability and lack of ambiguity.

    I'm attaching an updated version of the patch (version 3), and am about to attach a diff against the last version (which was version 2)

    There are some other substantial changes in the patch:

    • Line numbers reported in frames were incorrect; I've fixed this, mimicking the "dis" module/PyCode_Addr2Line
    • I've started to add various "py-" commands (with selftests):
      • "py-list" command, which tries to mimic gdb's "list" command, but at the level of Python source code
      • "py-up" and "py-down" command, which go up and down the stack, based on the location of PyEval_EvalFrameEx frames
      • "py-bt" which prints a python-level backtrace, based on the location of PyEval_EvalFrameEx frames
    • I added a trivial script (Lib/test/test_gdb_sample.py) for use by the test cases for the above
    • Fixed a bug in PyStringObjectPtr.proxyval() so that it now supports strings with embedded NUL characters
    • Various comment fixes.

    I split out the test cases into multiple classes, invoking them all. All pass on my system, taking under 20 seconds ("Ran 28 tests in 17.770s")

    @davidmalcolm
    Copy link
    Member Author

    Attaching diff from v2 to v3

    @davidmalcolm
    Copy link
    Member Author

    I'm attaching a new version of the patch (v4), against svn trunk (r79422)

    Changes since v3:

    • added support for PySetObject (set/frozenset)
    • added support for PyBaseExceptionObject (BaseException)
    • fixed a signed vs unsigned char issue that led to exceptions in gdb for PyStringObject instances containing bytes in the range 0x80-0xff
    • handle the case of loops in the object reference graph. In previous versions, the gdb prettyprinter would get stuck in infinite recursion following the pointers. The new version keeps track of the set of all pointers we've visited so far, stopping the traversal at object's we've already reached (analagous to Py_ReprEnter and Py_ReprLeave).
    • unit tests for all of the above (full test suite passes on my machine thus: "Ran 35 tests in 13.547s"; this is a Fedora 12 x86_64 box with gdb-7.0.1-33.fc12)

    @davidmalcolm
    Copy link
    Member Author

    (adding diff from v3 to v4, for ease of review)

    @davidmalcolm
    Copy link
    Member Author

    I'm attaching a new version of the patch (v5), against svn trunk (r79517)

    I've been testing these hooks by using "gdb attach" to attach to real-world python programs.

    When doing this with earlier versions of the hooks, gdb would pause for many seconds when printing some values (e.g. during a backtrace). The issue was that "proxyval" scrapes the entire graph of python objects from the inferior process into gdb, and then prints it. As well as being slow, this can be many pages of textual output.

    To counter this, I've rewritten the pretty-printers to use a stream-based approach. The various PyObjectPtr subclasses have a "write_repr" method that writes the represenation to a file-like object whilst they recurse. I supply a file-like object "TruncatedStringIO", which raises an exception after a limit is reached: by default 1K of string data. (some simple subclasses simply reuse repr(self.proxyval()) for this, but objects that can follow cross-references have their own write_repr implementation).

    I've also rewritten frame handling: I eliminated the FrameInfo class, moving its functionality to the PyFrameObjectPtr class, and introduced a new "Frame" class to wrap gdb.Frame. This allowed a big simplification of the extension commands, and fixed various bugs (involving inlining and optimization)

    Within the py-up/py-down and py-bt commands, the code now prefixes Python frame printing with C-level frame indexes, to better tie together the C-level and Python-level views of the stack.

    I've added a couple of new commands:

    • "py-print" - takes a string argument. It attempts to look up the Python variable with that name, and print its value (it searches first in the locals of the current frame, then globals, then builtins).
    • "py-locals" - prints Python local variables; an analog of gdb's "info locals"

    Other changes since v4:

    • Add a ">" marker to the output of "py-list", indicating the current location
    • Add pretty-printer for PyCFunctionObject ("built-in function" and "built-in method")
    • Add selftests for the above
    • Make frame printing fetch information on locals on demand, rather than within FrameInfo's __init__ method (the latter is completely gone)
    • Introduce PyObjectPtr.pyop_field method, simplifying the code
    • Change the set/frozenset selftests to remove a reliance on ordering within the representation of the members
    • Try to be more robust for the case where PyEval_EvalFrameEx's "f" parameter is optimized out (see https://bugzilla.redhat.com/show_bug.cgi?id=556975 )
    • Remove the FIXME in PyLongObjectPtr: I've tested the code on builds with both sizeof(digit) == 2 and == 4
    • More selftests for unicode printing

    All tests pass on my box (Fedora 12 x86_64 with gdb-7.0.1-33.fc12.x86_64): "Ran 45 tests in 17.439s"

    @davidmalcolm
    Copy link
    Member Author

    Adding diff from v4 to v5, for ease of review.

    For my reference, md5sum of v5's hooks:
    d3e039bb1279e71e847cc7ade10d3272 python-gdb.py

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Apr 1, 2010

    Thanks for the patch. I have committed it as r79548, with two modifications:

    • I added a future import for with_statement, as many installations will still use Python 2.5
    • I commented out many of the tests, as they were failing for me. I'll report those as several issues.

    Please understand that this commit went to the trunk, which will be 2.7. After 2.7b1, no new features are acceptable, so you can only perform bug fixes on the code that is now committed. Please create new issues for any such bug fixes.

    Once the tests all pass, I'll merge this into 3.2.

    @loewis loewis mannequin closed this as completed Apr 1, 2010
    @mdickinson
    Copy link
    Member

    A nitpick: on OS X, the gdb script ends up being called:

    python.exe-gdb.py

    Is this intentional? If it is, then I'll add this filename to the svn:ignore property. (And also to make distclean, I guess. Is python-gdb.py currently deleted by a 'make distclean'?)

    @davidmalcolm
    Copy link
    Member Author

    A nitpick: on OS X, the gdb script ends up being called:

    python.exe-gdb.py

    Is this intentional? If it is, then I'll add this filename to the
    svn:ignore property. (And also to make distclean, I guess. Is python-
    gdb.py currently deleted by a 'make distclean'?)

    That was unexpected, but re-reading Makefile.pre.in I see where it's coming from. Please do add it to svn:ignore.

    Looks like I forgot to add the removal of the file to the "distclean" target. Sorry about that. Presumably adding:
    -rm -f python*-gdb.py
    should do the trick (as I understand it, if the glob fails to match, it will happily fail to "rm" the non-existant file named "python*-gdb.py").

    Another nit I spotted: the buildbot is "unexpectedly" skipping test_gdb on Win32 (with "gdb not found in path"). Looking at _expectations in Lib/test/regrtest.py it strikes me that test_gdb is likely to be skipped on every configuration other than on "linux2") - should I wire that in with a decorator within test_gdb.py?

    To my knowledge, OS X doesn't ship with gdb 7 - though presumably someone could build their own copy, linking with the system Python, and if so, the python.exe-gdb.py file would then be of use in debugging the freshly-built ./python.exe

    @mdickinson
    Copy link
    Member

    Added python.exe-gdb.py to svn:ignore in r79616.

    @mdickinson
    Copy link
    Member

    To my knowledge, OS X doesn't ship with gdb 7

    That sounds right. On my OS X 10.6.3 machine, the system gdb is:

    Mark-Dickinsons-MacBook-Pro:py3k dickinsm$ gdb --version
    GNU gdb 6.3.50-20050815 (Apple version gdb-1346) (Fri Sep 18 20:40:51 UTC 2009)

    I have the impression that few people have got gdb 7 working on OS X yet, but I might be wrong. Certainly macports doesn't seem to offer it yet. I expect that'll change, though.

    [About make distclean]

    Presumably adding:
    -rm -f python*-gdb.py
    should do the trick

    Yes, that seems to work. Thanks! Added in r79617.

    @benjaminp
    Copy link
    Contributor

    Could I request that the python-gdb.py script not be put in the top level directory? It's a little annoying to have another executable script starting with "python" when you're trying to test the "python" executable.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Apr 3, 2010

    That is tricky to do. gdb will consider it automatically only if it is called <exename>-gdb.py.

    I agree that there should be a better solution, though. David, is it possible to somehow hook this into .gdbinit, with an arbitrary path?

    Benjamin, would a .gdbinit in the top-level build directory still be annoying?

    @jyasskin
    Copy link
    Mannequin

    jyasskin mannequin commented Apr 3, 2010

    http://sourceware.org/gdb/current/onlinedocs/gdb/Auto_002dloading.html says "If this file exists and is readable, gdb will evaluate it as a Python script." So maybe it doesn't need to be executable?

    @benjaminp
    Copy link
    Contributor

    2010/4/3 Martin v. Löwis <report@bugs.python.org>:

    Benjamin, would a .gdbinit in the top-level build directory still be annoying?

    That would be fine with me.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Apr 3, 2010

    I have now changed Makefile.pre.in to not install python-gdb.py as a script anymore; this seems to work fine. People will still need to remove there existing python-gdb.py (or make clean) to see this change.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants