* [PATCH v2] [gdb/python] Add non-variadic versions of PyObject_CallMethod
@ 2024-06-11 8:55 Tom de Vries
2024-06-11 19:55 ` Tom Tromey
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2024-06-11 8:55 UTC (permalink / raw)
To: gdb-patches
I noticed 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 argument.
OTOH, passing no 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 non-variadic versions of PyObject_CallMethod that hide these details, such
that we can use the more intuitive:
...
gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get(), "hscroll",
num_to_scroll));
...
and:
...
gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "render"));
...
Tested on x86_64-linux.
---
gdb/python/py-breakpoint.c | 2 +-
gdb/python/py-disasm.c | 6 ++--
gdb/python/py-finishbreakpoint.c | 4 +--
gdb/python/py-framefilter.c | 17 +++++----
gdb/python/py-tui.c | 20 +++++------
gdb/python/python-internal.h | 61 ++++++++++++++++++++++++++++++--
6 files changed, 81 insertions(+), 29 deletions(-)
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index da74d69d357..ce0096680f5 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 (gdb_PyObject_CallMethod (py_bp, stop_func));
stop = 1;
if (result != NULL)
diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index 9337514acba..7fd595bf8c4 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -855,9 +855,9 @@ 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 (gdb_PyObject_CallMethod ((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 c74a2473a81..b6f85c202d6 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -344,8 +344,8 @@ 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 (gdb_PyObject_CallMethod (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 0cd15977d2f..6ae14c223b5 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<char> *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 (gdb_PyObject_CallMethod (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 (gdb_PyObject_CallMethod (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 (gdb_PyObject_CallMethod (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 (gdb_PyObject_CallMethod (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 (gdb_PyObject_CallMethod (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 (gdb_PyObject_CallMethod (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 (gdb_PyObject_CallMethod (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 (gdb_PyObject_CallMethod (filter, "line"));
int line;
if (py_line == NULL)
diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index 5dcec4bd2b1..57b13148e2e 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 (gdb_PyObject_CallMethod (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 (gdb_PyObject_CallMethod (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 (gdb_PyObject_CallMethod (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 (gdb_PyObject_CallMethod (m_window.get (), "vscroll",
+ num_to_scroll));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -248,9 +246,9 @@ 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 (gdb_PyObject_CallMethod (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 d07f239ca65..f4d7ae41c72 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -100,6 +100,12 @@
#define PyEval_ReleaseLock()
#endif
+/* Format string chars as defined here [1], in a readable and intuitive form.
+ [1] https://docs.python.org/3/c-api/arg.html#building-values. */
+
+#define GDB_PY_INT_ARG "i"
+#define GDB_PY_UNSIGNED_INT_ARG "I"
+
/* Python supplies HAVE_LONG_LONG and some `long long' support when it
is available. These defines let us handle the differences more
cleanly.
@@ -154,8 +160,10 @@ typedef long Py_hash_t;
template<typename... Args>
static inline PyObject *
-gdb_PyObject_CallMethod (PyObject *o, const char *method, const char *format,
- Args... args) /* ARI: editCase function */
+gdb_compatibility_PyObject_CallMethod (PyObject *o, const char *method,
+ const char *format,
+ Args... args) /* ARI: editCase
+ function. */
{
return PyObject_CallMethod (o,
const_cast<char *> (method),
@@ -164,7 +172,54 @@ gdb_PyObject_CallMethod (PyObject *o, const char *method, const char *format,
}
#undef PyObject_CallMethod
-#define PyObject_CallMethod gdb_PyObject_CallMethod
+#define PyObject_CallMethod gdb_compatibility_PyObject_CallMethod
+
+/* Non-variadic versions of PyObject_CallMethod. */
+
+static inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name)
+{
+ return PyObject_CallMethod (obj, name, nullptr);
+}
+
+template<typename T>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, T);
+
+template<>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, int arg1)
+{
+ return PyObject_CallMethod (obj, name, GDB_PY_INT_ARG, arg1);
+}
+
+template<typename T, typename T2>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, T, T2);
+
+template<>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, unsigned int arg1,
+ gdb_py_longest arg2)
+{
+ return PyObject_CallMethod (obj, name,
+ GDB_PY_UNSIGNED_INT_ARG GDB_PY_LL_ARG,
+ arg1, arg2);
+}
+
+template<typename T, typename T2, typename T3>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, T, T2, T3);
+
+template<>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, int arg1, int arg2,
+ int arg3)
+{
+ return PyObject_CallMethod (obj, name,
+ GDB_PY_INT_ARG GDB_PY_INT_ARG GDB_PY_INT_ARG,
+ arg1, arg2, arg3);
+}
/* The 'name' parameter of PyErr_NewException was missing the 'const'
qualifier in Python <= 3.4. Hence, we wrap it in a function to
base-commit: 60e221e9b7fae5ff40b79b93bdbff909989c2bae
--
2.35.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] [gdb/python] Add non-variadic versions of PyObject_CallMethod
2024-06-11 8:55 [PATCH v2] [gdb/python] Add non-variadic versions of PyObject_CallMethod Tom de Vries
@ 2024-06-11 19:55 ` Tom Tromey
2024-06-11 22:31 ` Tom de Vries
0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2024-06-11 19:55 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> Add non-variadic versions of PyObject_CallMethod that hide these details, such
Tom> that we can use the more intuitive:
I expanded on this with some template magic, to make gdb automatically
handle all possible overloads. Let me know what you think.
I think this should probably go one step further and make the
PyObject_CallMethod macro expand to some error, and then change gdb to
use some more obviously gdb-specific name, like gdbpy_call_method --
that way people with Python experience won't be confused by the gdb API.
Tom
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index da74d69d357..d72782e30c6 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 (PyObject_CallMethod (py_bp, stop_func));
stop = 1;
if (result != NULL)
diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index 9337514acba..e538d1e35f7 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -857,7 +857,7 @@ gdbpy_disassembler::read_memory_func (bfd_vma memaddr, gdb_byte *buff,
overridden by the user. */
gdbpy_ref<> result_obj (PyObject_CallMethod ((PyObject *) obj,
"read_memory",
- "I" GDB_PY_LL_ARG, len, offset));
+ len, offset));
/* Handle any exceptions. */
if (result_obj == nullptr)
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index c74a2473a81..8726d6554d2 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 (PyObject_CallMethod (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 0cd15977d2f..c38c0219db1 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<char> *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 (PyObject_CallMethod (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 (PyObject_CallMethod (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 (PyObject_CallMethod (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 (PyObject_CallMethod (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 (PyObject_CallMethod (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 (PyObject_CallMethod (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 (PyObject_CallMethod (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 (PyObject_CallMethod (filter, "line"));
int line;
if (py_line == NULL)
diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index 5dcec4bd2b1..2fd11970255 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 (PyObject_CallMethod (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 (PyObject_CallMethod (m_window.get (), "render"));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -213,7 +211,7 @@ 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));
+ num_to_scroll));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -227,7 +225,7 @@ 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));
+ num_to_scroll));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -249,7 +247,7 @@ 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_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 d07f239ca65..44ac33da34d 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -145,24 +145,64 @@ typedef long Py_hash_t;
#define PyMem_RawMalloc PyMem_Malloc
#endif
+/* A template variable holding the format character (as for
+ Py_BuildValue) for a given type. */
+template<typename T>
+constexpr char gdbpy_method_format = '\0';
+
+template<>
+constexpr char gdbpy_method_format<gdb_py_longest> = GDB_PY_LL_ARG[0];
+
+template<>
+constexpr char gdbpy_method_format<gdb_py_ulongest> = GDB_PY_LLU_ARG[0];
+
+template<>
+constexpr char gdbpy_method_format<int> = 'i';
+
+template<>
+constexpr char gdbpy_method_format<unsigned> = 'I';
+
+/* A helper function to compute the PyObject_CallMethod /
+ Py_BuildValue format given the argument types. */
+
+template<typename... Args>
+constexpr std::array<char, sizeof... (Args) + 1>
+gdbpy_make_fmt ()
+{
+ return { gdbpy_method_format<Args>..., '\0' };
+}
+
+/* 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. */
+
+static inline PyObject *
+gdb_PyObject_CallMethod (PyObject *o, const char *method)
+{
+ return PyObject_CallMethod (o, const_cast<char *> (method), nullptr);
+}
+
/* 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. */
-template<typename... Args>
+ This variant accepts any number of arguments and automatically
+ computes the format string, ensuring that format/argument
+ mismatches are impossible. */
+
+template<typename Arg, typename... Args>
static inline PyObject *
-gdb_PyObject_CallMethod (PyObject *o, const char *method, const char *format,
- Args... args) /* ARI: editCase function */
+gdb_PyObject_CallMethod (PyObject *o, const char *method,
+ Arg arg, Args... args)
{
+ constexpr const auto fmt = gdbpy_make_fmt<Arg, Args...> ();
return PyObject_CallMethod (o,
const_cast<char *> (method),
- const_cast<char *> (format),
- args...);
+ const_cast<char *> (fmt.data ()),
+ arg, args...);
}
+/* Ensure that gdb always uses the type-safe wrapper. */
#undef PyObject_CallMethod
#define PyObject_CallMethod gdb_PyObject_CallMethod
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] [gdb/python] Add non-variadic versions of PyObject_CallMethod
2024-06-11 19:55 ` Tom Tromey
@ 2024-06-11 22:31 ` Tom de Vries
2024-06-12 9:09 ` Tom de Vries
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2024-06-11 22:31 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 866 bytes --]
On 6/11/24 21:55, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>
> Tom> Add non-variadic versions of PyObject_CallMethod that hide these details, such
> Tom> that we can use the more intuitive:
>
> I expanded on this with some template magic, to make gdb automatically
> handle all possible overloads. Let me know what you think.
>
This is what I hoped to achieve when I started out rewriting this, but
didn't manage. So, I'm very happy with this solution, thanks for
helping out with this.
> I think this should probably go one step further and make the
> PyObject_CallMethod macro expand to some error, and then change gdb to
> use some more obviously gdb-specific name, like gdbpy_call_method --
> that way people with Python experience won't be confused by the gdb API.
I took a stab at this, attached below.
Thanks,
- Tom
[-- Attachment #2: 0001-gdb-python-Add-non-variadic-versions-of-PyObject_Cal.patch --]
[-- Type: text/x-patch, Size: 10879 bytes --]
From 22374806a29d1de43f8831fd91ee0b9864c9630c Mon Sep 17 00:00:00 2001
From: Tom Tromey <tom@tromey.com>
Date: Tue, 11 Jun 2024 13:55:54 -0600
Subject: [PATCH] [gdb/python] Add non-variadic versions of PyObject_CallMethod
---
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 | 68 ++++++++++++++++++++++++++------
6 files changed, 77 insertions(+), 37 deletions(-)
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index da74d69d357..1b8c7174d34 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 9337514acba..5206c7628f5 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 c74a2473a81..1b620e6d30e 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 0cd15977d2f..4ae583b4331 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<char> *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 5dcec4bd2b1..9df86df8c40 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 d07f239ca65..540b40b0a00 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -145,26 +145,72 @@ 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<typename T>
+constexpr char gdbpy_method_format = '\0';
+
+template<>
+constexpr char gdbpy_method_format<gdb_py_longest> = GDB_PY_LL_ARG[0];
+
+template<>
+constexpr char gdbpy_method_format<gdb_py_ulongest> = GDB_PY_LLU_ARG[0];
+
+template<>
+constexpr char gdbpy_method_format<int> = 'i';
+
+template<>
+constexpr char gdbpy_method_format<unsigned> = 'I';
+
+/* A helper function to compute the PyObject_CallMethod /
+ Py_BuildValue format given the argument types. */
template<typename... Args>
+constexpr std::array<char, sizeof... (Args) + 1>
+gdbpy_make_fmt ()
+{
+ return { gdbpy_method_format<Args>..., '\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<char *> (method),
- const_cast<char *> (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<typename Arg, typename... Args>
+static inline PyObject *
+gdbpy_call_method (PyObject *o, const char *method,
+ Arg arg, Args... args)
+{
+ constexpr const auto fmt = gdbpy_make_fmt<Arg, Args...> ();
+
+ /* PyObject_CallMethod's 'method' and 'format' parameters were missing the
+ 'const' qualifier before Python 3.4. */
+ return PyObject_CallMethod (o,
+ const_cast<char *> (method),
+ const_cast<char *> (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<typename... Args>
+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
base-commit: 6dfd07222c02edc792447049ba94518ae982f362
--
2.35.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] [gdb/python] Add non-variadic versions of PyObject_CallMethod
2024-06-11 22:31 ` Tom de Vries
@ 2024-06-12 9:09 ` Tom de Vries
0 siblings, 0 replies; 4+ messages in thread
From: Tom de Vries @ 2024-06-12 9:09 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 6/12/24 00:31, Tom de Vries wrote:
> On 6/11/24 21:55, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>
>> Tom> Add non-variadic versions of PyObject_CallMethod that hide these
>> details, such
>> Tom> that we can use the more intuitive:
>>
>> I expanded on this with some template magic, to make gdb automatically
>> handle all possible overloads. Let me know what you think.
>>
>
> This is what I hoped to achieve when I started out rewriting this, but
> didn't manage. So, I'm very happy with this solution, thanks for
> helping out with this.
>
>> I think this should probably go one step further and make the
>> PyObject_CallMethod macro expand to some error, and then change gdb to
>> use some more obviously gdb-specific name, like gdbpy_call_method --
>> that way people with Python experience won't be confused by the gdb API.
>
> I took a stab at this, attached below.
>
Submitted as v3 here (
https://sourceware.org/pipermail/gdb-patches/2024-June/209903.html ),
with $subject slightly changed.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-06-12 9:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-11 8:55 [PATCH v2] [gdb/python] Add non-variadic versions of PyObject_CallMethod Tom de Vries
2024-06-11 19:55 ` Tom Tromey
2024-06-11 22:31 ` Tom de Vries
2024-06-12 9:09 ` Tom de Vries
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).