Skip to content

gh-94731: Revert to C-style casts for _Py_CAST #94782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 3 additions & 51 deletions Include/pyport.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<type>. 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 <cstddef>
# define _Py_STATIC_CAST(type, expr) static_cast<type>(expr)
extern "C++" {
namespace {
template <typename type>
inline type _Py_CAST_impl(long int ptr) {
return reinterpret_cast<type>(ptr);
}
template <typename type>
inline type _Py_CAST_impl(int ptr) {
return reinterpret_cast<type>(ptr);
}
#if __cplusplus >= 201103
template <typename type>
inline type _Py_CAST_impl(std::nullptr_t) {
return static_cast<type>(nullptr);
}
#endif

template <typename type, typename expr_type>
inline type _Py_CAST_impl(expr_type *expr) {
return reinterpret_cast<type>(expr);
}

template <typename type, typename expr_type>
inline type _Py_CAST_impl(expr_type const *expr) {
return reinterpret_cast<type>(const_cast<expr_type *>(expr));
}

template <typename type, typename expr_type>
inline type _Py_CAST_impl(expr_type &expr) {
return static_cast<type>(expr);
}

template <typename type, typename expr_type>
inline type _Py_CAST_impl(expr_type const &expr) {
return static_cast<type>(const_cast<expr_type &>(expr));
}
}
}
# define _Py_CAST(type, expr) _Py_CAST_impl<type>(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
Expand Down
76 changes: 73 additions & 3 deletions Lib/test/_testcppext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<VirtualPyObject*>(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.

Expand All @@ -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;
}

Expand All @@ -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
Expand Down
5 changes: 0 additions & 5 deletions Lib/test/setup_testcppext.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
6 changes: 6 additions & 0 deletions Lib/test/test_cppext.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import sys
import unittest
import subprocess
import sysconfig
from test import support
from test.support import os_helper

Expand All @@ -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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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++``.