From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id KqK1GCDdCmroNAUAWB0awg (envelope-from ) for ; Mon, 18 May 2026 05:34:24 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=cy4Ifxky; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 50FFE1E062; Mon, 18 May 2026 05:34:24 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 1C5441E062 for ; Mon, 18 May 2026 05:34:23 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5673B4BA23FF for ; Mon, 18 May 2026 09:34:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5673B4BA23FF Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=cy4Ifxky Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 18B624BA23F0 for ; Mon, 18 May 2026 09:33:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 18B624BA23F0 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 18B624BA23F0 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1779096833; cv=none; b=C5BBs6Jve/GwJhW0h3Qb171oUVjnKlyeKqK//KFhU9gGZwpU2mhY3ZnpNcGwsF1cvWANm33gWGctVBTcfQvfvyugcWmJpuHqHdRiVcUN91W1OVTo/6Vvf9GKQ3gQKNZ+Vtg7NL44gr35OHFSQKTbzf1AN6U0Jy1Mo5RQPj7/CLM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1779096833; c=relaxed/simple; bh=27VNMmGxO0dwAIemITlDOYU+AkBM+G/YLSaazUPOm/M=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=DkiLTm9amvb9urAoWjvNT3DNfrQ8TGRDO+ryMpM/vmV1OUzFiTCoHHXd/QLaSTJotDtElU9xrkpp0hscd8akLFr5mf3rBQxHWttEUhEvLT+GB8S6qonCs7/dgwaz4XWoURWcZU1aw2xMB2zJGPDykpCNGggSbQ4u63T3hhjSBx0= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=cy4Ifxky DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 18B624BA23F0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1779096832; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=SPKEYoZ7aj1jew5xAHHGLMXCkA0oMkdBKxqcUWtB3D0=; b=cy4Ifxky7ni9UO01UdeGQPgZPwsR10lcV4/5gQ+VAhsoaCQdg5KajBuYhydJwZWMxvX005 tY+sZLa7qnu4TAYbs4KZLKSu7GKvpbBpQg7GrcOdW3x5EbXYlLWsyJRe1wCUU0mqv4qpgc NBX33KsVAzFGe1Zlhb7Uh+wzr0qirFo= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-470-ZYlPj9sYPsioa4tEy0kBpw-1; Mon, 18 May 2026 05:33:51 -0400 X-MC-Unique: ZYlPj9sYPsioa4tEy0kBpw-1 X-Mimecast-MFC-AGG-ID: ZYlPj9sYPsioa4tEy0kBpw_1779096830 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-44f1b4d0fb0so1162803f8f.1 for ; Mon, 18 May 2026 02:33:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779096830; x=1779701630; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=SPKEYoZ7aj1jew5xAHHGLMXCkA0oMkdBKxqcUWtB3D0=; b=q2cu7b+6GJJCmfoas3HMEsM91G6gC/OIX0mln0DYQ3zB7v86fg2E1Y9X0aQulamyax YbX0O9O2UmL7jQcG+vu846fouiXXUr14gYB9fEAFAiH+tMDoYLi5VXQ0WN95j1V93bWG weMVyjzUnbqRek5ciNeE3LIFmwiMQhYk+z6h62hv/ocln6E2EmqQjMzr41Hm2Y9eYbSI Ixua0pb9TvSe80MZt2tYTuDWkaTSb9t1a0UlTyi7Wi+hGjlx8ScMv2l8w+lZ8/f3qz9a 5ej1EDgXZn0KgjwnF0r0fR209XEw50fxYtC9Np+07JBcuIFn1AMIYtwzzUUnieHuts/M LaVQ== X-Forwarded-Encrypted: i=1; AFNElJ+g/JVgN9Qp9wWBjrMYFdIkYox5HIVB7GrWY38bwBrF0RfGto9IuABLllk9tEcn4t4Eh6lIphqR6avSQw==@sourceware.org X-Gm-Message-State: AOJu0YzVEU/VIYg65yOSunrEd6Lpl6246RzG2YrdCK857wcjYsVMeKRe QWLrAda8PiQSVGRlJFmJjbdDghZW9wD2az+qXnKN5WE0Lbqlu4a7JpLlNciosS9u1Rb11iJP2f7 ZHbEibtUkDMEU1MYYyF/oId2KNC1LSWZ3PJiB8dm6OB1OtWKLujO33LhWfjRCo/bwcFXvslY= X-Gm-Gg: Acq92OH9fDSJM/uSQ0bfsTEP97goHfRGb0xwN/gb/OoZbp/NszdKRf+cbVMTlQGDZQX qLksGynt/Ml/PeG9hcerZ2qbcpohIhXbhiOjDEpesK23ElWL2zo7tqTCHXjOgigPDixUFuVxnoz lqtHcP8/B045P29NrYn/xswjA3GTKXs9DjkLhUN8zZrZLJxhqCHXY2dzHsd7LUq7ArluI3JEKp4 FqtfkB97652Tu33WAj1/8LsyJZcNwM5wnJd9sacadsaPmvXjuMh2Gj2vac6buyYDt9jFURD8abC KEPpHqjUbXoHgHWLHoqElxLF9hYIOoZ+vHpZL9CdnZI6In7nwEbEKErmlODlaFlvWAVNeOk5Mjc jT5RWerngJrBlxKO/ X-Received: by 2002:a05:6000:230d:b0:446:96b1:f5f with SMTP id ffacd0b85a97d-45e5c35da70mr19842911f8f.8.1779096829945; Mon, 18 May 2026 02:33:49 -0700 (PDT) X-Received: by 2002:a05:6000:230d:b0:446:96b1:f5f with SMTP id ffacd0b85a97d-45e5c35da70mr19842859f8f.8.1779096829413; Mon, 18 May 2026 02:33:49 -0700 (PDT) Received: from localhost ([31.111.84.232]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-45d9ec3ac86sm35593416f8f.14.2026.05.18.02.33.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 May 2026 02:33:48 -0700 (PDT) From: Andrew Burgess To: Tom Tromey , gdb-patches@sourceware.org Cc: Tom Tromey Subject: Re: [PATCH v2 3/4] Add wrappers for Python implementation functions and methods In-Reply-To: <20260515-python-safety-initial-v2-3-6129cadf258a@tromey.com> References: <20260515-python-safety-initial-v2-0-6129cadf258a@tromey.com> <20260515-python-safety-initial-v2-3-6129cadf258a@tromey.com> Date: Mon, 18 May 2026 10:33:48 +0100 Message-ID: <877bp1cb9f.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: WJNuKHzxhPq5vgnHdKNru6kw8pXOucIvGJd4RlBY2rE_1779096830 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Tom Tromey writes: > This adds some wrappers for Python implementation functions and > methods, and a couple of new constexpr functions to create PyMethodDef > entries. This provides a few safety benefits: > > * The new-style API approach (see previous patch) is implemented by > the wrapper. That is, exceptions are caught here and transformed. > > * The implementation functions can now return any reasonable type, > with automatic conversion by the wrapper. > > * The function API and the appropriate METH_* flags are handled > together, avoiding any possible discrepancy. > > This approach also means that we can modify the old rule that gdb > calls must be wrapped in a try/catch -- the try/catch is now provided > by the wrapper function, so the implementation can be written in a > more natural way. > > Note this patch is not 100% complete. There should be one more > wrapper for case where a method takes a single argument (though we > probably cannot use METH_O unfortunately). There may be some other > holes as well. > --- > gdb/python/py-safety.h | 320 +++++++++++++++++++++++++++++++++++++++++++ > gdb/python/python-internal.h | 1 + > 2 files changed, 321 insertions(+) > > diff --git a/gdb/python/py-safety.h b/gdb/python/py-safety.h > new file mode 100644 > index 00000000000..62018dacc0f > --- /dev/null > +++ b/gdb/python/py-safety.h > @@ -0,0 +1,320 @@ > +/* Wrappers for some Python safety. > + > + Copyright (C) 2026 Free Software Foundation, Inc. > + > + This file is part of GDB. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +#ifndef GDB_PYTHON_PY_SAFETY_H > +#define GDB_PYTHON_PY_SAFETY_H > + > +#include "gdbsupport/traits.h" > +#include "py-ref.h" > +#include "py-wrappers.h" > +#include "charset.h" > + > +/* This file holds wrapper templates for the various ways that gdb > + code might be exposed to Python. These wrappers are part of gdb's > + "Python safety" approach -- utilities designed to try to prevent > + refcount problems, missing error checks, and that also remove the > + need to wrap calls into gdb in an explicit try/catch. > + > + See py-wrappers.h for some more discussion of this. > + > + Implementation functions and methods -- the stuff you write to > + expose some part of gdb to Python -- are written in a certain > + style. > + > + An implementation function is a module-level function. For example > + something like 'gdb.register_window_type' is implemented as a > + function. An implementation method is a method of the gdb class > + implementing a certain Python type; for example > + 'gdb.TuiWindow.width' is implemented via a method of > + gdbpy_tui_window. > + > + Each of these will accept gdbpy_borrowed_ref arguments (or in some > + more limited situations, a gdbpy_opt_borrowed_ref) and return any > + relevant type, which will be automatically converted (see the > + 'to_python' overloads below) to the correct Python type. Note that > + if the 'to_python' functions aren't appropriate in your case, you > + can always use the escape hatch of returning a gdbpy_ref<> and > + handling type conversion manually. > + > + There are some constexpr functions below that wrap the > + implementation methods, and create PyMethodDef objects and the > + like. > + > + Implementation methods are expected to use the wrappers in > + py-wrappers.h and not generally call into Python directly. > + > + Implementation details of the method-wrapping safety code are put > + into this namespace, just to emphasize that these shouldn't be used > + elsewhere. Skip past the namespace to find the public APIs. */ > +namespace safety_details > +{ > +/* Overloads of "to_python" are used by the safety wrappers to convert > + a function's real return value to a Python object. A new reference > + will always be returned. For the time being these are kept as a > + detail of the method-wrapping code. However we may want to > + consider exposing these more generally. */ > + > +static inline PyObject * > +to_python (bool value) > +{ > + /* Note that this cannot fail. */ > + return PyBool_FromLong (value); > +} > + > +template> > +static inline PyObject * > +to_python (T value) > +{ > + if constexpr (std::is_signed::value) > + return gdb_py_object_from_longest (value).release (); > + else > + return gdb_py_object_from_ulongest (value).release (); > +} > + > +static inline PyObject * > +to_python (const char *value) > +{ > + if (value == nullptr) > + return py_none ().release (); > + return PyUnicode_Decode (value, strlen (value), host_charset (), nullptr); > +} > + > +static inline PyObject * > +to_python (std::string &&value) > +{ > + return PyUnicode_Decode (value.c_str (), value.size (), > + host_charset (), nullptr); > +} > + > +static inline PyObject * > +to_python (const std::string &value) > +{ > + return PyUnicode_Decode (value.c_str (), value.size (), > + host_charset (), nullptr); > +} > + > +static inline PyObject * > +to_python (gdb::unique_xmalloc_ptr &&value) > +{ > + if (value == nullptr) > + return py_none ().release (); > + return PyUnicode_Decode (value.get (), strlen (value.get ()), > + host_charset (), nullptr); > +} > + > +static inline PyObject * > +to_python (gdbpy_ref<> &&value) > +{ > + return value.release (); > +} > + > +/* An instantiation of this function is used when calling a gdb method > + from Python. It accepts some number of arguments (normally > + gdbpy_borrowed_ref or gdbpy_opt_borrowed_ref), and then then calls > + the underlying function F. Any exceptions are caught and > + converted, and the return value of F is converted to a Python > + object as appropriate. */ > +template > +PyObject * > +wrapped_function (Args... args) > +{ > + try > + { > + using result_type = typename std::invoke_result_t; > + > + if constexpr (std::is_void_v) > + { > + F (args...); > + return py_none ().release (); > + } > + else > + return to_python (F (args...)); > + } > + catch (const gdb_python_exception &pye) > + { > + gdb_assert (PyErr_Occurred () != nullptr); > + return nullptr; > + } > + catch (const gdb_exception &exc) > + { > + return gdbpy_handle_gdb_exception (nullptr, exc); > + } > +} > + > +/* An instantiation of this function is used when calling a gdb method > + from Python. It accepts some number of arguments (normally > + gdbpy_borrowed_ref or gdbpy_opt_borrowed_ref), and then then calls > + the underlying function F. Any exceptions are caught and > + converted, and the return value of F is converted to a Python > + object as appropriate. */ > +template > +PyObject * > +wrapped_method (Ret (Class::*meth) (Args...), Class *self, Args... args) > +{ > + try > + { > + if constexpr (std::is_void_v) > + { > + (self->*meth) (args...); > + return py_none ().release (); > + } > + else > + return to_python ((self->*meth) (args...)); > + } > + catch (const gdb_python_exception &pye) > + { > + gdb_assert (PyErr_Occurred () != nullptr); > + return nullptr; > + } > + catch (const gdb_exception &exc) > + { > + return gdbpy_handle_gdb_exception (nullptr, exc); > + } > +} > + > +template > +PyObject * > +fn_wrapper (PyObject *self, PyObject *args, PyObject *kw) > +{ > + return wrapped_function (gdbpy_borrowed_ref (args), > + gdbpy_opt_borrowed_ref (kw)); > +} > + > +template > +PyObject * > +varargs_wrapper (PyObject *self, PyObject *args, PyObject *kw) > +{ > + return wrapped_method (M, static_cast (self), > + gdbpy_borrowed_ref (args), > + gdbpy_opt_borrowed_ref (kw)); > +} > + > +} /* namespace safety_details */ > + > +/* Create a PyMethodDef for a no-argument method. It takes the > + underlying class C and a pointer-to-method M as template > + parameters, and the name and documentation as arguments. The > + method M is wrapped to call to_python and to catch exceptions per > + the safety protocol. */ > +template > +constexpr PyMethodDef > +noargs_method (std::string_view name, std::string_view doc) Using std::string_view seems a little suspect here. Calling std::string_view::data doesn't guarantee a NULL terminated string, and creating a string_view from a C-string only encodes the length of the C-string, not 'length + 1' which would be required to capture the NULL terminator. Now given how this gets used in the next patch we're going to be fine, you pass a C-string, Python will read outside the bounds of the string_view and find the NULL terminator, but that all just seems a little ... iffy. If we don't actually care about the length of the string_view, then should we be using string_view at all? Shouldn't we just be honest with ourselves and pass through 'const char *'? There are other uses of string_view below, and the same thoughts apply. > +{ > + using namespace safety_details; > + return { > + name.data (), > + [] (PyObject *self, PyObject *args) -> PyObject * > + { > + return wrapped_method (M, static_cast (self)); > + }, > + METH_NOARGS, > + doc.data (), > + }; > +} > + > +/* This is used to create the PyMethodDef for a varargs function. It > + takes the underlying implementation function as a template > + argument, and also arguments for the method name and documentation > + string. > + > + The underlying function should accept a gdbpy_borrowed_ref argument > + (the arguments to the Python function), and then a > + gdbpy_opt_borrowed_ref for the keywords. The function can return > + any type (see the to_python overloads); and should throw an > + exception on error. If gdb_python_exception is thrown, the Python > + exception must already have been set. > + > + The gdb policy is that varargs methods must also accept keywords, > + and this is enforced here. */ > +template > +constexpr PyMethodDef > +varargs_function (std::string_view name, std::string_view doc) > +{ > + using namespace safety_details; > + return { > + name.data (), > + (PyCFunction) fn_wrapper, > + /* gdb's rule is that varargs should also use keywords. */ > + METH_VARARGS | METH_KEYWORDS, > + doc.data (), > + }; > +} > + > +/* Like a varargs function, but this implements a method on some > + Python type that gdb provides. The implementation class and a > + pointer-to-method must be specified. */ > +template > +constexpr PyMethodDef > +varargs_method (std::string_view name, std::string_view doc) > +{ > + using namespace safety_details; > + return { > + name.data (), > + (PyCFunction) varargs_wrapper, > + /* gdb's rule is that varargs should also use keywords. */ > + METH_VARARGS | METH_KEYWORDS, > + doc.data (), > + }; > +} > + > +/* A function that wraps a "repr" or "str" method. */ > +template > +PyObject * > +wrap_repr (PyObject *arg) Maybe something like 'wrap_tp_callback' might be better? I just know every time I see 'wrap_repr' for something that isn't 'repr' I'm going to assume copy & paste error, and then have to go and remind myself that 'wrap_repr' has wider uses. Now's a great time to find a better name before this starts getting used more widely.. Thanks, Andrew > +{ > + using namespace safety_details; > + return wrapped_method (M, static_cast (arg)); > +} > + > +/* A function that wraps a "get" method. */ > +template > +PyObject * > +wrap_getter (PyObject *arg, void *closure) > +{ > + using namespace safety_details; > + /* In gdb the closure argument is never used. */ > + return wrapped_method (M, static_cast (arg)); > +} > + > +/* A function that wraps a "set" method. */ > +template > +int > +wrap_setter (PyObject *arg, PyObject *value, void *closure) > +{ > + using namespace safety_details; > + try > + { > + C *self = static_cast (arg); > + /* In gdb the closure argument is never used. */ > + (self->*M) (gdbpy_opt_borrowed_ref (value)); > + } > + catch (const gdb_python_exception &pye) > + { > + gdb_assert (PyErr_Occurred () != nullptr); > + return -1; > + } > + catch (const gdb_exception &exc) > + { > + return gdbpy_handle_gdb_exception (-1, exc); > + } > + > + return 0; > +} > + > +#endif /* GDB_PYTHON_PY_SAFETY_H */ > diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h > index 6df0c62e2b3..e4f35f8cd88 100644 > --- a/gdb/python/python-internal.h > +++ b/gdb/python/python-internal.h > @@ -1383,5 +1383,6 @@ py_notimplemented () > #undef Py_RETURN_NOTIMPLEMENTED > > #include "py-wrappers.h" > +#include "py-safety.h" > > #endif /* GDB_PYTHON_PYTHON_INTERNAL_H */ > > -- > 2.49.0