Skip to content

Commit

Permalink
Issue #608: Turn accessor macros in objc-object.h into functions
Browse files Browse the repository at this point in the history 
This is a first step in making objc-object.m safe to use
in a free-threaded build.

The change to method-accessor.m fixes a real bug: a PyObjCClass
instance was used as a PyObjCObject one. This happens to work
due to both having the Objective-C value as their first field,
but is undefined behaviour in C.
  • Loading branch information
@ronaldoussoren
ronaldoussoren committed Dec 8, 2024
1 parent 5909ad0 commit 2f69116
Show file tree
Hide file tree
Showing 8 changed files with 253 additions and 156 deletions.
184 changes: 108 additions & 76 deletions pyobjc-core/Modules/objc/libffi_support.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1475,34 +1475,43 @@ int PyObjCFFI_Setup(PyObject* m __attribute__((__unused__)))
default:
v = pythonify_c_value(argtype, args[i]);

if (v != NULL && unlikely(PyObjCObject_IsBlock(v) && PyObjCObject_GetBlock(v) == NULL)) {
/* Value is an (Objective-)C block for which we don't have a Python
* signature
*
* 1) Try to extract from the metadata system
* 2) Try to extract from the ObjC runtime
*
* Both systems may not have the required information.
*/
if (v != NULL && unlikely(PyObjCObject_Check(v) && PyObjCObject_IsBlock(v))) {
PyObjCMethodSignature* block_methinfo = PyObjCObject_GetBlockSignature(v);
if (unlikely(block_methinfo == NULL)) {
/* Value is an (Objective-)C block for which we don't have a Python
* signature
*
* 1) Try to extract from the metadata system
* 2) Try to extract from the ObjC runtime
*
* Both systems may not have the required information.
*/

if (methinfo->argtype[i]->callable != NULL) {
PyObjCObject_SET_BLOCK(v, methinfo->argtype[i]->callable);
Py_INCREF(methinfo->argtype[i]->callable);
if (methinfo->argtype[i]->callable != NULL) {
if (PyObjCObject_SetBlockSignature(v, methinfo->argtype[i]->callable) == -1) {
goto error;
}

} else {
const char* signature = PyObjCBlock_GetSignature(v);
if (signature != NULL) {
PyObjCMethodSignature* sig =
PyObjCMethodSignature_WithMetaData(signature, NULL, YES);
} else {
const char* signature = PyObjCBlock_GetSignature(v);
if (signature != NULL) {
PyObjCMethodSignature* sig =
PyObjCMethodSignature_WithMetaData(signature, NULL, YES);

if (sig == NULL) {
Py_DECREF(v);
v = NULL;
} else {
PyObjCObject_SET_BLOCK(v, sig);
sig = NULL;
if (sig == NULL) {
Py_DECREF(v);
v = NULL;
} else {
if (PyObjCObject_SetBlockSignature(v, sig) == -1) {
Py_DECREF(sig);
goto error;
}
Py_DECREF(sig);
}
}
}
} else {
Py_DECREF(block_methinfo);
}
}
}
Expand Down Expand Up @@ -3900,33 +3909,42 @@ Py_ssize_t argbuf_len __attribute__((__unused__)), /* only used in debug builds
return NULL;
}

if (PyObjCObject_IsBlock(objc_result)
&& PyObjCObject_GetBlock(objc_result) == NULL) {
/* Result is an (Objective-)C block for which we don't have a Python
* signature
*
* 1) Try to extract from the metadata system
* 2) Try to extract from the ObjC runtime
*
* Both systems may not have the required information.
*/

if (methinfo->rettype->callable != NULL) {
PyObjCObject_SET_BLOCK(objc_result, methinfo->rettype->callable);
Py_INCREF(methinfo->rettype->callable);
} else {
const char* signature = PyObjCBlock_GetSignature(objc_result);
if (signature != NULL) {
PyObjCMethodSignature* sig =
PyObjCMethodSignature_WithMetaData(signature, NULL, YES);
if (PyObjCObject_Check(objc_result) && PyObjCObject_IsBlock(objc_result)) {
PyObjCMethodSignature* block_methinfo = PyObjCObject_GetBlockSignature(objc_result);
if (block_methinfo == NULL) {
/* Result is an (Objective-)C block for which we don't have a Python
* signature
*
* 1) Try to extract from the metadata system
* 2) Try to extract from the ObjC runtime
*
* Both systems may not have the required information.
*/

if (sig == NULL) {
Py_DECREF(objc_result);
if (methinfo->rettype->callable != NULL) {
if (PyObjCObject_SetBlockSignature(objc_result, methinfo->rettype->callable) == -1) {
return NULL;
}
PyObjCObject_SET_BLOCK(objc_result, sig);
sig = NULL;
} else {
const char* signature = PyObjCBlock_GetSignature(objc_result);
if (signature != NULL) {
PyObjCMethodSignature* sig =
PyObjCMethodSignature_WithMetaData(signature, NULL, YES);

if (sig == NULL) {
Py_DECREF(objc_result);
return NULL;
}
if (PyObjCObject_SetBlockSignature(objc_result, sig) == -1) {
Py_DECREF(objc_result);
Py_DECREF(sig);
return NULL;
}
Py_CLEAR(sig);
}
}
} else {
Py_DECREF(block_methinfo);
}
}
} else {
Expand Down Expand Up @@ -4035,11 +4053,15 @@ Py_ssize_t argbuf_len __attribute__((__unused__)), /* only used in debug builds
[tmp release];

if (v != NULL && methinfo->argtype[i]->callable != NULL) {
if (PyObjCObject_IsBlock(v)
&& PyObjCObject_GetBlock(v) == NULL) {
PyObjCObject_SET_BLOCK(
v, methinfo->argtype[i]->callable);
Py_INCREF(methinfo->argtype[i]->callable);
if (PyObjCObject_Check(v) && PyObjCObject_IsBlock(v)) {
PyObjCMethodSignature* methinfo = PyObjCObject_GetBlockSignature(v);
if (methinfo == NULL) {
if (PyObjCObject_SetBlockSignature( v, methinfo->argtype[i]->callable) == -1) {
goto error_cleanup;
}
} else {
Py_DECREF(methinfo);
}
}
}
} else {
Expand Down Expand Up @@ -4221,36 +4243,46 @@ Py_ssize_t argbuf_len __attribute__((__unused__)), /* only used in debug builds
return NULL;
}

if (PyObjCObject_IsBlock(objc_result)
&& PyObjCObject_GetBlock(objc_result) == NULL) {
/* Result is an (Objective-)C block for which we don't have a Python
* signature
*
* 1) Try to extract from the metadata system
* 2) Try to extract from the ObjC runtime
*
* Both systems may not have the required information.
*
* XXX: Move to separate function!
*/

if (methinfo->rettype->callable != NULL) {
PyObjCObject_SET_BLOCK(objc_result, methinfo->rettype->callable);
Py_INCREF(methinfo->rettype->callable);
} else {
const char* signature = PyObjCBlock_GetSignature(objc_result);
if (signature != NULL) {
PyObjCMethodSignature* sig =
PyObjCMethodSignature_WithMetaData(signature, NULL, YES);
if (PyObjCObject_Check(objc_result) && PyObjCObject_IsBlock(objc_result)) {
PyObjCMethodSignature* block_methinfo = PyObjCObject_GetBlockSignature(objc_result);
if (block_methinfo == NULL) {
/* Result is an (Objective-)C block for which we don't have a Python
* signature
*
* 1) Try to extract from the metadata system
* 2) Try to extract from the ObjC runtime
*
* Both systems may not have the required information.
*
* XXX: Move to separate function!
*/

if (sig == NULL) {
Py_DECREF(objc_result);
if (methinfo->rettype->callable != NULL) {
if (PyObjCObject_SetBlockSignature(objc_result, methinfo->rettype->callable) == -1) {
return NULL;
}
PyObjCObject_SET_BLOCK(objc_result, sig);
sig = NULL;
} else {
const char* signature = PyObjCBlock_GetSignature(objc_result);
if (signature != NULL) {
PyObjCMethodSignature* sig =
PyObjCMethodSignature_WithMetaData(signature, NULL, YES);

if (sig == NULL) {
Py_DECREF(objc_result);
return NULL;
}
if (PyObjCObject_SetBlockSignature(objc_result, sig) == -1) {
Py_DECREF(objc_result);
Py_DECREF(sig);
return NULL;
}
Py_CLEAR(sig);
}
}
} else {
Py_DECREF(block_methinfo);
}

}
} else {

Expand Down
16 changes: 15 additions & 1 deletion pyobjc-core/Modules/objc/method-accessor.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,21 @@
} else {
IMP sel_imp =
[PyObjCSelector_GetClass(meta) instanceMethodForSelector:sel];
IMP cur_imp = [PyObjCObject_GetObject(self) methodForSelector:sel];
IMP cur_imp;

/* XXX: The typecheck here is necessary because some callers pass a
* class, I'm not yet sure this is correct. Found by stricter
* checks in PyObjCObject_GetObject.
*/
if (PyObjCObject_Check(self)) {
cur_imp = [PyObjCObject_GetObject(self) methodForSelector:sel];
} else if (PyObjCClass_Check(self)) {
cur_imp = [PyObjCClass_GetClass(self) methodForSelector:sel];
} else {
PyErr_SetString(PyObjCExc_Error, "Unsupported case for find_selector");
return NULL;
}

if (sel_imp == cur_imp) {
return meta;
} else {
Expand Down
50 changes: 24 additions & 26 deletions pyobjc-core/Modules/objc/objc-object.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,32 +27,30 @@ typedef struct {
extern PyObjCClassObject PyObjCObject_Type;
#define PyObjCObject_Check(obj) PyObject_TypeCheck(obj, (PyTypeObject*)&PyObjCObject_Type)

PyObject* _Nullable PyObjCObject_New(id objc_object, int flags, int retain);
PyObject* _Nullable PyObjCObject_FindSelector(PyObject* cls, SEL selector);
id _Nullable PyObjCObject_GetObject(PyObject* object);
void PyObjCObject_ClearObject(PyObject* object);

/* XXX: Is the macro still needed with LTO? */
#define PyObjCObject_GetObject(object) (((PyObjCObject*)(object))->objc_object)

void _PyObjCObject_FreeDeallocHelper(PyObject* obj);
PyObject* _Nullable _PyObjCObject_NewDeallocHelper(id objc_object);

#define PyObjCObject_GetFlags(object) (((PyObjCObject*)(object))->flags)
#define PyObjCObject_IsBlock(object) (PyObjCObject_GetFlags(object) & PyObjCObject_kBLOCK)
#define PyObjCObject_IsMagic(object) \
(PyObjCObject_GetFlags(object) & PyObjCObject_kMAGIC_COOKIE)
#define PyObjCObject_GetBlock(object) (((PyObjCBlockObject*)(object))->signature)
#define PyObjCObject_SET_BLOCK(object, value) \
(((PyObjCBlockObject*)(object))->signature = (value))

PyObject* _Nullable PyObjCObject_GetAttr(PyObject* object, PyObject* key);
PyObject* _Nullable PyObjCObject_GetAttrString(PyObject* object, char* key);
PyObject* _Nullable PyObjCObject_NewTransient(id objc_object, int* cookie);
void PyObjCObject_ReleaseTransient(PyObject* proxy, int cookie);

/* XXX: Move to different file */
PyObject* _Nullable PyObjC_get_c_void_p(void);
extern PyObject* _Nullable PyObjCObject_New(id objc_object, int flags, int retain);
extern PyObject* _Nullable PyObjCObject_FindSelector(PyObject* cls, SEL selector);
extern id _Nullable PyObjCObject_GetObject(PyObject* object);
extern unsigned int PyObjCObject_GetFlags(PyObject* object);

extern void PyObjCObject_ClearObject(PyObject* object);

extern void _PyObjCObject_FreeDeallocHelper(PyObject* obj);
extern PyObject* _Nullable _PyObjCObject_NewDeallocHelper(id objc_object);
extern bool PyObjCObject_IsBlock(PyObject* object);
extern bool PyObjCObject_IsMagic(PyObject* object);
extern PyObjCMethodSignature* _Nullable PyObjCObject_GetBlockSignature(PyObject* object);
extern int PyObjCObject_SetBlockSignature(PyObject* object, PyObjCMethodSignature* methinfo);

/*
* XXX: these defines should be in the .m file
*/
#define PyObjCObject_FLAGS(object) (((PyObjCObject*)(object))->flags)
#define PyObjCObject_OBJECT(object) (((PyObjCObject*)(object))->objc_object)

extern PyObject* _Nullable PyObjCObject_GetAttr(PyObject* object, PyObject* key);
extern PyObject* _Nullable PyObjCObject_GetAttrString(PyObject* object, char* key);
extern PyObject* _Nullable PyObjCObject_NewTransient(id objc_object, int* cookie);
extern void PyObjCObject_ReleaseTransient(PyObject* proxy, int cookie);

NS_ASSUME_NONNULL_END

Expand Down
Loading

0 comments on commit 2f69116

Please sign in to comment.