From: Bruno Larsen <blarsen@redhat.com>
To: Matheus Branco Borella <dark.ryu.550@gmail.com>,
gdb-patches@sourceware.org
Subject: Re: [PATCH] Add __repr__() implementation to a few Python types
Date: Wed, 18 Jan 2023 18:05:46 +0100 [thread overview]
Message-ID: <995ae7c2-3804-f2fd-e070-d83407b17400@redhat.com> (raw)
In-Reply-To: <20230111003510.620-1-dark.ryu.550@gmail.com>
On 11/01/2023 01:35, Matheus Branco Borella via Gdb-patches wrote:
> Only a few types in the Python API currently have __repr__() implementations.
> This patch adds a few more of them. specifically: it adds __repr__()
> implementations to gdb.Symbol, gdb.Architecture, gdb.Block, gdb.Breakpoint,
> and gdb.Type.
>
> This makes it easier to play around the GDB Python API in the Python interpreter
> session invoked with the 'pi' command in GDB, giving more easily accessible tipe
> information to users.
>
> An example of how this would look like:
> ```
> (gdb) pi
>>> gdb.lookup_type("char")
> <gdb.Type code=TYPE_CODE_INT name=char>
>>> gdb.lookup_global_symbol("main")
> <gdb.Symbol print_name=main>
> ```
>
> One thing to note about this patch is that it makes use of u8 string literals,
> so as to make sure we meet python's expectations of strings passed to it using
> PyUnicode_FromFormat being encoded in utf8. This should remove the chance of
> odd compilation environments spitting out strings Python would consider invalid
> for the function we're calling.
Thanks for working on this. I am not very familiar with the python side
of GDB code, so I'll mostly comment on style. Nits inlined.
Also, while applying I get this:
Applying: Add __repr__() implementation to a few Python types
.git/rebase-apply/patch:267: trailing whitespace.
I have tested and see no regressions on my x86 machine.
Tested-By: Bruno Larsen <blarsen@redhat.com>
> ---
> gdb/python/py-arch.c | 18 +++++++-
> gdb/python/py-block.c | 30 ++++++++++++-
> gdb/python/py-breakpoint.c | 49 +++++++++++++++++++++-
> gdb/python/py-symbol.c | 16 ++++++-
> gdb/python/py-type.c | 31 +++++++++++++-
> gdb/testsuite/gdb.python/py-arch.exp | 4 ++
> gdb/testsuite/gdb.python/py-block.exp | 4 +-
> gdb/testsuite/gdb.python/py-breakpoint.exp | 26 +++++++-----
> gdb/testsuite/gdb.python/py-symbol.exp | 1 +
> gdb/testsuite/gdb.python/py-type.exp | 4 ++
> 10 files changed, 165 insertions(+), 18 deletions(-)
>
> diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
> index cf0978560f9..5384a0d0d0c 100644
> --- a/gdb/python/py-arch.c
> +++ b/gdb/python/py-arch.c
> @@ -319,6 +319,22 @@ archpy_integer_type (PyObject *self, PyObject *args, PyObject *kw)
> return type_to_type_object (type);
> }
>
> +/* __repr__ implementation for gdb.Architecture. */
> +
> +static PyObject *
> +archpy_repr (PyObject *self)
> +{
> + const auto gdbarch = arch_object_to_gdbarch (self);
> + if (gdbarch == nullptr)
> + return PyUnicode_FromFormat
> + ("<gdb.Architecture (invalid)>");
> +
> + return PyUnicode_FromFormat
> + ("<gdb.Architecture arch_name=%s printable_name=%s>",
> + gdbarch_bfd_arch_info (gdbarch)->arch_name,
> + gdbarch_bfd_arch_info (gdbarch)->printable_name);
> +}
> +
> /* Implementation of gdb.architecture_names(). Return a list of all the
> BFD architecture names that GDB understands. */
>
> @@ -391,7 +407,7 @@ PyTypeObject arch_object_type = {
> 0, /* tp_getattr */
> 0, /* tp_setattr */
> 0, /* tp_compare */
> - 0, /* tp_repr */
> + archpy_repr, /* tp_repr */
> 0, /* tp_as_number */
> 0, /* tp_as_sequence */
> 0, /* tp_as_mapping */
> diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
> index b9aea3aca69..1b8433d41e7 100644
> --- a/gdb/python/py-block.c
> +++ b/gdb/python/py-block.c
> @@ -23,6 +23,7 @@
> #include "symtab.h"
> #include "python-internal.h"
> #include "objfiles.h"
> +#include <sstream>
>
> struct block_object {
> PyObject_HEAD
> @@ -424,6 +425,33 @@ blpy_iter_is_valid (PyObject *self, PyObject *args)
> Py_RETURN_TRUE;
> }
>
> +/* __repr__ implementation for gdb.Block. */
> +
> +static PyObject *
> +blpy_repr (PyObject *self)
> +{
> + const auto block = block_object_to_block (self);
> + if (block == nullptr)
> + return PyUnicode_FromFormat("<gdb.Block (invalid)>");
missing space before (
> +
> + const auto name = block->function () ? block->function ()->print_name () : "<anonymous>";
> +
> + block_iterator iter;
> + block_iterator_first (block, &iter);
> +
> + std::stringstream ss;
> + const struct symbol *symbol;
> + while ((symbol = block_iterator_next (&iter)) != nullptr)
> + {
> + ss << std::endl;
> + ss << symbol->print_name () << ",";
> + }
> + if(!ss.str ().empty ())
> + ss << std::endl;
> +
> + return PyUnicode_FromFormat("<gdb.Block %s {%s}>", name, ss.str ().c_str ());
and here
> +}
> +
> int
> gdbpy_initialize_blocks (void)
> {
> @@ -486,7 +514,7 @@ PyTypeObject block_object_type = {
> 0, /*tp_getattr*/
> 0, /*tp_setattr*/
> 0, /*tp_compare*/
> - 0, /*tp_repr*/
> + blpy_repr, /*tp_repr*/
> 0, /*tp_as_number*/
> 0, /*tp_as_sequence*/
> &block_object_as_mapping, /*tp_as_mapping*/
> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index de7b9f4266b..ce307f7be21 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -33,6 +33,7 @@
> #include "location.h"
> #include "py-event.h"
> #include "linespec.h"
> +#include <sstream>
>
> extern PyTypeObject breakpoint_location_object_type
> CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_location_object");
> @@ -967,6 +968,23 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
> return 0;
> }
>
> +/* __repr__ implementation for gdb.Breakpoint. */
> +
> +static PyObject *
> +bppy_repr(PyObject *self)
here too
> +{
> + const auto bp = (struct gdbpy_breakpoint_object*) self;
> + if (bp->bp == nullptr)
> + return PyUnicode_FromFormat ("<gdb.Breakpoint (invalid)>");
> +
> + return PyUnicode_FromFormat
> + ("<gdb.Breakpoint number=%d thread=%d hits=%d enable_count=%d>",
> + bp->bp->number,
> + bp->bp->thread,
> + bp->bp->hit_count,
> + bp->bp->enable_count);
I'm not sure if this is a "me" thing, but I try to keep line count as
low as possible, and only break when reaching the 72 character soft limit.
> +}
> +
> /* Append to LIST the breakpoint Python object associated to B.
>
> Return true on success. Return false on failure, with the Python error
> @@ -1389,7 +1407,7 @@ PyTypeObject breakpoint_object_type =
> 0, /*tp_getattr*/
> 0, /*tp_setattr*/
> 0, /*tp_compare*/
> - 0, /*tp_repr*/
> + bppy_repr, /*tp_repr*/
> 0, /*tp_as_number*/
> 0, /*tp_as_sequence*/
> 0, /*tp_as_mapping*/
> @@ -1604,6 +1622,33 @@ bplocpy_dealloc (PyObject *py_self)
> Py_TYPE (py_self)->tp_free (py_self);
> }
>
> +/* __repr__ implementation for gdb.BreakpointLocation. */
> +
> +static PyObject *
> +bplocpy_repr (PyObject *py_self)
> +{
> + const auto self = (gdbpy_breakpoint_location_object *) py_self;
> + if (self->owner == nullptr || self->owner->bp == nullptr || self->owner->bp != self->bp_loc->owner)
> + return PyUnicode_FromFormat ("<gdb.BreakpointLocation (invalid)>");
> +
> + const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
> +
> + std::stringstream ss;
> + ss << std::endl << enabled << std::endl;
> + ss << "requested_address=0x" << std::hex << self->bp_loc->requested_address << " ";
> + ss << "address=0x" << self->bp_loc->address << " " << std::dec << std::endl;
> + if (self->bp_loc->symtab != nullptr)
> + {
> + ss << self->bp_loc->symtab->filename << ":" << self->bp_loc->line_number << " " << std::endl;
> + }
> +
> + const auto fn_name = self->bp_loc->function_name.get ();
> + if (fn_name != nullptr)
> + ss << "in " << fn_name << " " << std::endl;
> +
> + return PyUnicode_FromFormat ("<gdb.BreakpointLocation %s>", ss.str ().c_str ());
> +}
> +
> /* Attribute get/set Python definitions. */
>
> static gdb_PyGetSetDef bp_location_object_getset[] = {
> @@ -1635,7 +1680,7 @@ PyTypeObject breakpoint_location_object_type =
> 0, /*tp_getattr*/
> 0, /*tp_setattr*/
> 0, /*tp_compare*/
> - 0, /*tp_repr*/
> + bplocpy_repr, /*tp_repr*/
> 0, /*tp_as_number*/
> 0, /*tp_as_sequence*/
> 0, /*tp_as_mapping*/
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index 93c86964f3e..5a8149bbe66 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -375,6 +375,20 @@ sympy_dealloc (PyObject *obj)
> Py_TYPE (obj)->tp_free (obj);
> }
>
> +/* __repr__ implementation for gdb.Symbol. */
> +
> +static PyObject *
> +sympy_repr (PyObject *self)
> +{
> + const auto symbol = symbol_object_to_symbol (self);
> + if (symbol == nullptr)
> + return PyUnicode_FromFormat ("<gdb.Symbol (invalid)>");
> +
> + return PyUnicode_FromFormat
> + ("<gdb.Symbol print_name=%s>",
> + symbol->print_name ());
> +}
> +
> /* Implementation of
> gdb.lookup_symbol (name [, block] [, domain]) -> (symbol, is_field_of_this)
> A tuple with 2 elements is always returned. The first is the symbol
> @@ -732,7 +746,7 @@ PyTypeObject symbol_object_type = {
> 0, /*tp_getattr*/
> 0, /*tp_setattr*/
> 0, /*tp_compare*/
> - 0, /*tp_repr*/
> + sympy_repr, /*tp_repr*/
> 0, /*tp_as_number*/
> 0, /*tp_as_sequence*/
> 0, /*tp_as_mapping*/
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index 928efacfe8a..abe127eca76 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -442,6 +442,7 @@ typy_is_signed (PyObject *self, void *closure)
> Py_RETURN_TRUE;
> }
>
> +
spurious white line here
> /* Return the type, stripped of typedefs. */
> static PyObject *
> typy_strip_typedefs (PyObject *self, PyObject *args)
> @@ -1026,6 +1027,34 @@ typy_template_argument (PyObject *self, PyObject *args)
> return value_to_value_object (val);
> }
>
> +/* __repr__ implementation for gdb.Type. */
> +
> +static PyObject *
> +typy_repr (PyObject *self)
> +{
> + const auto type = type_object_to_type (self);
> + if (type == nullptr)
> + return PyUnicode_FromFormat ("<gdb.Type (invalid)>");
> +
> + const char *code = pyty_codes[type->code ()].name;
> + string_file type_name;
> + try
> + {
> + current_language->print_type (type, "",
> + &type_name, -1, 0,
> + &type_print_raw_options);
> + }
> + catch (const gdb_exception &except)
> + {
> + GDB_PY_HANDLE_EXCEPTION (except);
> + }
> + auto py_typename = PyUnicode_Decode
> + (type_name.c_str (), type_name.size (),
> + host_charset (), NULL);
> +
> + return PyUnicode_FromFormat ("<gdb.Type code=%s name=%U>", code, py_typename);
> +}
> +
> static PyObject *
> typy_str (PyObject *self)
> {
> @@ -1612,7 +1641,7 @@ PyTypeObject type_object_type =
> 0, /*tp_getattr*/
> 0, /*tp_setattr*/
> 0, /*tp_compare*/
> - 0, /*tp_repr*/
> + typy_repr, /*tp_repr*/
> &type_object_as_number, /*tp_as_number*/
> 0, /*tp_as_sequence*/
> &typy_mapping, /*tp_as_mapping*/
> diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
> index 1fbbc47c872..a60b4a25cbb 100644
> --- a/gdb/testsuite/gdb.python/py-arch.exp
> +++ b/gdb/testsuite/gdb.python/py-arch.exp
> @@ -29,6 +29,8 @@ if ![runto_main] {
> # Test python/15461. Invalid architectures should not trigger an
> # internal GDB assert.
> gdb_py_test_silent_cmd "python empty = gdb.Architecture()" "get empty arch" 0
> +gdb_test "python print(repr (empty))" "<gdb\\.Architecture \\(invalid\\)>" \
> + "Test empty achitecture __repr__ does not trigger an assert"
> gdb_test "python print(empty.name())" ".*Architecture is invalid.*" \
> "Test empty architecture.name does not trigger an assert"
> gdb_test "python print(empty.disassemble())" ".*Architecture is invalid.*" \
> @@ -46,6 +48,8 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
> gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(gdb.Value(pc))" \
> "disassemble no end no count" 0
>
> +gdb_test "python print (repr (arch))" "<gdb.Architecture arch_name=.* printable_name=.*>" "test __repr__ for architecture"
> +
> gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
> gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
> gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
> diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
> index 0a88aec56a0..5e3d1c72d5e 100644
> --- a/gdb/testsuite/gdb.python/py-block.exp
> +++ b/gdb/testsuite/gdb.python/py-block.exp
> @@ -39,7 +39,7 @@ gdb_continue_to_breakpoint "Block break here."
> gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
> gdb_py_test_silent_cmd "python block = frame.block()" \
> "Get block, initial innermost block" 0
> -gdb_test "python print (block)" "<gdb.Block object at $hex>" "check block not None"
> +gdb_test "python print (block)" "<gdb.Block .* \{.*\}>" "check block not None"
> gdb_test "python print (block.function)" "None" "first anonymous block"
> gdb_test "python print (block.start)" "${decimal}" "check start not None"
> gdb_test "python print (block.end)" "${decimal}" "check end not None"
> @@ -73,7 +73,7 @@ gdb_test "python print (block.function)" "block_func" \
> gdb_test "up" ".*"
> gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
> gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
> -gdb_test "python print (block)" "<gdb.Block object at $hex>" \
> +gdb_test "python print (repr (block))" "<gdb.Block .* \{.*\}>" \
> "Check Frame 2's block not None"
> gdb_test "python print (block.function)" "main" "main block"
>
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index e36e87dc291..0c904a12c90 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -50,11 +50,14 @@ proc_with_prefix test_bkpt_basic { } {
> return 0
> }
>
> + set num_exp "-?\[0-9\]+"
> + set repr_pattern "<gdb.Breakpoint number=$num_exp thread=$num_exp hits=$num_exp enable_count=$num_exp>"
> +
> # Now there should be one breakpoint: main.
> gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" \
> "Get Breakpoint List" 0
> - gdb_test "python print (blist\[0\])" \
> - "<gdb.Breakpoint object at $hex>" "Check obj exists @main"
> + gdb_test "python print (repr (blist\[0\]))" \
> + "$repr_pattern" "Check obj exists @main"
> gdb_test "python print (blist\[0\].location)" \
> "main." "Check breakpoint location @main"
> gdb_test "python print (blist\[0\].pending)" "False" \
> @@ -71,12 +74,12 @@ proc_with_prefix test_bkpt_basic { } {
> "Get Breakpoint List" 0
> gdb_test "python print (len(blist))" \
> "2" "Check for two breakpoints"
> - gdb_test "python print (blist\[0\])" \
> - "<gdb.Breakpoint object at $hex>" "Check obj exists @main 2"
> + gdb_test "python print (repr (blist\[0\]))" \
> + "$repr_pattern" "Check obj exists @main 2"
> gdb_test "python print (blist\[0\].location)" \
> "main." "Check breakpoint location @main 2"
> - gdb_test "python print (blist\[1\])" \
> - "<gdb.Breakpoint object at $hex>" "Check obj exists @mult_line"
> + gdb_test "python print (repr (blist\[1\]))" \
> + "$repr_pattern" "Check obj exists @mult_line"
>
> gdb_test "python print (blist\[1\].location)" \
> "py-breakpoint\.c:${mult_line}*" \
> @@ -224,14 +227,17 @@ proc_with_prefix test_bkpt_invisible { } {
> return 0
> }
>
> + set num_exp "-?\[0-9\]+"
> + set repr_pattern "<gdb.Breakpoint number=$num_exp thread=$num_exp hits=$num_exp enable_count=$num_exp>"
> +
> delete_breakpoints
> set ibp_location [gdb_get_line_number "Break at multiply."]
> gdb_py_test_silent_cmd "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" \
> "Set invisible breakpoint" 0
> gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
> "Get Breakpoint List" 0
> - gdb_test "python print (ilist\[0\])" \
> - "<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 1"
> + gdb_test "python print (repr (ilist\[0\]))" \
> + "$repr_pattern" "Check invisible bp obj exists 1"
> gdb_test "python print (ilist\[0\].location)" \
> "py-breakpoint\.c:$ibp_location*" "Check breakpoint location 1"
> gdb_test "python print (ilist\[0\].visible)" \
> @@ -243,8 +249,8 @@ proc_with_prefix test_bkpt_invisible { } {
> "Set invisible breakpoint" 0
> gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
> "Get Breakpoint List" 0
> - gdb_test "python print (ilist\[0\])" \
> - "<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 2"
> + gdb_test "python print (repr (ilist\[0\]))" \
> + "$repr_pattern" "Check invisible bp obj exists 2"
> gdb_test "python print (ilist\[0\].location)" \
> "py-breakpoint\.c:$ibp_location*" "Check breakpoint location 2"
> gdb_test "python print (ilist\[0\].visible)" \
> diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> index ad06b07c2c6..e0baed9b6d4 100644
> --- a/gdb/testsuite/gdb.python/py-symbol.exp
> +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> @@ -44,6 +44,7 @@ clean_restart ${binfile}
> # point where we don't have a current frame, and we don't want to
> # require one.
> gdb_py_test_silent_cmd "python main_func = gdb.lookup_global_symbol(\"main\")" "Lookup main" 1
> +gdb_test "python print (repr (main_func))" "<gdb.Symbol print_name=.*>" "test main_func.__repr__"
> gdb_test "python print (main_func.is_function)" "True" "test main_func.is_function"
> gdb_test "python print (gdb.lookup_global_symbol(\"junk\"))" "None" "test lookup_global_symbol(\"junk\")"
>
> diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
> index 594c9749d8e..95cdfa54a6e 100644
> --- a/gdb/testsuite/gdb.python/py-type.exp
> +++ b/gdb/testsuite/gdb.python/py-type.exp
> @@ -393,3 +393,7 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
> test_type_equality
> }
> }
> +
> +# Test __repr__()
> +gdb_test "python print (repr (gdb.lookup_type ('char')))" \
> + "<gdb.Type code=TYPE_CODE_INT name=char>" "test __repr__()"
--
Cheers,
Bruno
next prev parent reply other threads:[~2023-01-18 17:05 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <3Cc9d2dc49-45c7-d6b9-c567-4ec78dd870a0>
2023-01-11 0:35 ` Matheus Branco Borella
2023-01-18 17:05 ` Bruno Larsen [this message]
2023-01-18 18:02 ` Andrew Burgess
2023-01-19 8:23 ` Bruno Larsen
2023-01-20 1:43 ` Matheus Branco Borella
2023-01-20 16:45 ` Andrew Burgess
2023-01-24 14:45 ` Andrew Burgess
2023-05-18 3:33 ` Matheus Branco Borella
2023-05-19 21:27 ` [PATCHv3 0/2] " Andrew Burgess
2023-05-19 21:27 ` [PATCHv3 1/2] gdb: have mdict_size always return a symbol count Andrew Burgess
2023-05-19 21:27 ` [PATCHv3 2/2] gdb: add __repr__() implementation to a few Python types Andrew Burgess
2023-06-07 17:05 ` [PATCHv3 0/2] Add " Matheus Branco Borella
2023-06-08 18:46 ` Andrew Burgess
2023-06-09 12:33 ` Andrew Burgess
2023-07-04 11:09 ` Andrew Burgess
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=995ae7c2-3804-f2fd-e070-d83407b17400@redhat.com \
--to=blarsen@redhat.com \
--cc=dark.ryu.550@gmail.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).