public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v3] [gdb/python] Add typesafe wrapper around PyObject_CallMethod
@ 2024-06-12  9:01 Tom de Vries
  2024-06-12 13:59 ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: Tom de Vries @ 2024-06-12  9:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

From: Tom Tromey <tom@tromey.com>

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 <tdevries@suse.de>
---
 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 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..bd30d0c1d03 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<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] 6+ messages in thread

* Re: [PATCH v3] [gdb/python] Add typesafe wrapper around PyObject_CallMethod
  2024-06-12  9:01 [PATCH v3] [gdb/python] Add typesafe wrapper around PyObject_CallMethod Tom de Vries
@ 2024-06-12 13:59 ` Tom Tromey
  2024-06-12 17:03   ` Tom de Vries
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2024-06-12 13:59 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches, Tom Tromey

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Add a typesafe wrapper around PyObject_CallMethod that hides these
Tom> details, such that we can use the more intuitive:
Tom> ...
Tom>       gdbpy_ref<> result (gdbpy_call_method (m_window.get(), "hscroll",
Tom>                                              num_to_scroll));
Tom> ...
Tom> and:
Tom> ...
Tom>       gdbpy_ref<> result (gdbpy_call_method (m_window.get (), "render"));
Tom> ...

Tom> Tested on x86_64-linux.

Tom> Co-Authored-By: Tom de Vries <tdevries@suse.de>

Thanks for following up on this.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] [gdb/python] Add typesafe wrapper around PyObject_CallMethod
  2024-06-12 13:59 ` Tom Tromey
@ 2024-06-12 17:03   ` Tom de Vries
  2024-06-19  9:09     ` Tom de Vries
  0 siblings, 1 reply; 6+ messages in thread
From: Tom de Vries @ 2024-06-12 17:03 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 6/12/24 15:59, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Add a typesafe wrapper around PyObject_CallMethod that hides these
> Tom> details, such that we can use the more intuitive:
> Tom> ...
> Tom>       gdbpy_ref<> result (gdbpy_call_method (m_window.get(), "hscroll",
> Tom>                                              num_to_scroll));
> Tom> ...
> Tom> and:
> Tom> ...
> Tom>       gdbpy_ref<> result (gdbpy_call_method (m_window.get (), "render"));
> Tom> ...
> 
> Tom> Tested on x86_64-linux.
> 
> Tom> Co-Authored-By: Tom de Vries <tdevries@suse.de>
> 
> Thanks for following up on this.
> Approved-By: Tom Tromey <tom@tromey.com>

Committed, with this fix:
...
  template<typename T>
-constexpr char gdbpy_method_format = '\0';
+constexpr char gdbpy_method_format;
...
to make sure that we get a compile time error rather than a runtime one 
when using unsupported types.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] [gdb/python] Add typesafe wrapper around PyObject_CallMethod
  2024-06-12 17:03   ` Tom de Vries
@ 2024-06-19  9:09     ` Tom de Vries
  2024-06-19 11:15       ` Tom de Vries
  0 siblings, 1 reply; 6+ messages in thread
From: Tom de Vries @ 2024-06-19  9:09 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Lancelot SIX

On 6/12/24 19:03, Tom de Vries wrote:
> On 6/12/24 15:59, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>
>> Tom> Add a typesafe wrapper around PyObject_CallMethod that hides these
>> Tom> details, such that we can use the more intuitive:
>> Tom> ...
>> Tom>       gdbpy_ref<> result (gdbpy_call_method (m_window.get(), 
>> "hscroll",
>> Tom>                                              num_to_scroll));
>> Tom> ...
>> Tom> and:
>> Tom> ...
>> Tom>       gdbpy_ref<> result (gdbpy_call_method (m_window.get (), 
>> "render"));
>> Tom> ...
>>
>> Tom> Tested on x86_64-linux.
>>
>> Tom> Co-Authored-By: Tom de Vries <tdevries@suse.de>
>>
>> Thanks for following up on this.
>> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Committed, with this fix:
> ...
>   template<typename T>
> -constexpr char gdbpy_method_format = '\0';
> +constexpr char gdbpy_method_format;
> ...
> to make sure that we get a compile time error rather than a runtime one 
> when using unsupported types.
> 

And that caused a problem that Lancelot submitted a fix for here ( 
https://sourceware.org/pipermail/gdb-patches/2024-June/210071.html ).

However, that doesn't seem to be the only problem.

With gcc 14, on fedora rawhide, the build fails due to:
...
/usr/bin/ld: python/py-disasm.o: in function `gdb::ref_ptr<_object, 
gdbpy_ref_policy<_object> > gdbpy_call_method<unsigned int, long 
long>(_object*, char const*, unsigned int, long long)':
/data/vries/gdb/src/gdb/python/python-internal.h:207:(.text+0x384f): 
undefined reference to `_object* PyObject_CallMethod<_object*, char*, 
char*, unsigned int, long long>(_object*, char*, char*, unsigned int, 
long long)'
/usr/bin/ld: python/py-tui.o: in function `gdb::ref_ptr<_object, 
gdbpy_ref_policy<_object> > gdbpy_call_method<int>(_object*, char 
const*, int)':
/data/vries/gdb/src/gdb/python/python-internal.h:207:(.text+0x1235): 
undefined reference to `_object* PyObject_CallMethod<_object*, char*, 
char*, int>(_object*, char*, char*, int)'
/usr/bin/ld: python/py-tui.o: in function `gdb::ref_ptr<_object, 
gdbpy_ref_policy<_object> > gdbpy_call_method<int, int, int>(_object*, 
char const*, int, int, int)':
/data/vries/gdb/src/gdb/python/python-internal.h:207:(.text+0x12b0): 
undefined reference to `_object* PyObject_CallMethod<_object*, char*, 
char*, int, int, int>(_object*, char*, char*, int, int, int)'
...

I tried applying the patch mentioned above, and that didn't fix it.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] [gdb/python] Add typesafe wrapper around PyObject_CallMethod
  2024-06-19  9:09     ` Tom de Vries
@ 2024-06-19 11:15       ` Tom de Vries
  2024-06-19 12:46         ` Tom de Vries
  0 siblings, 1 reply; 6+ messages in thread
From: Tom de Vries @ 2024-06-19 11:15 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Lancelot SIX

On 6/19/24 11:09, Tom de Vries wrote:
> On 6/12/24 19:03, Tom de Vries wrote:
>> On 6/12/24 15:59, Tom Tromey wrote:
>>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>>
>>> Tom> Add a typesafe wrapper around PyObject_CallMethod that hides these
>>> Tom> details, such that we can use the more intuitive:
>>> Tom> ...
>>> Tom>       gdbpy_ref<> result (gdbpy_call_method (m_window.get(), 
>>> "hscroll",
>>> Tom>                                              num_to_scroll));
>>> Tom> ...
>>> Tom> and:
>>> Tom> ...
>>> Tom>       gdbpy_ref<> result (gdbpy_call_method (m_window.get (), 
>>> "render"));
>>> Tom> ...
>>>
>>> Tom> Tested on x86_64-linux.
>>>
>>> Tom> Co-Authored-By: Tom de Vries <tdevries@suse.de>
>>>
>>> Thanks for following up on this.
>>> Approved-By: Tom Tromey <tom@tromey.com>
>>
>> Committed, with this fix:
>> ...
>>   template<typename T>
>> -constexpr char gdbpy_method_format = '\0';
>> +constexpr char gdbpy_method_format;
>> ...
>> to make sure that we get a compile time error rather than a runtime 
>> one when using unsupported types.
>>
> 
> And that caused a problem that Lancelot submitted a fix for here ( 
> https://sourceware.org/pipermail/gdb-patches/2024-June/210071.html ).
> 
> However, that doesn't seem to be the only problem.
> 
> With gcc 14, on fedora rawhide, the build fails due to:
> ...
> /usr/bin/ld: python/py-disasm.o: in function `gdb::ref_ptr<_object, 
> gdbpy_ref_policy<_object> > gdbpy_call_method<unsigned int, long 
> long>(_object*, char const*, unsigned int, long long)':
> /data/vries/gdb/src/gdb/python/python-internal.h:207:(.text+0x384f): 
> undefined reference to `_object* PyObject_CallMethod<_object*, char*, 
> char*, unsigned int, long long>(_object*, char*, char*, unsigned int, 
> long long)'
> /usr/bin/ld: python/py-tui.o: in function `gdb::ref_ptr<_object, 
> gdbpy_ref_policy<_object> > gdbpy_call_method<int>(_object*, char 
> const*, int)':
> /data/vries/gdb/src/gdb/python/python-internal.h:207:(.text+0x1235): 
> undefined reference to `_object* PyObject_CallMethod<_object*, char*, 
> char*, int>(_object*, char*, char*, int)'
> /usr/bin/ld: python/py-tui.o: in function `gdb::ref_ptr<_object, 
> gdbpy_ref_policy<_object> > gdbpy_call_method<int, int, int>(_object*, 
> char const*, int, int, int)':
> /data/vries/gdb/src/gdb/python/python-internal.h:207:(.text+0x12b0): 
> undefined reference to `_object* PyObject_CallMethod<_object*, char*, 
> char*, int, int, int>(_object*, char*, char*, int, int, int)'
> ...
> 
> I tried applying the patch mentioned above, and that didn't fix it.

OK, that seems to be fixed by dropping the poisoning of PyObject_CallMethod:
...
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index fec0010a444..fae7d94a339 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -228,13 +228,6 @@ gdbpy_call_method (const gdbpy_ref<> &o, const char 
*method, Args... args)
    return gdbpy_call_method (o.get (), method, args...);
  }

-/* Poison PyObject_CallMethod.  The typesafe wrapper gdbpy_call_method 
should be
-   used instead.  */
-#undef 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
     avoid errors when compiled with -Werror.  */

...

I'll submit a patch.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] [gdb/python] Add typesafe wrapper around PyObject_CallMethod
  2024-06-19 11:15       ` Tom de Vries
@ 2024-06-19 12:46         ` Tom de Vries
  0 siblings, 0 replies; 6+ messages in thread
From: Tom de Vries @ 2024-06-19 12:46 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Lancelot SIX

On 6/19/24 13:15, Tom de Vries wrote:
> On 6/19/24 11:09, Tom de Vries wrote:
>> On 6/12/24 19:03, Tom de Vries wrote:
>>> On 6/12/24 15:59, Tom Tromey wrote:
>>>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>>>
>>>> Tom> Add a typesafe wrapper around PyObject_CallMethod that hides these
>>>> Tom> details, such that we can use the more intuitive:
>>>> Tom> ...
>>>> Tom>       gdbpy_ref<> result (gdbpy_call_method (m_window.get(), 
>>>> "hscroll",
>>>> Tom>                                              num_to_scroll));
>>>> Tom> ...
>>>> Tom> and:
>>>> Tom> ...
>>>> Tom>       gdbpy_ref<> result (gdbpy_call_method (m_window.get (), 
>>>> "render"));
>>>> Tom> ...
>>>>
>>>> Tom> Tested on x86_64-linux.
>>>>
>>>> Tom> Co-Authored-By: Tom de Vries <tdevries@suse.de>
>>>>
>>>> Thanks for following up on this.
>>>> Approved-By: Tom Tromey <tom@tromey.com>
>>>
>>> Committed, with this fix:
>>> ...
>>>   template<typename T>
>>> -constexpr char gdbpy_method_format = '\0';
>>> +constexpr char gdbpy_method_format;
>>> ...
>>> to make sure that we get a compile time error rather than a runtime 
>>> one when using unsupported types.
>>>
>>
>> And that caused a problem that Lancelot submitted a fix for here ( 
>> https://sourceware.org/pipermail/gdb-patches/2024-June/210071.html ).
>>
>> However, that doesn't seem to be the only problem.
>>
>> With gcc 14, on fedora rawhide, the build fails due to:
>> ...
>> /usr/bin/ld: python/py-disasm.o: in function `gdb::ref_ptr<_object, 
>> gdbpy_ref_policy<_object> > gdbpy_call_method<unsigned int, long 
>> long>(_object*, char const*, unsigned int, long long)':
>> /data/vries/gdb/src/gdb/python/python-internal.h:207:(.text+0x384f): 
>> undefined reference to `_object* PyObject_CallMethod<_object*, char*, 
>> char*, unsigned int, long long>(_object*, char*, char*, unsigned int, 
>> long long)'
>> /usr/bin/ld: python/py-tui.o: in function `gdb::ref_ptr<_object, 
>> gdbpy_ref_policy<_object> > gdbpy_call_method<int>(_object*, char 
>> const*, int)':
>> /data/vries/gdb/src/gdb/python/python-internal.h:207:(.text+0x1235): 
>> undefined reference to `_object* PyObject_CallMethod<_object*, char*, 
>> char*, int>(_object*, char*, char*, int)'
>> /usr/bin/ld: python/py-tui.o: in function `gdb::ref_ptr<_object, 
>> gdbpy_ref_policy<_object> > gdbpy_call_method<int, int, int>(_object*, 
>> char const*, int, int, int)':
>> /data/vries/gdb/src/gdb/python/python-internal.h:207:(.text+0x12b0): 
>> undefined reference to `_object* PyObject_CallMethod<_object*, char*, 
>> char*, int, int, int>(_object*, char*, char*, int, int, int)'
>> ...
>>
>> I tried applying the patch mentioned above, and that didn't fix it.
> 
> OK, that seems to be fixed by dropping the poisoning of 
> PyObject_CallMethod:
> ...
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> index fec0010a444..fae7d94a339 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -228,13 +228,6 @@ gdbpy_call_method (const gdbpy_ref<> &o, const char 
> *method, Args... args)
>     return gdbpy_call_method (o.get (), method, args...);
>   }
> 
> -/* Poison PyObject_CallMethod.  The typesafe wrapper gdbpy_call_method 
> should be
> -   used instead.  */
> -#undef 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
>      avoid errors when compiled with -Werror.  */
> 
> ...
> 
> I'll submit a patch.
> 

I ended up re-implementing the poisoning, without using templates.

Submitted here ( 
https://sourceware.org/pipermail/gdb-patches/2024-June/210103.html ).

Thanks,
- Tom



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-06-19 12:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-12  9:01 [PATCH v3] [gdb/python] Add typesafe wrapper around PyObject_CallMethod Tom de Vries
2024-06-12 13:59 ` Tom Tromey
2024-06-12 17:03   ` Tom de Vries
2024-06-19  9:09     ` Tom de Vries
2024-06-19 11:15       ` Tom de Vries
2024-06-19 12:46         ` 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).