From 111fb4b6d617e63f5ad3cc2419f66d9d16c79ec4 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 15 Oct 2024 16:23:45 -0400 Subject: [PATCH 01/22] Setup build. --- Include/internal/pycore_dynarray.h | 29 +++++++++++++++++++++++++++++ Makefile.pre.in | 2 ++ Python/dynarray.c | 5 +++++ 3 files changed, 36 insertions(+) create mode 100644 Include/internal/pycore_dynarray.h create mode 100644 Python/dynarray.c diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h new file mode 100644 index 00000000000000..b611c2095b2dd5 --- /dev/null +++ b/Include/internal/pycore_dynarray.h @@ -0,0 +1,29 @@ +#ifndef Py_INTERNAL_DYNARRAY_H +#define Py_INTERNAL_DYNARRAY_H +#ifdef __cplusplus +extern "C" { +#endif + +#include "Python.h" // Py_ssize_t + +#ifndef Py_BUILD_CORE +# error "this header requires Py_BUILD_CORE define" +#endif + +typedef void (*_Py_dynarray_deallocator)(void *); + +typedef struct { + _Py_dynarray_deallocator deallocator; + void *contents; +} _Py_dynarray_item; + +typedef struct { + _Py_dynarray_item **items; + Py_ssize_t capacity; + Py_ssize_t length; +} _Py_dynarray_t; + +#ifdef __cplusplus +} +#endif +#endif /* !Py_INTERNAL_DYNARRAY_H */ diff --git a/Makefile.pre.in b/Makefile.pre.in index 07c8a4d20142db..1b0f12840f2773 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -435,6 +435,7 @@ PYTHON_OBJS= \ Python/critical_section.o \ Python/crossinterp.o \ Python/dynamic_annotations.o \ + Python/dynarray.o \ Python/errors.o \ Python/flowgraph.o \ Python/frame.o \ @@ -1197,6 +1198,7 @@ PYTHON_HEADERS= \ $(srcdir)/Include/internal/pycore_dict.h \ $(srcdir)/Include/internal/pycore_dict_state.h \ $(srcdir)/Include/internal/pycore_dtoa.h \ + $(srcdir)/Include/internal/pycore_dynarray.h \ $(srcdir)/Include/internal/pycore_exceptions.h \ $(srcdir)/Include/internal/pycore_faulthandler.h \ $(srcdir)/Include/internal/pycore_fileutils.h \ diff --git a/Python/dynarray.c b/Python/dynarray.c new file mode 100644 index 00000000000000..e569fb48c66cd2 --- /dev/null +++ b/Python/dynarray.c @@ -0,0 +1,5 @@ +/* + * Dynamic array implementation + */ + +#include "pycore_dynarray.h" From 6e305d18799c81975cba9d303d37dd1a0af59af1 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 15 Oct 2024 16:43:57 -0400 Subject: [PATCH 02/22] Simple implementation. --- Include/internal/pycore_dynarray.h | 12 ++---- Python/dynarray.c | 64 ++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index b611c2095b2dd5..f3ac7e0565c3a1 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -10,18 +10,14 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif -typedef void (*_Py_dynarray_deallocator)(void *); +typedef void (*_PyDynArray_Deallocator)(void *); typedef struct { - _Py_dynarray_deallocator deallocator; - void *contents; -} _Py_dynarray_item; - -typedef struct { - _Py_dynarray_item **items; + void **items; Py_ssize_t capacity; Py_ssize_t length; -} _Py_dynarray_t; + _PyDynArray_Deallocator deallocator; +} _PyDynArray; #ifdef __cplusplus } diff --git a/Python/dynarray.c b/Python/dynarray.c index e569fb48c66cd2..3b9a97fd78d916 100644 --- a/Python/dynarray.c +++ b/Python/dynarray.c @@ -3,3 +3,67 @@ */ #include "pycore_dynarray.h" + +_PyDynArray * +_PyDynArray_InitWithSize(_PyDynArray *array, + Py_ssize_t initial, + _PyDynArray_Deallocator deallocator) +{ + assert(array != NULL); + assert(initial > 0); + void **items = PyMem_RawCalloc(sizeof(void *), initial); + if (items == NULL) + { + PyMem_RawFree(array); + return NULL; + } + + array->capacity = initial; + array->items = items; + array->length = 0; + array->deallocator = deallocator; + + return array; +} + +int +_PyDynArray_Append(_PyDynArray *array, void *item) +{ + assert(array != NULL); + array->items[array->length++] = item; + if (array->length == array->capacity) + { + // Need to resize + array->capacity *= 2; + void **new_items = PyMem_RawRealloc( + array->items, + sizeof(void *) * array->capacity); + if (new_items == NULL) + { + --array->length; + return -1; + } + + array->items = new_items; + } + return 0; +} + +void +_PyDynArray_Clear(_PyDynArray *array) +{ + assert(array != NULL); + assert(array->deallocator != NULL); + for (Py_ssize_t i = 0; i < array->length; ++i) + { + array->deallocator(array->items[i]); + } + PyMem_RawFree(array->items); + + // It would be nice if others could reuse the allocation for another + // dynarray later, so clear all the fields. + array->items = NULL; + array->length = 0; + array->capacity = 0; + array->deallocator = NULL; +} From 210d4d286246a5de83e79d22205bf577dbf5fccf Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 15 Oct 2024 16:45:09 -0400 Subject: [PATCH 03/22] Add to header file. --- Include/internal/pycore_dynarray.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index f3ac7e0565c3a1..e98a5b0c3f897b 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -19,6 +19,17 @@ typedef struct { _PyDynArray_Deallocator deallocator; } _PyDynArray; +PyAPI_FUNC(_PyDynArray *) +_PyDynArray_InitWithSize(_PyDynArray *array, + Py_ssize_t initial, + _PyDynArray_Deallocator deallocator); + +int +_PyDynArray_Append(_PyDynArray *array, void *item); + +void +_PyDynArray_Clear(_PyDynArray *array); + #ifdef __cplusplus } #endif From 2c77397217b1372b9fa53a344ce387ee87fb449f Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 15 Oct 2024 16:53:21 -0400 Subject: [PATCH 04/22] Add convenience macros. --- Include/internal/pycore_dynarray.h | 51 ++++++++++++++++++++++++++---- Python/dynarray.c | 16 ++++++---- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index e98a5b0c3f897b..142e9d1233c10a 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -10,6 +10,8 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif +#define _PyDynArray_DEFAULT_SIZE 16 + typedef void (*_PyDynArray_Deallocator)(void *); typedef struct { @@ -19,16 +21,51 @@ typedef struct { _PyDynArray_Deallocator deallocator; } _PyDynArray; -PyAPI_FUNC(_PyDynArray *) +PyAPI_FUNC(int) _PyDynArray_InitWithSize(_PyDynArray *array, - Py_ssize_t initial, - _PyDynArray_Deallocator deallocator); + _PyDynArray_Deallocator deallocator, + Py_ssize_t initial); + +PyAPI_FUNC(int) _PyDynArray_Append(_PyDynArray *array, void *item); + +PyAPI_FUNC(void) _PyDynArray_Clear(_PyDynArray *array); -int -_PyDynArray_Append(_PyDynArray *array, void *item); +static inline void +_PyDynArray_Free(_PyDynArray *array) +{ + _PyDynArray_Clear(array); + PyMem_RawFree(array); +} + +static inline int +_PyDynArray_Init(_PyDynArray *array, _PyDynArray_Deallocator deallocator) +{ + return _PyDynArray_InitWithSize(array, deallocator, _PyDynArray_DEFAULT_SIZE); +} -void -_PyDynArray_Clear(_PyDynArray *array); +static inline _PyDynArray * +_PyDynArray_NewWithSize(_PyDynArray_Deallocator deallocator, Py_ssize_t initial) +{ + _PyDynArray *array = PyMem_RawMalloc(sizeof(_PyDynArray)); + if (array == NULL) + { + return NULL; + } + + if (_PyDynArray_InitWithSize(array, deallocator, initial) < 0) + { + PyMem_RawFree(array); + return NULL; + } + + return array; +} + +static inline _PyDynArray * +_PyDynArray_New(_PyDynArray_Deallocator deallocator) +{ + return _PyDynArray_NewWithSize(deallocator, _PyDynArray_DEFAULT_SIZE); +} #ifdef __cplusplus } diff --git a/Python/dynarray.c b/Python/dynarray.c index 3b9a97fd78d916..b572acbbdb83e4 100644 --- a/Python/dynarray.c +++ b/Python/dynarray.c @@ -4,18 +4,17 @@ #include "pycore_dynarray.h" -_PyDynArray * +int _PyDynArray_InitWithSize(_PyDynArray *array, - Py_ssize_t initial, - _PyDynArray_Deallocator deallocator) + _PyDynArray_Deallocator deallocator, + Py_ssize_t initial) { assert(array != NULL); assert(initial > 0); void **items = PyMem_RawCalloc(sizeof(void *), initial); if (items == NULL) { - PyMem_RawFree(array); - return NULL; + return -1; } array->capacity = initial; @@ -23,7 +22,7 @@ _PyDynArray_InitWithSize(_PyDynArray *array, array->length = 0; array->deallocator = deallocator; - return array; + return 0; } int @@ -56,7 +55,10 @@ _PyDynArray_Clear(_PyDynArray *array) assert(array->deallocator != NULL); for (Py_ssize_t i = 0; i < array->length; ++i) { - array->deallocator(array->items[i]); + if (array->deallocator != NULL) + { + array->deallocator(array->items[i]); + } } PyMem_RawFree(array->items); From d90329a0ecd7173c03e0a3f484dcc35763a5e155 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 15 Oct 2024 17:06:13 -0400 Subject: [PATCH 05/22] Add docstrings and some sanity checks. --- Include/internal/pycore_dynarray.h | 67 ++++++++++++++++++++++++++++++ Python/dynarray.c | 5 +-- 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index 142e9d1233c10a..9a0f01c9f4a8ea 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -21,28 +21,70 @@ typedef struct { _PyDynArray_Deallocator deallocator; } _PyDynArray; +static inline void +_PyDynArray_ASSERT_VALID(_PyDynArray *array) +{ + assert(array != NULL); + assert(array->items != NULL); +} + +/* + * Initialize a dynamic array with an initial size. + * + * Returns -1 upon failure, 0 otherwise. + */ PyAPI_FUNC(int) _PyDynArray_InitWithSize(_PyDynArray *array, _PyDynArray_Deallocator deallocator, Py_ssize_t initial); +/* + * Append to a dynamic array. + * + * Returns -1 upon failure, 0 otherwise. + * If this fails, the deallocator is not ran on *item*. + */ PyAPI_FUNC(int) _PyDynArray_Append(_PyDynArray *array, void *item); +/* + * Clear all the fields on a dynamic array. + * + * Note that this does *not* free the actual dynamic array + * structure--use _PyDynArray_Free() for that. + */ PyAPI_FUNC(void) _PyDynArray_Clear(_PyDynArray *array); +/* + * Clear all the fields on a dynamic array, and then + * free the dynamic array structure itself. + * + * The passed array must have been created by _PyDynArray_New() + */ static inline void _PyDynArray_Free(_PyDynArray *array) { + _PyDynArray_ASSERT_VALID(array); _PyDynArray_Clear(array); PyMem_RawFree(array); } +/* + * Equivalent to _PyDynArray_InitWithSize() with a size of 16. + * + * Returns -1 upon failure, 0 otherwise. + */ static inline int _PyDynArray_Init(_PyDynArray *array, _PyDynArray_Deallocator deallocator) { return _PyDynArray_InitWithSize(array, deallocator, _PyDynArray_DEFAULT_SIZE); } +/* + * Allocate and create a new dynamic array on the heap. + * + * The returned pointer should be freed with _PyDynArray_Free() + * If this function fails, it returns NULL. + */ static inline _PyDynArray * _PyDynArray_NewWithSize(_PyDynArray_Deallocator deallocator, Py_ssize_t initial) { @@ -58,15 +100,40 @@ _PyDynArray_NewWithSize(_PyDynArray_Deallocator deallocator, Py_ssize_t initial) return NULL; } + _PyDynArray_ASSERT_VALID(array); // Sanity check return array; } +/* + * Equivalent to _PyDynArray_NewWithSize() with a size of 16. + */ static inline _PyDynArray * _PyDynArray_New(_PyDynArray_Deallocator deallocator) { return _PyDynArray_NewWithSize(deallocator, _PyDynArray_DEFAULT_SIZE); } +/* + * Get an item from the array. + * + * If the index is not valid, this is undefined behavior. + */ +static inline void * +_PyDynArray_GET_ITEM(_PyDynArray *array, Py_ssize_t index) +{ + _PyDynArray_ASSERT_VALID(array); + assert(index > 0); + assert(index < array->length); + return array->items[index]; +} + +static inline Py_ssize_t +_PyDynArray_LENGTH(_PyDynArray *array) +{ + _PyDynArray_ASSERT_VALID(array); + return array->length; +} + #ifdef __cplusplus } #endif diff --git a/Python/dynarray.c b/Python/dynarray.c index b572acbbdb83e4..58e4b26eef5907 100644 --- a/Python/dynarray.c +++ b/Python/dynarray.c @@ -28,7 +28,7 @@ _PyDynArray_InitWithSize(_PyDynArray *array, int _PyDynArray_Append(_PyDynArray *array, void *item) { - assert(array != NULL); + _PyDynArray_ASSERT_VALID(array); array->items[array->length++] = item; if (array->length == array->capacity) { @@ -51,8 +51,7 @@ _PyDynArray_Append(_PyDynArray *array, void *item) void _PyDynArray_Clear(_PyDynArray *array) { - assert(array != NULL); - assert(array->deallocator != NULL); + _PyDynArray_ASSERT_VALID(array); for (Py_ssize_t i = 0; i < array->length; ++i) { if (array->deallocator != NULL) From 3173c08eada906ef694687d48f6e8e07e66039e4 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 15 Oct 2024 17:30:01 -0400 Subject: [PATCH 06/22] Add some simple tests. --- Include/internal/pycore_dynarray.h | 3 ++ Modules/_testinternalcapi.c | 63 ++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index 9a0f01c9f4a8ea..86dbdc03ab4fbf 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -51,6 +51,9 @@ PyAPI_FUNC(int) _PyDynArray_Append(_PyDynArray *array, void *item); * * Note that this does *not* free the actual dynamic array * structure--use _PyDynArray_Free() for that. + * + * It's safe to call _PyDynArray_Init() or InitWithSize() again + * on the array after calling this. */ PyAPI_FUNC(void) _PyDynArray_Clear(_PyDynArray *array); diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index c403075fbb2501..0c5b5639c62bfe 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -17,6 +17,7 @@ #include "pycore_compile.h" // _PyCompile_CodeGen() #include "pycore_context.h" // _PyContext_NewHamtForTests() #include "pycore_dict.h" // _PyManagedDictPointer_GetValues() +#include "pycore_dynarray.h" // _PyDynArray #include "pycore_fileutils.h" // _Py_normpath() #include "pycore_flowgraph.h" // _PyCompile_OptimizeCfg() #include "pycore_frame.h" // _PyInterpreterFrame @@ -2048,6 +2049,67 @@ identify_type_slot_wrappers(PyObject *self, PyObject *Py_UNUSED(ignored)) return _PyType_GetSlotWrapperNames(); } +static int +test_dynarray_common(_PyDynArray *array) +{ + #define APPEND(ptr) do { \ + if (_PyDynArray_Append(array, ptr) < 0) \ + { \ + return -1; \ + } \ + } while (0); + + // Some dummy pointers + APPEND(Py_None); + APPEND(Py_True); + APPEND(Py_False); + + // Make sure that indexes work + assert(_PyDynArray_GET_ITEM(array, 0) == Py_None); + assert(_PyDynArray_GET_ITEM(array, 1) == Py_True); + assert(_PyDynArray_GET_ITEM(array, 2) == Py_False); + + // Now, let's make sure that resizing works. + for (int i = 0; i < (_PyDynArray_DEFAULT_SIZE * 2); ++i) + { + APPEND(NULL); + } + + // Make sure that nothing got corrupted + assert(_PyDynArray_GET_ITEM(array, 0) == Py_None); + assert(_PyDynArray_GET_ITEM(array, 1) == Py_True); + assert(_PyDynArray_GET_ITEM(array, 2) == Py_False); + return 0; +} + +static PyObject * +test_dynarray(PyObject *self, PyObject *unused) +{ + _PyDynArray array; + // In a loop to make sure that reinitialization works + for (int i = 0; i < 3; ++i) + { + if (_PyDynArray_Init(&array, NULL) < 0) + { + PyErr_NoMemory(); + return NULL; + } + + if (test_dynarray_common(&array) < 0) + { + PyErr_NoMemory(); + _PyDynArray_Clear(&array); + return NULL; + } + + _PyDynArray_Clear(&array); + } + + _PyDynArray *heap_array = _PyDynArray_New(NULL); + test_dynarray_common(heap_array); + _PyDynArray_Free(heap_array); + Py_RETURN_NONE; +} static PyMethodDef module_functions[] = { {"get_configs", get_configs, METH_NOARGS}, @@ -2145,6 +2207,7 @@ static PyMethodDef module_functions[] = { GH_119213_GETARGS_METHODDEF {"get_static_builtin_types", get_static_builtin_types, METH_NOARGS}, {"identify_type_slot_wrappers", identify_type_slot_wrappers, METH_NOARGS}, + {"test_dynarray", test_dynarray, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; From 6a8f0264acc9d20424bcff659c75b7c0eb63251d Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 15 Oct 2024 17:35:03 -0400 Subject: [PATCH 07/22] Clean up tests. --- Modules/_testinternalcapi.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 0c5b5639c62bfe..e896683e117053 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -2052,12 +2052,12 @@ identify_type_slot_wrappers(PyObject *self, PyObject *Py_UNUSED(ignored)) static int test_dynarray_common(_PyDynArray *array) { - #define APPEND(ptr) do { \ - if (_PyDynArray_Append(array, ptr) < 0) \ - { \ - return -1; \ - } \ - } while (0); +#define APPEND(ptr) do { \ + if (_PyDynArray_Append(array, ptr) < 0) \ + { \ + return -1; \ + } \ +} while (0); // Some dummy pointers APPEND(Py_None); @@ -2075,6 +2075,8 @@ test_dynarray_common(_PyDynArray *array) APPEND(NULL); } +#undef APPEND + // Make sure that nothing got corrupted assert(_PyDynArray_GET_ITEM(array, 0) == Py_None); assert(_PyDynArray_GET_ITEM(array, 1) == Py_True); @@ -2106,7 +2108,12 @@ test_dynarray(PyObject *self, PyObject *unused) } _PyDynArray *heap_array = _PyDynArray_New(NULL); - test_dynarray_common(heap_array); + if (test_dynarray_common(heap_array) < 0) + { + _PyDynArray_Free(heap_array); + PyErr_NoMemory(); + return NULL; + } _PyDynArray_Free(heap_array); Py_RETURN_NONE; } From 84a922a2c0488ac3788299a70c5dcd8de0caf717 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 15 Oct 2024 17:43:36 -0400 Subject: [PATCH 08/22] Add a test for deallocators. --- Modules/_testinternalcapi.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index e896683e117053..4cc059b62800f7 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -2107,6 +2107,7 @@ test_dynarray(PyObject *self, PyObject *unused) _PyDynArray_Clear(&array); } + // Array allocated on the heap _PyDynArray *heap_array = _PyDynArray_New(NULL); if (test_dynarray_common(heap_array) < 0) { @@ -2115,6 +2116,35 @@ test_dynarray(PyObject *self, PyObject *unused) return NULL; } _PyDynArray_Free(heap_array); + + // Still on the heap, but now the fields are too + _PyDynArray *array_with_deallocator = _PyDynArray_New(PyMem_Free); + if (array_with_deallocator == NULL) + { + PyErr_NoMemory(); + return NULL; + } +#define SILLY_STRING "My hovercraft is full of eels" + char *my_string = PyMem_Malloc(sizeof(SILLY_STRING)); + if (my_string == NULL) + { + _PyDynArray_Free(array_with_deallocator); + PyErr_NoMemory(); + return NULL; + } + strcpy(my_string, SILLY_STRING); + if (_PyDynArray_Append(array_with_deallocator, my_string) < 0) + { + PyMem_Free(my_string); + _PyDynArray_Free(array_with_deallocator); + PyErr_NoMemory(); + return NULL; + } + + assert(!strcmp(_PyDynArray_GET_ITEM(array_with_deallocator, 0), SILLY_STRING)); + _PyDynArray_Free(array_with_deallocator); +#undef SILLY_STRING + Py_RETURN_NONE; } From caf037cdaaccce778d79dde1ebe9c3022a10d71d Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 15 Oct 2024 17:51:02 -0400 Subject: [PATCH 09/22] Boy oh boy do I hate the Windows build system. --- PCbuild/_freeze_module.vcxproj | 1 + PCbuild/_freeze_module.vcxproj.filters | 3 +++ PCbuild/pythoncore.vcxproj | 2 ++ PCbuild/pythoncore.vcxproj.filters | 6 ++++++ 4 files changed, 12 insertions(+) diff --git a/PCbuild/_freeze_module.vcxproj b/PCbuild/_freeze_module.vcxproj index a3c2d32c454e04..71433924c80675 100644 --- a/PCbuild/_freeze_module.vcxproj +++ b/PCbuild/_freeze_module.vcxproj @@ -201,6 +201,7 @@ + diff --git a/PCbuild/_freeze_module.vcxproj.filters b/PCbuild/_freeze_module.vcxproj.filters index 91b1d75fb8df5e..9c1e29c9f38bfe 100644 --- a/PCbuild/_freeze_module.vcxproj.filters +++ b/PCbuild/_freeze_module.vcxproj.filters @@ -130,6 +130,9 @@ Source Files + + Source Files + Source Files diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 3b33c6bf6bb91d..8caeeaca519ac5 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -231,6 +231,7 @@ + @@ -587,6 +588,7 @@ + diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters index ee2930b10439a9..c6b24d4106617c 100644 --- a/PCbuild/pythoncore.vcxproj.filters +++ b/PCbuild/pythoncore.vcxproj.filters @@ -615,6 +615,9 @@ Include\internal + + Include\internal + Include\internal @@ -1316,6 +1319,9 @@ Python + + Python + Python From 14e46a4adca08371cf3e65a668568757ee9e1681 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 15 Oct 2024 17:54:21 -0400 Subject: [PATCH 10/22] Correction: I hate all build systems. --- Include/internal/pycore_dynarray.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index 86dbdc03ab4fbf..33749c83a0063d 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -125,7 +125,7 @@ static inline void * _PyDynArray_GET_ITEM(_PyDynArray *array, Py_ssize_t index) { _PyDynArray_ASSERT_VALID(array); - assert(index > 0); + assert(index >= 0); assert(index < array->length); return array->items[index]; } From be1de70ed1460bc1aa615b4946fa77dfda1b5304 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 19 Oct 2024 10:03:32 -0400 Subject: [PATCH 11/22] Add set and remove operations. --- Include/internal/pycore_dynarray.h | 26 ++++++++++++++++++++++++-- Python/dynarray.c | 30 +++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index 33749c83a0063d..7051cde922caf0 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -28,6 +28,14 @@ _PyDynArray_ASSERT_VALID(_PyDynArray *array) assert(array->items != NULL); } +static inline void +_PyDynArray_ASSERT_INDEX(_PyDynArray *array, Py_ssize_t index) +{ + // Ensure the index is valid + assert(index >= 0); + assert(index < array->length); +} + /* * Initialize a dynamic array with an initial size. * @@ -57,6 +65,21 @@ PyAPI_FUNC(int) _PyDynArray_Append(_PyDynArray *array, void *item); */ PyAPI_FUNC(void) _PyDynArray_Clear(_PyDynArray *array); +/* + * Set a value at index in the array. + * + * If an item already exists at the target index, the deallocator + * is called on it. + */ +PyAPI_FUNC(void) +_PyDynArray_Set(_PyDynArray *array, Py_ssize_t index, void *item); + +/* + * Remove the item at the index, and call the deallocator on it. + */ +PyAPI_FUNC(void) +_PyDynArray_Remove(_PyDynArray *array, Py_ssize_t index); + /* * Clear all the fields on a dynamic array, and then * free the dynamic array structure itself. @@ -125,8 +148,7 @@ static inline void * _PyDynArray_GET_ITEM(_PyDynArray *array, Py_ssize_t index) { _PyDynArray_ASSERT_VALID(array); - assert(index >= 0); - assert(index < array->length); + _PyDynArray_ASSERT_INDEX(array, index); return array->items[index]; } diff --git a/Python/dynarray.c b/Python/dynarray.c index 58e4b26eef5907..f56303c7aafe2e 100644 --- a/Python/dynarray.c +++ b/Python/dynarray.c @@ -1,5 +1,5 @@ /* - * Dynamic array implementation + * Dynamic array implementation. */ #include "pycore_dynarray.h" @@ -43,11 +43,39 @@ _PyDynArray_Append(_PyDynArray *array, void *item) return -1; } + // XXX Zero-out the new capacity? array->items = new_items; } return 0; } +void +_PyDynArray_Set(_PyDynArray *array, Py_ssize_t index, void *item) +{ + _PyDynArray_ASSERT_VALID(array); + _PyDynArray_ASSERT_INDEX(array, index); + if (array->items[index] != NULL) + { + array->deallocator(array->items[index]); + } + array->items[index] = item; +} + +void +_PyDynArray_Remove(_PyDynArray *array, Py_ssize_t index) +{ + _PyDynArray_ASSERT_VALID(array); + _PyDynArray_ASSERT_INDEX(array, index); + assert(array->items[index] != NULL); + array->deallocator(array->items[index]); + + for (Py_ssize_t i = array->length; i >= index; --i) + { + array->items[i] = array->items[i - 1]; + } + --array->length; +} + void _PyDynArray_Clear(_PyDynArray *array) { From db5adbfdd0a9d98e1ff145811c4f32e1fb14a380 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 19 Oct 2024 10:22:46 -0400 Subject: [PATCH 12/22] Add tests for set and remove. --- Modules/_testinternalcapi.c | 25 +++++++++++++++++++++++++ Python/dynarray.c | 26 ++++++++++++++------------ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 4cc059b62800f7..5a3c08c7184499 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -2081,6 +2081,14 @@ test_dynarray_common(_PyDynArray *array) assert(_PyDynArray_GET_ITEM(array, 0) == Py_None); assert(_PyDynArray_GET_ITEM(array, 1) == Py_True); assert(_PyDynArray_GET_ITEM(array, 2) == Py_False); + + _PyDynArray_Remove(array, 0); + assert(_PyDynArray_GET_ITEM(array, 0) == Py_True); + assert(_PyDynArray_GET_ITEM(array, 1) == Py_False); + + Py_ssize_t index = _PyDynArray_DEFAULT_SIZE; + _PyDynArray_Set(array, index, (void *) 1); + assert(_PyDynArray_GET_ITEM(array, index) == (void *) 1); return 0; } @@ -2142,6 +2150,23 @@ test_dynarray(PyObject *self, PyObject *unused) } assert(!strcmp(_PyDynArray_GET_ITEM(array_with_deallocator, 0), SILLY_STRING)); + + int *other_ptr = PyMem_Malloc(sizeof(int)); + if (other_ptr == NULL) + { + _PyDynArray_Free(array_with_deallocator); + PyErr_NoMemory(); + return NULL; + } + *other_ptr = 42; + if (_PyDynArray_Append(array_with_deallocator, other_ptr) < 0) + { + _PyDynArray_Free(array_with_deallocator); + PyMem_Free(other_ptr); + PyErr_NoMemory(); + } + assert(_PyDynArray_GET_ITEM(array_with_deallocator, 1) == other_ptr); + _PyDynArray_Remove(array_with_deallocator, 1); _PyDynArray_Free(array_with_deallocator); #undef SILLY_STRING diff --git a/Python/dynarray.c b/Python/dynarray.c index f56303c7aafe2e..1f8cb49450c4cf 100644 --- a/Python/dynarray.c +++ b/Python/dynarray.c @@ -4,6 +4,15 @@ #include "pycore_dynarray.h" +static inline void +call_deallocator_maybe(_PyDynArray *array, Py_ssize_t index) +{ + if (array->deallocator != NULL && array->items[index] != NULL) + { + array->deallocator(array->items[index]); + } +} + int _PyDynArray_InitWithSize(_PyDynArray *array, _PyDynArray_Deallocator deallocator, @@ -54,10 +63,7 @@ _PyDynArray_Set(_PyDynArray *array, Py_ssize_t index, void *item) { _PyDynArray_ASSERT_VALID(array); _PyDynArray_ASSERT_INDEX(array, index); - if (array->items[index] != NULL) - { - array->deallocator(array->items[index]); - } + call_deallocator_maybe(array, index); array->items[index] = item; } @@ -66,12 +72,11 @@ _PyDynArray_Remove(_PyDynArray *array, Py_ssize_t index) { _PyDynArray_ASSERT_VALID(array); _PyDynArray_ASSERT_INDEX(array, index); - assert(array->items[index] != NULL); - array->deallocator(array->items[index]); + call_deallocator_maybe(array, index); - for (Py_ssize_t i = array->length; i >= index; --i) + for (Py_ssize_t i = index; i < array->length - 1; ++i) { - array->items[i] = array->items[i - 1]; + array->items[i] = array->items[i + 1]; } --array->length; } @@ -82,10 +87,7 @@ _PyDynArray_Clear(_PyDynArray *array) _PyDynArray_ASSERT_VALID(array); for (Py_ssize_t i = 0; i < array->length; ++i) { - if (array->deallocator != NULL) - { - array->deallocator(array->items[i]); - } + call_deallocator_maybe(array, i); } PyMem_RawFree(array->items); From aad50d80e4731b357c8d5ac8ad23f87803fdf3f7 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 19 Oct 2024 10:36:47 -0400 Subject: [PATCH 13/22] Update docstrings. --- Include/internal/pycore_dynarray.h | 46 ++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index 7051cde922caf0..0844f127459941 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -12,15 +12,39 @@ extern "C" { #define _PyDynArray_DEFAULT_SIZE 16 +/* + * Deallocator for items on a _PyDynArray structure. A NULL pointer + * will never be given to the deallocator. + */ typedef void (*_PyDynArray_Deallocator)(void *); +/* + * Internal only dynamic array for CPython. + */ typedef struct { + /* + * The actual items in the dynamic array. + * Don't access this field publicly to get + * items--use _PyDynArray_GET_ITEM() instead. + */ void **items; + /* + * The length of the actual items array allocation. + */ Py_ssize_t capacity; + /* + * The number of items in the array. + * Don't use this field publicly--use _PyDynArray_LENGTH() + */ Py_ssize_t length; + /* + * The deallocator, set by one of the initializer functions. + * This may be NULL. + */ _PyDynArray_Deallocator deallocator; } _PyDynArray; + static inline void _PyDynArray_ASSERT_VALID(_PyDynArray *array) { @@ -37,7 +61,10 @@ _PyDynArray_ASSERT_INDEX(_PyDynArray *array, Py_ssize_t index) } /* - * Initialize a dynamic array with an initial size. + * Initialize a dynamic array with an initial size and deallocator. + * + * If the deallocator is NULL, then nothing happens to items upon + * removal and upon array clearing. * * Returns -1 upon failure, 0 otherwise. */ @@ -69,13 +96,14 @@ PyAPI_FUNC(void) _PyDynArray_Clear(_PyDynArray *array); * Set a value at index in the array. * * If an item already exists at the target index, the deallocator - * is called on it. + * is called on it, if the array has one set. */ PyAPI_FUNC(void) _PyDynArray_Set(_PyDynArray *array, Py_ssize_t index, void *item); /* - * Remove the item at the index, and call the deallocator on it. + * Remove the item at the index, and call the deallocator on it (if the array + * has one set). */ PyAPI_FUNC(void) _PyDynArray_Remove(_PyDynArray *array, Py_ssize_t index); @@ -84,7 +112,7 @@ _PyDynArray_Remove(_PyDynArray *array, Py_ssize_t index); * Clear all the fields on a dynamic array, and then * free the dynamic array structure itself. * - * The passed array must have been created by _PyDynArray_New() + * The array must have been created by _PyDynArray_New() */ static inline void _PyDynArray_Free(_PyDynArray *array) @@ -95,7 +123,7 @@ _PyDynArray_Free(_PyDynArray *array) } /* - * Equivalent to _PyDynArray_InitWithSize() with a size of 16. + * Equivalent to _PyDynArray_InitWithSize() with a default size of 16. * * Returns -1 upon failure, 0 otherwise. */ @@ -132,6 +160,9 @@ _PyDynArray_NewWithSize(_PyDynArray_Deallocator deallocator, Py_ssize_t initial) /* * Equivalent to _PyDynArray_NewWithSize() with a size of 16. + * + * The returned array must be freed with _PyDynArray_Free(). + * Returns NULL on failure. */ static inline _PyDynArray * _PyDynArray_New(_PyDynArray_Deallocator deallocator) @@ -140,7 +171,7 @@ _PyDynArray_New(_PyDynArray_Deallocator deallocator) } /* - * Get an item from the array. + * Get an item from the array. This cannot fail. * * If the index is not valid, this is undefined behavior. */ @@ -152,6 +183,9 @@ _PyDynArray_GET_ITEM(_PyDynArray *array, Py_ssize_t index) return array->items[index]; } +/* + * Get the length of the array. This cannot fail. + */ static inline Py_ssize_t _PyDynArray_LENGTH(_PyDynArray *array) { From 0ecc65f5dfc42304ee0b0cb0a5d6fe19c6c04533 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 19 Oct 2024 10:42:56 -0400 Subject: [PATCH 14/22] Add pop. --- Include/internal/pycore_dynarray.h | 23 +++++++++++++++++++++++ Python/dynarray.c | 25 ++++++++++++++++++++----- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index 0844f127459941..bdee04170000e7 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -97,6 +97,8 @@ PyAPI_FUNC(void) _PyDynArray_Clear(_PyDynArray *array); * * If an item already exists at the target index, the deallocator * is called on it, if the array has one set. + * + * This cannot fail. */ PyAPI_FUNC(void) _PyDynArray_Set(_PyDynArray *array, Py_ssize_t index, void *item); @@ -104,10 +106,21 @@ _PyDynArray_Set(_PyDynArray *array, Py_ssize_t index, void *item); /* * Remove the item at the index, and call the deallocator on it (if the array * has one set). + * + * This cannot fail. */ PyAPI_FUNC(void) _PyDynArray_Remove(_PyDynArray *array, Py_ssize_t index); +/* + * Remove the item at the index *without* deallocating it, and + * return the item. + * + * This cannot fail. + */ +PyAPI_FUNC(void *) +_PyDynArray_Pop(_PyDynArray *array, Py_ssize_t index); + /* * Clear all the fields on a dynamic array, and then * free the dynamic array structure itself. @@ -193,6 +206,16 @@ _PyDynArray_LENGTH(_PyDynArray *array) return array->length; } +/* + * Pop the item at the end the array. + * This function cannot fail. + */ +static inline void * +_PyDynArray_PopTop(_PyDynArray *array, Py_ssize_t index) +{ + return _PyDynArray_PopTop(array, _PyDynArray_LENGTH(array) - 1); +} + #ifdef __cplusplus } #endif diff --git a/Python/dynarray.c b/Python/dynarray.c index 1f8cb49450c4cf..6e5d1e799f4683 100644 --- a/Python/dynarray.c +++ b/Python/dynarray.c @@ -67,18 +67,33 @@ _PyDynArray_Set(_PyDynArray *array, Py_ssize_t index, void *item) array->items[index] = item; } +static void +remove_no_dealloc(_PyDynArray *array, Py_ssize_t index) +{ + for (Py_ssize_t i = index; i < array->length - 1; ++i) + { + array->items[i] = array->items[i + 1]; + } + --array->length; +} + void _PyDynArray_Remove(_PyDynArray *array, Py_ssize_t index) { _PyDynArray_ASSERT_VALID(array); _PyDynArray_ASSERT_INDEX(array, index); call_deallocator_maybe(array, index); + remove_no_dealloc(array, index); +} - for (Py_ssize_t i = index; i < array->length - 1; ++i) - { - array->items[i] = array->items[i + 1]; - } - --array->length; +void * +_PyDynArray_Pop(_PyDynArray *array, Py_ssize_t index) +{ + _PyDynArray_ASSERT_VALID(array); + _PyDynArray_ASSERT_INDEX(array, index); + void *item = array->items[index]; + remove_no_dealloc(array, index); + return item; } void From 6a46733a8d3051a8d895df2d512799cae7ceeb9a Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 19 Oct 2024 10:46:02 -0400 Subject: [PATCH 15/22] Add tests for pop. --- Include/internal/pycore_dynarray.h | 4 ++-- Modules/_testinternalcapi.c | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index bdee04170000e7..c7b25cf1e82b35 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -211,9 +211,9 @@ _PyDynArray_LENGTH(_PyDynArray *array) * This function cannot fail. */ static inline void * -_PyDynArray_PopTop(_PyDynArray *array, Py_ssize_t index) +_PyDynArray_PopTop(_PyDynArray *array) { - return _PyDynArray_PopTop(array, _PyDynArray_LENGTH(array) - 1); + return _PyDynArray_Pop(array, _PyDynArray_LENGTH(array) - 1); } #ifdef __cplusplus diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 5a3c08c7184499..fabd94775dc79f 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -2075,6 +2075,10 @@ test_dynarray_common(_PyDynArray *array) APPEND(NULL); } + assert(_PyDynArray_PopTop(array) == NULL); + APPEND((void *) 42); + assert(_PyDynArray_PopTop(array) == (void *) 42); + #undef APPEND // Make sure that nothing got corrupted @@ -2089,6 +2093,7 @@ test_dynarray_common(_PyDynArray *array) Py_ssize_t index = _PyDynArray_DEFAULT_SIZE; _PyDynArray_Set(array, index, (void *) 1); assert(_PyDynArray_GET_ITEM(array, index) == (void *) 1); + assert(_PyDynArray_Pop(array, 0) == Py_True); return 0; } From 4d732dc384a49aa22653d28f62f4f1c58bcbe7d7 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 19 Oct 2024 10:52:42 -0400 Subject: [PATCH 16/22] Add insert. --- Include/internal/pycore_dynarray.h | 16 +++++++++--- Python/dynarray.c | 39 +++++++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index c7b25cf1e82b35..8612bd1e2ba9d8 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -74,15 +74,25 @@ _PyDynArray_InitWithSize(_PyDynArray *array, Py_ssize_t initial); /* - * Append to a dynamic array. + * Append to the array. * * Returns -1 upon failure, 0 otherwise. - * If this fails, the deallocator is not ran on *item*. + * If this fails, the deallocator is not ran on the item. */ PyAPI_FUNC(int) _PyDynArray_Append(_PyDynArray *array, void *item); /* - * Clear all the fields on a dynamic array. + * Insert an item at the target index. + * + * Returns -1 upon failure, 0 otherwise. + * If this fails, the deallocator is not ran on the item. + */ +PyAPI_FUNC(int) +_PyDynArray_Insert(_PyDynArray *array, Py_ssize_t index, void *item); + + +/* + * Clear all the fields on the array. * * Note that this does *not* free the actual dynamic array * structure--use _PyDynArray_Free() for that. diff --git a/Python/dynarray.c b/Python/dynarray.c index 6e5d1e799f4683..db715702ce25c6 100644 --- a/Python/dynarray.c +++ b/Python/dynarray.c @@ -34,11 +34,9 @@ _PyDynArray_InitWithSize(_PyDynArray *array, return 0; } -int -_PyDynArray_Append(_PyDynArray *array, void *item) +static int +resize_if_needed(_PyDynArray *array) { - _PyDynArray_ASSERT_VALID(array); - array->items[array->length++] = item; if (array->length == array->capacity) { // Need to resize @@ -55,6 +53,39 @@ _PyDynArray_Append(_PyDynArray *array, void *item) // XXX Zero-out the new capacity? array->items = new_items; } + + return 0; +} + +int +_PyDynArray_Append(_PyDynArray *array, void *item) +{ + _PyDynArray_ASSERT_VALID(array); + array->items[array->length++] = item; + if (resize_if_needed(array) < 0) + { + return -1; + } + return 0; +} + +int +_PyDynArray_Insert(_PyDynArray *array, Py_ssize_t index, void *item) +{ + _PyDynArray_ASSERT_VALID(array); + _PyDynArray_ASSERT_INDEX(array, index); + ++array->length; + array->items[index] = item; + + for (Py_ssize_t i = index + 1; i < array->length; ++i) + { + array->items[i] = array->items[i + 1]; + } + + if (resize_if_needed(array) < 0) + { + return -1; + } return 0; } From 95c77539bd50edd21bebce95b998bb5522abc1ed Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 19 Oct 2024 10:57:05 -0400 Subject: [PATCH 17/22] Add insertion. --- Include/internal/pycore_dynarray.h | 3 ++- Modules/_testinternalcapi.c | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/Include/internal/pycore_dynarray.h b/Include/internal/pycore_dynarray.h index 8612bd1e2ba9d8..6dfc2381028fb7 100644 --- a/Include/internal/pycore_dynarray.h +++ b/Include/internal/pycore_dynarray.h @@ -82,7 +82,8 @@ _PyDynArray_InitWithSize(_PyDynArray *array, PyAPI_FUNC(int) _PyDynArray_Append(_PyDynArray *array, void *item); /* - * Insert an item at the target index. + * Insert an item at the target index. The index + * must currently be a valid index in the array. * * Returns -1 upon failure, 0 otherwise. * If this fails, the deallocator is not ran on the item. diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index fabd94775dc79f..11e9bfa5650307 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -2086,14 +2086,31 @@ test_dynarray_common(_PyDynArray *array) assert(_PyDynArray_GET_ITEM(array, 1) == Py_True); assert(_PyDynArray_GET_ITEM(array, 2) == Py_False); + // Test removal _PyDynArray_Remove(array, 0); assert(_PyDynArray_GET_ITEM(array, 0) == Py_True); assert(_PyDynArray_GET_ITEM(array, 1) == Py_False); Py_ssize_t index = _PyDynArray_DEFAULT_SIZE; + // Test setting values _PyDynArray_Set(array, index, (void *) 1); assert(_PyDynArray_GET_ITEM(array, index) == (void *) 1); + + // We already tested PopTop, but it doesn't hurt to test this one. assert(_PyDynArray_Pop(array, 0) == Py_True); + + // Test insertion + _PyDynArray_Insert(array, 1, (void *) 99); + assert(_PyDynArray_GET_ITEM(array, 1) == (void *) 99); + + // Resizing with insertion + for (void *i = 0; i < (void *) 20; ++i) + { + _PyDynArray_Insert(array, 2, i); + assert(_PyDynArray_GET_ITEM(array, 2) == i); + } + assert(_PyDynArray_GET_ITEM(array, 2) == (void *) 19); + return 0; } From 51664b857e60ac3bcca70d483804aa57489e0ba2 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 19 Oct 2024 12:32:26 -0400 Subject: [PATCH 18/22] Add more heap tests and fix insertion. --- Modules/_testinternalcapi.c | 101 ++++++++++++++++++++++-------------- Python/dynarray.c | 9 ++-- 2 files changed, 68 insertions(+), 42 deletions(-) diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 11e9bfa5650307..dd0c6fe085a421 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -2109,11 +2109,73 @@ test_dynarray_common(_PyDynArray *array) _PyDynArray_Insert(array, 2, i); assert(_PyDynArray_GET_ITEM(array, 2) == i); } + + assert(_PyDynArray_GET_ITEM(array, 5) == (void *) 16); + assert(_PyDynArray_GET_ITEM(array, 4) == (void *) 17); + assert(_PyDynArray_GET_ITEM(array, 3) == (void *) 18); assert(_PyDynArray_GET_ITEM(array, 2) == (void *) 19); return 0; } +static int +test_heap_array(_PyDynArray *array) +{ +#define DO(item, operation) \ + do { \ + if (item == NULL) \ + { \ + _PyDynArray_Free(array); \ + PyErr_NoMemory(); \ + return -1; \ + } \ + if (operation < 0) \ + { \ + PyMem_Free(item); \ + _PyDynArray_Free(array); \ + PyErr_NoMemory(); \ + return -1; \ + }; \ + } while (0) + +#define SILLY_STRING "My hovercraft is full of eels" + // Heap append + char *my_string = PyMem_Malloc(sizeof(SILLY_STRING)); + DO(my_string, _PyDynArray_Append(array, my_string)); + strcpy(my_string, SILLY_STRING); + + // Heap insertion + int *other_ptr = PyMem_Malloc(sizeof(int)); + DO(other_ptr, _PyDynArray_Insert(array, 0, other_ptr)); + *other_ptr = 42; + assert(_PyDynArray_GET_ITEM(array, 0) == other_ptr); + + // Make sure our other allocation is still alive + assert(!strcmp(_PyDynArray_GET_ITEM(array, 1), SILLY_STRING)); + + // Heap pop + char *popped = _PyDynArray_PopTop(array); + assert(!strcmp(popped, my_string)); + PyMem_Free(popped); + + // A lot of heap appends. This is mainly + // so leak tests pick this up if deallocation + // wasn't happening. + for (int i = 0; i < 15; ++i) + { + int *ptr = PyMem_Malloc(sizeof(int)); + DO(ptr, _PyDynArray_Append(array, ptr)); + *ptr = i; + } + + // Heap removal + _PyDynArray_Remove(array, 10); +#undef SILLY_STRING +#undef DO + + return 0; +} + static PyObject * test_dynarray(PyObject *self, PyObject *unused) { @@ -2154,45 +2216,8 @@ test_dynarray(PyObject *self, PyObject *unused) PyErr_NoMemory(); return NULL; } -#define SILLY_STRING "My hovercraft is full of eels" - char *my_string = PyMem_Malloc(sizeof(SILLY_STRING)); - if (my_string == NULL) - { - _PyDynArray_Free(array_with_deallocator); - PyErr_NoMemory(); - return NULL; - } - strcpy(my_string, SILLY_STRING); - if (_PyDynArray_Append(array_with_deallocator, my_string) < 0) - { - PyMem_Free(my_string); - _PyDynArray_Free(array_with_deallocator); - PyErr_NoMemory(); - return NULL; - } - - assert(!strcmp(_PyDynArray_GET_ITEM(array_with_deallocator, 0), SILLY_STRING)); - - int *other_ptr = PyMem_Malloc(sizeof(int)); - if (other_ptr == NULL) - { - _PyDynArray_Free(array_with_deallocator); - PyErr_NoMemory(); - return NULL; - } - *other_ptr = 42; - if (_PyDynArray_Append(array_with_deallocator, other_ptr) < 0) - { - _PyDynArray_Free(array_with_deallocator); - PyMem_Free(other_ptr); - PyErr_NoMemory(); - } - assert(_PyDynArray_GET_ITEM(array_with_deallocator, 1) == other_ptr); - _PyDynArray_Remove(array_with_deallocator, 1); - _PyDynArray_Free(array_with_deallocator); -#undef SILLY_STRING - Py_RETURN_NONE; + return test_heap_array(array_with_deallocator) < 0 ? NULL : Py_None; } static PyMethodDef module_functions[] = { diff --git a/Python/dynarray.c b/Python/dynarray.c index db715702ce25c6..480c24aa2e183e 100644 --- a/Python/dynarray.c +++ b/Python/dynarray.c @@ -3,6 +3,7 @@ */ #include "pycore_dynarray.h" +#include "pycore_pymem.h" // _PyMem_RawStrdup static inline void call_deallocator_maybe(_PyDynArray *array, Py_ssize_t index) @@ -75,13 +76,13 @@ _PyDynArray_Insert(_PyDynArray *array, Py_ssize_t index, void *item) _PyDynArray_ASSERT_VALID(array); _PyDynArray_ASSERT_INDEX(array, index); ++array->length; - array->items[index] = item; - - for (Py_ssize_t i = index + 1; i < array->length; ++i) + for (Py_ssize_t i = array->length - 1; i >= index; --i) { - array->items[i] = array->items[i + 1]; + array->items[i] = array->items[i - 1]; } + array->items[index] = item; + if (resize_if_needed(array) < 0) { return -1; From b2be58f716853a796d04b5fbd457f1a694acc742 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 19 Oct 2024 12:38:12 -0400 Subject: [PATCH 19/22] Don't use GCC extension. --- Modules/_testinternalcapi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index dd0c6fe085a421..2c25026d9e293d 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -2104,10 +2104,10 @@ test_dynarray_common(_PyDynArray *array) assert(_PyDynArray_GET_ITEM(array, 1) == (void *) 99); // Resizing with insertion - for (void *i = 0; i < (void *) 20; ++i) + for (Py_ssize_t i = 0; i < 20; ++i) { - _PyDynArray_Insert(array, 2, i); - assert(_PyDynArray_GET_ITEM(array, 2) == i); + _PyDynArray_Insert(array, 2, (void *) i); + assert(_PyDynArray_GET_ITEM(array, 2) == (void *) i); } assert(_PyDynArray_GET_ITEM(array, 5) == (void *) 16); From a24fec2523b9657481ffa8d45f23a10e291c09d0 Mon Sep 17 00:00:00 2001 From: Kirill Podoprigora Date: Sat, 19 Oct 2024 23:17:06 +0300 Subject: [PATCH 20/22] > instead of >= --- Python/dynarray.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/dynarray.c b/Python/dynarray.c index 480c24aa2e183e..dc11e8d96a3197 100644 --- a/Python/dynarray.c +++ b/Python/dynarray.c @@ -76,7 +76,7 @@ _PyDynArray_Insert(_PyDynArray *array, Py_ssize_t index, void *item) _PyDynArray_ASSERT_VALID(array); _PyDynArray_ASSERT_INDEX(array, index); ++array->length; - for (Py_ssize_t i = array->length - 1; i >= index; --i) + for (Py_ssize_t i = array->length - 1; i > index; --i) { array->items[i] = array->items[i - 1]; } From 23902f9012e5cef16381ef5d0dcdc4db1439b3bd Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 19 Oct 2024 16:51:10 -0400 Subject: [PATCH 21/22] Fix behavior upon allocation failure. --- Python/dynarray.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Python/dynarray.c b/Python/dynarray.c index dc11e8d96a3197..b08fdf0e5e403e 100644 --- a/Python/dynarray.c +++ b/Python/dynarray.c @@ -47,7 +47,6 @@ resize_if_needed(_PyDynArray *array) sizeof(void *) * array->capacity); if (new_items == NULL) { - --array->length; return -1; } @@ -65,6 +64,7 @@ _PyDynArray_Append(_PyDynArray *array, void *item) array->items[array->length++] = item; if (resize_if_needed(array) < 0) { + array->items[--array->length] = NULL; return -1; } return 0; @@ -76,17 +76,21 @@ _PyDynArray_Insert(_PyDynArray *array, Py_ssize_t index, void *item) _PyDynArray_ASSERT_VALID(array); _PyDynArray_ASSERT_INDEX(array, index); ++array->length; + if (resize_if_needed(array) < 0) + { + // Grow the array beforehand, otherwise it's + // going to be a mess putting it back together if + // allocation fails. + --array->length; + return -1; + } + for (Py_ssize_t i = array->length - 1; i > index; --i) { array->items[i] = array->items[i - 1]; } array->items[index] = item; - - if (resize_if_needed(array) < 0) - { - return -1; - } return 0; } From 86f78c6e70d7cc04c59d3da188f6eda3bb5cd103 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 19 Oct 2024 16:59:02 -0400 Subject: [PATCH 22/22] Remove useless comment. --- Python/dynarray.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/dynarray.c b/Python/dynarray.c index b08fdf0e5e403e..de5b1517224169 100644 --- a/Python/dynarray.c +++ b/Python/dynarray.c @@ -50,7 +50,6 @@ resize_if_needed(_PyDynArray *array) return -1; } - // XXX Zero-out the new capacity? array->items = new_items; }