From 6cbb57f62d345d7a5d6aeb1b3b5d37a845344d5e Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 14 Jul 2022 11:57:18 +0200 Subject: [PATCH] gh-94731: Revert to C-style casts for _Py_CAST (GH-94782) Co-authored-by: da-woods --- Include/pyport.h | 54 +------------ Lib/test/_testcppext.cpp | 76 ++++++++++++++++++- Lib/test/setup_testcppext.py | 5 -- Lib/test/test_cppext.py | 6 ++ ...2-07-12-17-39-32.gh-issue-94731.9CPJNU.rst | 3 + 5 files changed, 85 insertions(+), 59 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2022-07-12-17-39-32.gh-issue-94731.9CPJNU.rst diff --git a/Include/pyport.h b/Include/pyport.h index 313bc8d21c83fd..b3ff2f4882e90f 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -14,62 +14,14 @@ #endif -// Macro to use C++ static_cast<>, reinterpret_cast<> and const_cast<> -// in the Python C API. -// -// In C++, _Py_CAST(type, expr) converts a constant expression to a -// non constant type using const_cast. For example, -// _Py_CAST(PyObject*, op) can convert a "const PyObject*" to -// "PyObject*". -// -// The type argument must not be a constant type. +// Macro to use C++ static_cast<> in the Python C API. #ifdef __cplusplus -#include # define _Py_STATIC_CAST(type, expr) static_cast(expr) -extern "C++" { - namespace { - template - inline type _Py_CAST_impl(long int ptr) { - return reinterpret_cast(ptr); - } - template - inline type _Py_CAST_impl(int ptr) { - return reinterpret_cast(ptr); - } -#if __cplusplus >= 201103 - template - inline type _Py_CAST_impl(std::nullptr_t) { - return static_cast(nullptr); - } -#endif - - template - inline type _Py_CAST_impl(expr_type *expr) { - return reinterpret_cast(expr); - } - - template - inline type _Py_CAST_impl(expr_type const *expr) { - return reinterpret_cast(const_cast(expr)); - } - - template - inline type _Py_CAST_impl(expr_type &expr) { - return static_cast(expr); - } - - template - inline type _Py_CAST_impl(expr_type const &expr) { - return static_cast(const_cast(expr)); - } - } -} -# define _Py_CAST(type, expr) _Py_CAST_impl(expr) - #else # define _Py_STATIC_CAST(type, expr) ((type)(expr)) -# define _Py_CAST(type, expr) ((type)(expr)) #endif +// Macro to use the more powerful/dangerous C-style cast even in C++. +#define _Py_CAST(type, expr) ((type)(expr)) // Static inline functions should use _Py_NULL rather than using directly NULL // to prevent C++ compiler warnings. On C++11 and newer, _Py_NULL is defined as diff --git a/Lib/test/_testcppext.cpp b/Lib/test/_testcppext.cpp index be7538826b6c4b..0e381a78c5ceed 100644 --- a/Lib/test/_testcppext.cpp +++ b/Lib/test/_testcppext.cpp @@ -12,6 +12,9 @@ # define NAME _testcpp03ext #endif +#define _STR(NAME) #NAME +#define STR(NAME) _STR(NAME) + PyDoc_STRVAR(_testcppext_add_doc, "add(x, y)\n" "\n" @@ -123,11 +126,77 @@ test_unicode(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) Py_RETURN_NONE; } +/* Test a `new`-allocated object with a virtual method. + * (https://github.com/python/cpython/issues/94731) */ + +class VirtualPyObject : public PyObject { +public: + VirtualPyObject(); + virtual ~VirtualPyObject() { + delete [] internal_data; + --instance_count; + } + virtual void set_internal_data() { + internal_data[0] = 1; + } + static void dealloc(PyObject* o) { + delete static_cast(o); + } + + // Number of "living" instances + static int instance_count; +private: + // buffer that can get corrupted + int* internal_data; +}; + +int VirtualPyObject::instance_count = 0; + +PyType_Slot VirtualPyObject_Slots[] = { + {Py_tp_free, (void*)VirtualPyObject::dealloc}, + {0, _Py_NULL}, +}; + +PyType_Spec VirtualPyObject_Spec = { + /* .name */ STR(NAME) ".VirtualPyObject", + /* .basicsize */ sizeof(VirtualPyObject), + /* .itemsize */ 0, + /* .flags */ Py_TPFLAGS_DEFAULT, + /* .slots */ VirtualPyObject_Slots, +}; + +VirtualPyObject::VirtualPyObject() { + // Create a temporary type (just so we don't need to store it) + PyObject *type = PyType_FromSpec(&VirtualPyObject_Spec); + // no good way to signal failure from a C++ constructor, so use assert + // for error handling + assert(type); + assert(PyObject_Init(this, (PyTypeObject *)type)); + Py_DECREF(type); + internal_data = new int[50]; + ++instance_count; +} + +static PyObject * +test_virtual_object(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) +{ + VirtualPyObject* obj = new VirtualPyObject(); + obj->set_internal_data(); + Py_DECREF(obj); + if (VirtualPyObject::instance_count != 0) { + return PyErr_Format( + PyExc_AssertionError, + "instance_count should be 0, got %d", + VirtualPyObject::instance_count); + } + Py_RETURN_NONE; +} static PyMethodDef _testcppext_methods[] = { {"add", _testcppext_add, METH_VARARGS, _testcppext_add_doc}, {"test_api_casts", test_api_casts, METH_NOARGS, _Py_NULL}, {"test_unicode", test_unicode, METH_NOARGS, _Py_NULL}, + {"test_virtual_object", test_virtual_object, METH_NOARGS, _Py_NULL}, // Note: _testcppext_exec currently runs all test functions directly. // When adding a new one, add a call there. @@ -152,6 +221,10 @@ _testcppext_exec(PyObject *module) if (!result) return -1; Py_DECREF(result); + result = PyObject_CallMethod(module, "test_virtual_object", ""); + if (!result) return -1; + Py_DECREF(result); + return 0; } @@ -163,9 +236,6 @@ static PyModuleDef_Slot _testcppext_slots[] = { PyDoc_STRVAR(_testcppext_doc, "C++ test extension."); -#define _STR(NAME) #NAME -#define STR(NAME) _STR(NAME) - static struct PyModuleDef _testcppext_module = { PyModuleDef_HEAD_INIT, // m_base STR(NAME), // m_name diff --git a/Lib/test/setup_testcppext.py b/Lib/test/setup_testcppext.py index ae81e33e23b6bd..c6b68104d1333c 100644 --- a/Lib/test/setup_testcppext.py +++ b/Lib/test/setup_testcppext.py @@ -17,8 +17,6 @@ # a C++ extension using the Python C API does not emit C++ compiler # warnings '-Werror', - # Warn on old-style cast (C cast) like: (PyObject*)op - '-Wold-style-cast', ] else: # Don't pass any compiler flag to MSVC @@ -37,9 +35,6 @@ def main(): name = '_testcpp11ext' cppflags = [*CPPFLAGS, f'-std={std}'] - if std == 'c++11': - # Warn when using NULL rather than _Py_NULL in static inline functions - cppflags.append('-Wzero-as-null-pointer-constant') cpp_ext = Extension( name, diff --git a/Lib/test/test_cppext.py b/Lib/test/test_cppext.py index 2c54d337116911..465894d24e7dfc 100644 --- a/Lib/test/test_cppext.py +++ b/Lib/test/test_cppext.py @@ -4,6 +4,7 @@ import sys import unittest import subprocess +import sysconfig from test import support from test.support import os_helper @@ -25,6 +26,11 @@ def test_build_cpp03(self): # With MSVC, the linker fails with: cannot open file 'python311.lib' # https://github.com/python/cpython/pull/32175#issuecomment-1111175897 @unittest.skipIf(MS_WINDOWS, 'test fails on Windows') + # Building and running an extension in clang sanitizing mode is not + # straightforward + @unittest.skipIf( + '-fsanitize' in (sysconfig.get_config_var('PY_CFLAGS') or ''), + 'test does not work with analyzing builds') # the test uses venv+pip: skip if it's not available @support.requires_venv_with_pip() def check_build(self, std_cpp03, extension_name): diff --git a/Misc/NEWS.d/next/C API/2022-07-12-17-39-32.gh-issue-94731.9CPJNU.rst b/Misc/NEWS.d/next/C API/2022-07-12-17-39-32.gh-issue-94731.9CPJNU.rst new file mode 100644 index 00000000000000..b7816d28215e7b --- /dev/null +++ b/Misc/NEWS.d/next/C API/2022-07-12-17-39-32.gh-issue-94731.9CPJNU.rst @@ -0,0 +1,3 @@ +Python again uses C-style casts for most casting operations when compiled +with C++. This may trigger compiler warnings, if they are enabled with e.g. +``-Wold-style-cast `` or ``-Wzero-as-null-pointer-constant`` options for ``g++``.