From 764af878259768bb70c65bdf3f3285c2d6409bbd Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Wed, 12 Jun 2024 18:58:49 +0200 Subject: [gdb/python] Add typesafe wrapper around PyObject_CallMethod In gdb/python/py-tui.c we have code like this: ... gdbpy_ref<> result (PyObject_CallMethod (m_window.get(), "hscroll", "i", num_to_scroll, nullptr)); ... The nullptr is superfluous, the format string already indicates that there's only one method argument. OTOH, passing no method args does use a nullptr: ... gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "render", nullptr)); ... Furthermore, choosing the right format string chars can be tricky. Add a typesafe wrapper around PyObject_CallMethod that hides these details, such that we can use the more intuitive: ... gdbpy_ref<> result (gdbpy_call_method (m_window.get(), "hscroll", num_to_scroll)); ... and: ... gdbpy_ref<> result (gdbpy_call_method (m_window.get (), "render")); ... Tested on x86_64-linux. Co-Authored-By: Tom de Vries Approved-By: Tom Tromey --- gdb/python/py-breakpoint.c | 2 +- gdb/python/py-disasm.c | 5 ++- gdb/python/py-finishbreakpoint.c | 3 +- gdb/python/py-framefilter.c | 17 +++++----- gdb/python/py-tui.c | 19 +++++------ gdb/python/python-internal.h | 70 +++++++++++++++++++++++++++++++++------- 6 files changed, 79 insertions(+), 37 deletions(-) diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c index da74d69..1b8c717 100644 --- a/gdb/python/py-breakpoint.c +++ b/gdb/python/py-breakpoint.c @@ -1174,7 +1174,7 @@ gdbpy_breakpoint_cond_says_stop (const struct extension_language_defn *extlang, if (PyObject_HasAttrString (py_bp, stop_func)) { - gdbpy_ref<> result (PyObject_CallMethod (py_bp, stop_func, NULL)); + gdbpy_ref<> result (gdbpy_call_method (py_bp, stop_func)); stop = 1; if (result != NULL) diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c index 9337514..5206c76 100644 --- a/gdb/python/py-disasm.c +++ b/gdb/python/py-disasm.c @@ -855,9 +855,8 @@ gdbpy_disassembler::read_memory_func (bfd_vma memaddr, gdb_byte *buff, /* Now call the DisassembleInfo.read_memory method. This might have been overridden by the user. */ - gdbpy_ref<> result_obj (PyObject_CallMethod ((PyObject *) obj, - "read_memory", - "I" GDB_PY_LL_ARG, len, offset)); + gdbpy_ref<> result_obj (gdbpy_call_method ((PyObject *) obj, "read_memory", + len, offset)); /* Handle any exceptions. */ if (result_obj == nullptr) diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c index c74a247..1b620e6 100644 --- a/gdb/python/py-finishbreakpoint.c +++ b/gdb/python/py-finishbreakpoint.c @@ -344,8 +344,7 @@ bpfinishpy_out_of_scope (struct finish_breakpoint_object *bpfinish_obj) if (bpfinish_obj->py_bp.bp->enable_state == bp_enabled && PyObject_HasAttrString (py_obj, outofscope_func)) { - gdbpy_ref<> meth_result (PyObject_CallMethod (py_obj, outofscope_func, - NULL)); + gdbpy_ref<> meth_result (gdbpy_call_method (py_obj, outofscope_func)); if (meth_result == NULL) gdbpy_print_stack (); } diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c index 0cd1597..4ae583b 100644 --- a/gdb/python/py-framefilter.c +++ b/gdb/python/py-framefilter.c @@ -59,7 +59,7 @@ extract_sym (PyObject *obj, gdb::unique_xmalloc_ptr *name, struct symbol **sym, const struct block **sym_block, const struct language_defn **language) { - gdbpy_ref<> result (PyObject_CallMethod (obj, "symbol", NULL)); + gdbpy_ref<> result (gdbpy_call_method (obj, "symbol")); if (result == NULL) return EXT_LANG_BT_ERROR; @@ -130,7 +130,7 @@ extract_value (PyObject *obj, struct value **value) { if (PyObject_HasAttrString (obj, "value")) { - gdbpy_ref<> vresult (PyObject_CallMethod (obj, "value", NULL)); + gdbpy_ref<> vresult (gdbpy_call_method (obj, "value")); if (vresult == NULL) return EXT_LANG_BT_ERROR; @@ -264,7 +264,7 @@ get_py_iter_from_func (PyObject *filter, const char *func) { if (PyObject_HasAttrString (filter, func)) { - gdbpy_ref<> result (PyObject_CallMethod (filter, func, NULL)); + gdbpy_ref<> result (gdbpy_call_method (filter, func)); if (result != NULL) { @@ -790,8 +790,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags, /* Get the underlying frame. This is needed to determine GDB architecture, and also, in the cases of frame variables/arguments to read them if they returned filter object requires us to do so. */ - gdbpy_ref<> py_inf_frame (PyObject_CallMethod (filter, "inferior_frame", - NULL)); + gdbpy_ref<> py_inf_frame (gdbpy_call_method (filter, "inferior_frame")); if (py_inf_frame == NULL) return EXT_LANG_BT_ERROR; @@ -831,7 +830,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags, address printing. */ if (PyObject_HasAttrString (filter, "address")) { - gdbpy_ref<> paddr (PyObject_CallMethod (filter, "address", NULL)); + gdbpy_ref<> paddr (gdbpy_call_method (filter, "address")); if (paddr == NULL) return EXT_LANG_BT_ERROR; @@ -906,7 +905,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags, /* Print frame function name. */ if (PyObject_HasAttrString (filter, "function")) { - gdbpy_ref<> py_func (PyObject_CallMethod (filter, "function", NULL)); + gdbpy_ref<> py_func (gdbpy_call_method (filter, "function")); const char *function = NULL; if (py_func == NULL) @@ -970,7 +969,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags, if (PyObject_HasAttrString (filter, "filename")) { - gdbpy_ref<> py_fn (PyObject_CallMethod (filter, "filename", NULL)); + gdbpy_ref<> py_fn (gdbpy_call_method (filter, "filename")); if (py_fn == NULL) return EXT_LANG_BT_ERROR; @@ -994,7 +993,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags, if (PyObject_HasAttrString (filter, "line")) { - gdbpy_ref<> py_line (PyObject_CallMethod (filter, "line", NULL)); + gdbpy_ref<> py_line (gdbpy_call_method (filter, "line")); int line; if (py_line == NULL) diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c index 5dcec4b..9df86df 100644 --- a/gdb/python/py-tui.c +++ b/gdb/python/py-tui.c @@ -164,8 +164,7 @@ tui_py_window::~tui_py_window () if (m_window != nullptr && PyObject_HasAttrString (m_window.get (), "close")) { - gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "close", - nullptr)); + gdbpy_ref<> result (gdbpy_call_method (m_window.get (), "close")); if (result == nullptr) gdbpy_print_stack (); } @@ -198,8 +197,7 @@ tui_py_window::rerender () if (PyObject_HasAttrString (m_window.get (), "render")) { - gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "render", - nullptr)); + gdbpy_ref<> result (gdbpy_call_method (m_window.get (), "render")); if (result == nullptr) gdbpy_print_stack (); } @@ -212,8 +210,8 @@ tui_py_window::do_scroll_horizontal (int num_to_scroll) if (PyObject_HasAttrString (m_window.get (), "hscroll")) { - gdbpy_ref<> result (PyObject_CallMethod (m_window.get(), "hscroll", - "i", num_to_scroll, nullptr)); + gdbpy_ref<> result (gdbpy_call_method (m_window.get (), "hscroll", + num_to_scroll)); if (result == nullptr) gdbpy_print_stack (); } @@ -226,8 +224,8 @@ tui_py_window::do_scroll_vertical (int num_to_scroll) if (PyObject_HasAttrString (m_window.get (), "vscroll")) { - gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "vscroll", - "i", num_to_scroll, nullptr)); + gdbpy_ref<> result (gdbpy_call_method (m_window.get (), "vscroll", + num_to_scroll)); if (result == nullptr) gdbpy_print_stack (); } @@ -248,9 +246,8 @@ tui_py_window::click (int mouse_x, int mouse_y, int mouse_button) if (PyObject_HasAttrString (m_window.get (), "click")) { - gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "click", - "iii", mouse_x, mouse_y, - mouse_button)); + gdbpy_ref<> result (gdbpy_call_method (m_window.get (), "click", + mouse_x, mouse_y, mouse_button)); if (result == nullptr) gdbpy_print_stack (); } diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h index d07f239..1ebb531 100644 --- a/gdb/python/python-internal.h +++ b/gdb/python/python-internal.h @@ -145,26 +145,74 @@ typedef long Py_hash_t; #define PyMem_RawMalloc PyMem_Malloc #endif -/* PyObject_CallMethod's 'method' and 'format' parameters were missing - the 'const' qualifier before Python 3.4. Hence, we wrap the - function in our own version to avoid errors with string literals. - Note, this is a variadic template because PyObject_CallMethod is a - varargs function and Python doesn't have a "PyObject_VaCallMethod" - variant taking a va_list that we could defer to instead. */ +/* A template variable holding the format character (as for + Py_BuildValue) for a given type. */ +template +constexpr char gdbpy_method_format; + +template<> +constexpr char gdbpy_method_format = GDB_PY_LL_ARG[0]; + +template<> +constexpr char gdbpy_method_format = GDB_PY_LLU_ARG[0]; + +template<> +constexpr char gdbpy_method_format = 'i'; + +template<> +constexpr char gdbpy_method_format = 'I'; + +/* A helper function to compute the PyObject_CallMethod / + Py_BuildValue format given the argument types. */ template +constexpr std::array +gdbpy_make_fmt () +{ + return { gdbpy_method_format..., '\0' }; +} + +/* Typesafe wrapper around PyObject_CallMethod. + + This variant accepts no arguments. */ + static inline PyObject * -gdb_PyObject_CallMethod (PyObject *o, const char *method, const char *format, - Args... args) /* ARI: editCase function */ +gdbpy_call_method (PyObject *o, const char *method) { + /* PyObject_CallMethod's 'method' and 'format' parameters were missing the + 'const' qualifier before Python 3.4. */ return PyObject_CallMethod (o, const_cast (method), - const_cast (format), - args...); + nullptr); } +/* Typesafe wrapper around PyObject_CallMethod. + + This variant accepts any number of arguments and automatically + computes the format string, ensuring that format/argument + mismatches are impossible. */ + +template +static inline PyObject * +gdbpy_call_method (PyObject *o, const char *method, + Arg arg, Args... args) +{ + constexpr const auto fmt = gdbpy_make_fmt (); + + /* PyObject_CallMethod's 'method' and 'format' parameters were missing the + 'const' qualifier before Python 3.4. */ + return PyObject_CallMethod (o, + const_cast (method), + const_cast (fmt.data ()), + arg, args...); +} + +/* Poison PyObject_CallMethod. The typesafe wrapper gdbpy_call_method should be + used instead. */ #undef PyObject_CallMethod -#define PyObject_CallMethod gdb_PyObject_CallMethod +template +PyObject * +PyObject_CallMethod (Args...); /* The 'name' parameter of PyErr_NewException was missing the 'const' qualifier in Python <= 3.4. Hence, we wrap it in a function to -- cgit v1.1