From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2a07:de40:b251:101:10:150:64:1]) by sourceware.org (Postfix) with ESMTPS id AD10D3858D34 for ; Wed, 12 Jun 2024 09:00:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AD10D3858D34 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org AD10D3858D34 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a07:de40:b251:101:10:150:64:1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718182861; cv=none; b=Z7maKYQVmGxI75BNgO/vEnbovgz57m17y+O5T12aumKXaXsKMZnukHFkbdLoqENGzYsyu5nJy0G8i1ebVKNU4DIXH96S4v01nsw90tjUSVgSUlXr7BkMS3X9+9dmdtExjwyZvGANXqYF+Bz/JQ9JQ+3axj7MRSE9ceDC+xRQIXY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718182861; c=relaxed/simple; bh=XziUcKklkJU29irqSpT5/nZhjcBD4QYhZ9iFukMt3GU=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature:From: To:Subject:Date:Message-Id:MIME-Version; b=L1IvS4mzEogmUtXRcrzmOFJs7tsJj4v8NhKkxhiSNvBOQAGO585D8LyGg3TXCFMNdd8RhfJ8d2A10uT7nIAKm07iQlHalpys7ZmQo8ZEOvP8+KEzUZTJ23hU19oYczms/g5hrmXcyhem4XZP1Q+uBUuxkfQV+wySv3YNehEHFvw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap1.dmz-prg2.suse.org (unknown [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 8A674341CF; Wed, 12 Jun 2024 09:00:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1718182857; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=cDCfKybyo7FfBxPum7dhGeSjpvbVmzPq6c6Qadm9P5w=; b=M07ZL+QAkHnlPw7V9LZWXnpKNsb3Je2T+B2WSubMNSnKkJ5O4I/UyJ51BSlOGFrtkclncF OW7VdJBrAQD/sBOun4lsH4SVTuG0ay/GmKW0sZrtXZgTEB7Ve2yPapONuwyVv7PCEsx4Wv 7LbgS6R24l1d/PXhKErpMkOwjuHJnD8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1718182857; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=cDCfKybyo7FfBxPum7dhGeSjpvbVmzPq6c6Qadm9P5w=; b=KmMicYRuoDBlT5ekG0+ZwPMnIAAoDPSsIxP/g+9SdOvhx2mjCZ2XD/JVO1vjCqLy/KGsM2 oJO874RYuv7mVoCg== Authentication-Results: smtp-out1.suse.de; none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1718182857; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=cDCfKybyo7FfBxPum7dhGeSjpvbVmzPq6c6Qadm9P5w=; b=M07ZL+QAkHnlPw7V9LZWXnpKNsb3Je2T+B2WSubMNSnKkJ5O4I/UyJ51BSlOGFrtkclncF OW7VdJBrAQD/sBOun4lsH4SVTuG0ay/GmKW0sZrtXZgTEB7Ve2yPapONuwyVv7PCEsx4Wv 7LbgS6R24l1d/PXhKErpMkOwjuHJnD8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1718182857; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=cDCfKybyo7FfBxPum7dhGeSjpvbVmzPq6c6Qadm9P5w=; b=KmMicYRuoDBlT5ekG0+ZwPMnIAAoDPSsIxP/g+9SdOvhx2mjCZ2XD/JVO1vjCqLy/KGsM2 oJO874RYuv7mVoCg== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 6CDB2137DF; Wed, 12 Jun 2024 09:00:57 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id vksYGcljaWYuZgAAD6G6ig (envelope-from ); Wed, 12 Jun 2024 09:00:57 +0000 From: Tom de Vries To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH v3] [gdb/python] Add typesafe wrapper around PyObject_CallMethod Date: Wed, 12 Jun 2024 11:01:37 +0200 Message-Id: <20240612090137.12654-1-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Score: -2.80 X-Spam-Level: X-Spamd-Result: default: False [-2.80 / 50.00]; BAYES_HAM(-3.00)[100.00%]; MID_CONTAINS_FROM(1.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; R_MISSING_CHARSET(0.50)[]; NEURAL_HAM_SHORT(-0.20)[-1.000]; MIME_GOOD(-0.10)[text/plain]; RCPT_COUNT_TWO(0.00)[2]; RCVD_VIA_SMTP_AUTH(0.00)[]; MIME_TRACE(0.00)[0:+]; ARC_NA(0.00)[]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; RCVD_COUNT_TWO(0.00)[2]; TO_MATCH_ENVRCPT_ALL(0.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:email,imap1.dmz-prg2.suse.org:helo]; RCVD_TLS_ALL(0.00)[] X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: From: Tom Tromey 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 --- 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 *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 +constexpr char gdbpy_method_format = '\0'; + +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 base-commit: 6dfd07222c02edc792447049ba94518ae982f362 -- 2.35.3