From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id K/NnJ1WvCWproAIAWB0awg (envelope-from ) for ; Sun, 17 May 2026 08:06:45 -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=PStIsQ6F; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 8E9331E024; Sun, 17 May 2026 08:06:45 -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 6FA811E024 for ; Sun, 17 May 2026 08:06:44 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 661AF4BA7996 for ; Sun, 17 May 2026 12:06:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 661AF4BA7996 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=PStIsQ6F Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id AD8DD4BA23F1 for ; Sun, 17 May 2026 12:06:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AD8DD4BA23F1 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 AD8DD4BA23F1 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1779019573; cv=none; b=mnDYGuYXKnLiSkBoWwX0ujej4m0OVhyeRSix6Q//okbfsXSzZ2c+CUaI1RSGNK5zgm4gPgyKhOUKuzA6Ha+veXQJWj7iwkRv5bu8pVQzXShVF6AERlPI4WORNXPOc3uX85WSsafJT7XX3UjEn3xAM+9EURp3aE9Xpg28MmM2XR4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1779019573; c=relaxed/simple; bh=1xxY2jq/vKysehfaAekW7Ycx6Sy0RqYJVKaqykx9scM=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=HuN6Rn/7mvaJ43EVdrY8oUOQIJ+zBpz2651Bfgy6WW3OWix0gqLjvv7FnnW+vrgXBRJyN4yNsZ7+ZKNCsKkT1YaDicU0UoPhCQzDZPVLdC3EwhNOIrgvJpXw+b1C46h8jN8A81oS9V4O71mJehxf+u6fUx6y1uXFLvJHldt72NQ= 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=PStIsQ6F DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AD8DD4BA23F1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1779019573; 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=G3a1d7tZRQuFZDC7okrqxy3ERiMPkY8w2GjPF24wFB0=; b=PStIsQ6Fv46ZOGdYssuVuSOMuy90m7Mfj1WTTePwXPX4Ha36mSHhM3RsF9JB7cWjirKCet 7lBVsUa6ZbKmhVcRqK+hFQAlB+dJoJqPZnBgqehU889wjfcloBimEeuo3ehQeOHWdL+qcH Spj5hpmhWPE8q/nIw8PZEceqChNFu00= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-477-ZVLO1OOAMV2jN698oX4d9w-1; Sun, 17 May 2026 08:06:11 -0400 X-MC-Unique: ZVLO1OOAMV2jN698oX4d9w-1 X-Mimecast-MFC-AGG-ID: ZVLO1OOAMV2jN698oX4d9w_1779019570 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-488d1b5bca0so4573425e9.2 for ; Sun, 17 May 2026 05:06:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779019569; x=1779624369; 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=G3a1d7tZRQuFZDC7okrqxy3ERiMPkY8w2GjPF24wFB0=; b=OBkME/ViTlNFAxC22KdaiFW1LJ3DoSXkFFi/EXl+Bl2X0v8+Nu8zgRwc53zQd9Wbe+ jYslBZLVOodJmO/bxxTEpLZ5nvJ05ylT8Mof9IB++KKhVHY5GscPypfUXp69Ru7N6hN7 2v/L9y5HcnsHxNoE3nklsI1QArgj0dtDObZbgZjbDrlGEkIO6/1DltU5jN/AJa8hqfd+ 2ZpjGqFSG2X1ztAcuOrN3U9MI858xp0Vjk9KTVZbW9jwQYj/RfjovnDJD1otPgIVPz5n IQO4a90OYYot+By/l3ePY37gQlxIBPqBlFJjQA4WFEvhOFzDoBOClDMctlGvDuaRmtxR 6Gzw== X-Forwarded-Encrypted: i=1; AFNElJ+xhT3/5RoCGqAqc+yg/OkXRhzWkKBPK6X3M/NxGQ//QFW2FP6Ye/ic/grHg11MXy+1Lc1ebaUa1l7Vtg==@sourceware.org X-Gm-Message-State: AOJu0YwrZajVMZjxB/rRBtA8uLf05gycyIut9ft6n4OXvVKgX7lfAAWA B7Fp/DcDIKi9EnRTRFT2DWZ1kwBQ4TWjcXwtOHBPakmRMS89xLJGcqO/NT334B+uDTz7cul6xry lsXNsSXLwznKJ+i5q+0aBW9NzkNDiEuVQ0XM4h5K+YEwUmF/i2IwlOP8ZAZ37gUCNxOKLc6E= X-Gm-Gg: Acq92OG497brgfeT0SijTYkEFlI5ulMbXUhH0ZsmrMNryBnLcJDF+dkQZVvzW9BUZrL TMLOE8DuoTABLEV1CAu77BLZ10S41+s6QJDL+rhrTbmNdNzDO6StSdvTUyvDL1JDKMZhNsVtOTh +rMM4Jkxw8iUqqh7+8aagAQ95rHR7YJHoWhtYDottf0H1Vqiem8IFKqONkctiUOGey9tHxkDEkI fWW4fYyolFJ18MjYHI5fX8sx54c+PIEYpBerEuH4KrC5trdgH4To2k8wvJvnwSnn2pRRwfm5C/X fn4iDU8kDpeQscumKx2n5dixZYRGxTVaIVp4rOKcp1n61aRQKwTlbXZAVYardZXzS3iDMKHQb9A BZTowO9LOIA8n0DXbDtU= X-Received: by 2002:a05:600c:a309:b0:489:2005:b36e with SMTP id 5b1f17b1804b1-48fe61f2bedmr127650495e9.19.1779019569143; Sun, 17 May 2026 05:06:09 -0700 (PDT) X-Received: by 2002:a05:600c:a309:b0:489:2005:b36e with SMTP id 5b1f17b1804b1-48fe61f2bedmr127650155e9.19.1779019568686; Sun, 17 May 2026 05:06:08 -0700 (PDT) Received: from localhost ([109.144.213.219]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48fe5cab882sm191643175e9.13.2026.05.17.05.06.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 17 May 2026 05:06:08 -0700 (PDT) From: Andrew Burgess To: Tom Tromey , gdb-patches@sourceware.org Cc: Tom Tromey Subject: Re: [PATCH v2 2/4] Add wrappers for some Python APIs In-Reply-To: <20260515-python-safety-initial-v2-2-6129cadf258a@tromey.com> References: <20260515-python-safety-initial-v2-0-6129cadf258a@tromey.com> <20260515-python-safety-initial-v2-2-6129cadf258a@tromey.com> Date: Sun, 17 May 2026 13:06:06 +0100 Message-ID: <87a4tyckb5.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 83PbfKbmCZ-pBwr-OIJXxJQWtRrQZLdVVoOt5mYy-_o_1779019570 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 new functions that wrap Python APIs. The wrapping > follows some proposed rules for Python safety in gdb: > > * Functions returning a new reference return gdbpy_ref<> > > * Errors are reported via exceptions, not special values > > * Functions accepting a stolen reference take a gdbpy_ref<>&& > --- > gdb/python/py-wrappers.h | 334 +++++++++++++++++++++++++++++++++++++++++++ > gdb/python/python-internal.h | 2 + > 2 files changed, 336 insertions(+) > > diff --git a/gdb/python/py-wrappers.h b/gdb/python/py-wrappers.h > new file mode 100644 > index 00000000000..78c1f8070cf > --- /dev/null > +++ b/gdb/python/py-wrappers.h > @@ -0,0 +1,334 @@ > +/* 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_WRAPPERS_H > +#define GDB_PYTHON_PY_WRAPPERS_H > + > +#include "py-ref.h" > + > +/* Gdb implements its own wrappers for many Python APIs. This is done > + in an attempt to be more safe. > + > + In particular, in gdb: > + > + - APIs returning a new reference will return gdbpy_ref<>. This > + makes reference counting errors less likely. > + > + - APIs will throw an exception rather than return a special value > + (NULL or -1). This makes error checking simpler. > + > + - APIs requiring a stolen reference take a gdbpy_ref<>&&, to make > + reference counting errors less likely. > + > + This file holds the currently-defined wrappers. If new APIs are > + needed, the normal approach is to add a wrapper here. > + > + APIs here are named after the underlying Python function, but using > + lower case and an "_" at each word break. */ > + > +/* The type of exception thrown when the Python exception has been > + set. */ > +struct gdb_python_exception > +{ > + gdb_python_exception () > + { > + gdb_assert (PyErr_Occurred ()); > + } > +}; > + > +template > +gdbpy_ref > +gdbpy_new () > +{ > + gdbpy_ref result (PyObject_New (T, T::corresponding_object_type)); > + if (result == nullptr) > + throw gdb_python_exception (); > + return result; > +} > + > +static inline gdbpy_ref<> > +gdbpy_bool_from_long (long value) > +{ > + /* This cannot fail. */ > + return gdbpy_ref<> (PyBool_FromLong (value)); > +} > + > +static inline char * > +gdbpy_bytes_as_string (gdbpy_borrowed_ref ref) > +{ > + char *result = PyBytes_AsString (ref); > + if (result == nullptr) > + throw gdb_python_exception (); > + return result; > +} > + > +static inline void > +gdbpy_bytes_as_string_and_size (gdbpy_borrowed_ref ref, > + char **buffer, > + Py_ssize_t *length) > +{ > + if (PyBytes_AsStringAndSize (ref, buffer, length) == -1) > + throw gdb_python_exception (); > +} > + > +static inline gdbpy_ref<> > +gdbpy_bytes_from_string (const char *str) > +{ > + gdb_assert (str != nullptr); > + gdbpy_ref<> result (PyBytes_FromString (str)); > + if (result == nullptr) > + throw gdb_python_exception (); > + return result; > +} > + > +static inline gdbpy_ref<> > +gdbpy_bytes_from_string_and_size (const char *str, Py_ssize_t len) > +{ > + /* Python allows STR==nullptr but it leaves the object > + uninitialized, and I think we should avoid this in gdb. */ > + gdb_assert (str != nullptr); > + gdbpy_ref<> result (PyBytes_FromStringAndSize (str, len)); > + if (result == nullptr) > + throw gdb_python_exception (); > + return result; > +} > + > +static inline Py_ssize_t > +gdbpy_bytes_size (gdbpy_borrowed_ref ref) > +{ > + Py_ssize_t result = PyBytes_Size (ref); > + if (result == -1) > + throw gdb_python_exception (); > + return result; > +} > + > +static inline gdbpy_ref<> > +gdbpy_new_list (Py_ssize_t len) > +{ > + gdbpy_ref<> result (PyList_New (len)); > + if (result == nullptr) > + throw gdb_python_exception (); > + return result; > +} > + > +static inline void > +gdbpy_list_append (gdbpy_borrowed_ref list, gdbpy_borrowed_ref val) > +{ > + if (PyList_Append (list, val) < 0) > + throw gdb_python_exception (); > +} > + > +static inline gdbpy_ref<> > +gdbpy_new_dict () > +{ > + gdbpy_ref<> result (PyDict_New ()); > + if (result == nullptr) > + throw gdb_python_exception (); > + return result; > +} > + > +static inline void > +gdbpy_dict_set_item_string (gdbpy_borrowed_ref dict, > + const char *key, > + gdbpy_borrowed_ref value) > +{ > + if (PyDict_SetItemString (dict, key, value) != 0) > + throw gdb_python_exception (); > +} > + > +static inline void > +gdbpy_dict_del_item_string (gdbpy_borrowed_ref dict, > + const char *key) > +{ > + if (PyDict_DelItemString (dict, key) == -1) > + throw gdb_python_exception (); > +} > + > +static inline gdbpy_borrowed_ref > +gdbpy_dict_get_item_with_error (gdbpy_borrowed_ref dict, > + gdbpy_borrowed_ref key) > +{ > + PyObject *result = PyDict_GetItemWithError (dict, key); > + if (result == nullptr) > + throw gdb_python_exception (); > + return result; PyDict_GetItemWithError will return NULL *without* an exception set if KEY is not found in DICT. This will then trigger an assert in the gdb_python_exception constructor. Because of the *cough* lack of comments, it's unclear what the function's intended behaviour is, but this feels like a bug. > +} > + > +static inline gdbpy_ref<> > +gdbpy_dict_keys (gdbpy_borrowed_ref dict) > +{ > + gdbpy_ref<> result (PyDict_Keys (dict)); > + if (result == nullptr) > + throw gdb_python_exception (); > + return result; > +} > + > +static inline gdbpy_ref<> > +gdbpy_unicode_from_string (std::string_view str) > +{ > + gdbpy_ref<> result (PyUnicode_FromStringAndSize (str.data (), str.size ())); > + if (result == nullptr) > + throw gdb_python_exception (); > + return result; > +} > + > +static inline gdbpy_ref<> > +gdbpy_unicode_from_format (const char *fmt, ...) > +{ > + va_list args; > + va_start (args, fmt); > + gdbpy_ref<> result (PyUnicode_FromFormatV (fmt, args)); > + if (result == nullptr) > + throw gdb_python_exception (); > + va_end (args); If an exception is thrown in this case then we fail to call va_end, which I believe is technically undefined behaviour (though in reality it's probably harmless), still, we should fix this. > + return result; > +} > + > +[[noreturn]] static inline void > +gdbpy_err_set_string (gdbpy_borrowed_ref type, const char *str) > +{ > + PyErr_SetString (type, str); > + throw gdb_python_exception (); > +} > + > +/* This is a template because PyErr_FormatV was only added in Python > + 3.5. */ > +template > +[[noreturn]] void > +gdbpy_err_format (gdbpy_borrowed_ref type, const char *fmt, Arg... args) > +{ > + PyErr_Format (type, fmt, std::forward (args)...); This isn't really a request for a change, more a question for my own education. My understanding is that std::forward is intended to be used with forwarding reference arguments, which ARGS is not. So in this case ARGS will have been captured by-value, so there is no difference between what you have now and: PyErr_Format (type, fmt, args...); Am I correct here? Or is std::forward doing something that I'm not understanding? Or did you mean to capture ARGS as 'Arg&&... args', in which case std::forward would be needed / useful? I had a look and there are actually loads of places in GDB where we use std::forward on arguments that are captured by-value, so my assumption here is that I'm not understanding this topic fully, but if I don't ask, I'll never know. > + throw gdb_python_exception (); > +} > + > +template > +void > +gdbpy_arg_parse_tuple_and_keywords (gdbpy_borrowed_ref args, > + gdbpy_opt_borrowed_ref kw, > + const char *fmt, > + const char **keywords, > + Arg... outputs) > +{ > + /* It would be cool if callers could use references to the > + out-parameters and also if gdbpy_borrowed_ref could be used for > + those. That requires some hairy template metaprogramming > + though. */ > + if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, fmt, keywords, > + std::forward (outputs)...)) Here's another place where std::forward is used on by-value OUTPUTS. > + throw gdb_python_exception (); > +} > + > +static inline void > +gdbpy_arg_parse_tuple (gdbpy_borrowed_ref param, const char *format, ...) > +{ > + va_list args; > + va_start (args, format); > + if (!PyArg_VaParse (param, format, args)) > + throw gdb_python_exception (); > + va_end (args); Exception causes va_end to be skipped again. > +} > + Thanks, Andrew