Add type-aware allocator functions to the default allocator#87
Add type-aware allocator functions to the default allocator#87QuietMisdreavus wants to merge 9 commits into
Conversation
rdar://170007028
in order for the typed memory operations use of typed allocators to work, calls to allocator functions need to be decorated with the _MALLOC_TYPED attribute. since cmark accesses its allocator wrappers through function pointers and the cmark_mem struct, this commit adds the groundwork necessary to allow the typed allocator wrappers to be called correctly.
QuietMisdreavus
requested review from
compnerd,
d-ronnqvist,
franklinsch and
patshaughnessy
ojhunt
left a comment
ojhunt
left a comment
There was a problem hiding this comment.
This appears to be set up to work correctly with the typed allocators, but I can't comment on the style/design, etc for swift/cmark itself
snprajwal
left a comment
snprajwal
left a comment
There was a problem hiding this comment.
Mostly looks good, some minor concerns. Thanks for working on this!
| CMARK_GFM_EXPORT | ||
| void *cmark_mem_calloc_typed(cmark_mem *mem, size_t count, size_t size, cmark_malloc_type_id type_id); | ||
|
|
||
| #define cmark_mem_calloc_typed_backdeploy cmark_mem_calloc_typed | ||
|
|
||
| CMARK_GFM_EXPORT | ||
| void *cmark_mem_realloc_typed(cmark_mem *mem, void *ptr, size_t size, cmark_malloc_type_id type_id); | ||
|
|
||
| #define cmark_mem_realloc_typed_backdeploy cmark_mem_realloc_typed |
There was a problem hiding this comment.
Both these functions are gated behind _MALLOC_TYPE_ENABLED in mem.c, they would be declared but undefined symbols on platforms that don't define the macro.
There was a problem hiding this comment.
Oh right, i didn't account for when the feature isn't present. 🤦♀️ I can write up fallbacks so that these symbols resolve correctly.
There was a problem hiding this comment.
Thanks for adding the fallbacks. However, that still means that these symbols are available on platforms that don't define the macro, and passthrough to the untyped version. It would be cleaner for us to instead gate these declarations behind the macro so that the symbol is not available on platforms where it can't be used.
There was a problem hiding this comment.
I wasn't too keen on completely deleting the functions when typed allocators aren't available, but i guess they're not being directly called anyway, so i pushed up a commit moving the definitions into the guard.
Now that the wrappers are added to the type-aware allocator machinery, we can silence the allocator wrapper warning (which still fires here) to prevent future false-positives.
| #else | ||
|
|
||
| void *cmark_mem_calloc_typed(cmark_mem *mem, size_t count, size_t size, cmark_malloc_type_id type_id) { | ||
| return cmark_mem_calloc(mem, count, size); | ||
| } | ||
|
|
||
| void *cmark_mem_realloc_typed(cmark_mem *mem, void *ptr, size_t size, cmark_malloc_type_id type_id) { | ||
| return cmark_mem_realloc(mem, ptr, size); | ||
| } | ||
|
|
||
| #endif |
There was a problem hiding this comment.
Ref the comment in the header file: we can remove this and instead gate the declarations behind #if defined(_MALLOC_TYPE_ENABLED) && _MALLOC_TYPE_ENABLED
3 participants
Resolves rdar://170007028
This PR adapts swift-cmark to use type-aware memory allocators in its default allocator set. This required a significant divergence from upstream, due to the requirements for the automatic typed-memory allocator annotations: They can't be applied to function pointers. This means that all the calls to
cmark_memmethods needed to be reworked into standalone functions so that the swap for the type-aware version could occur.Implementation
This PR adds two new fields to
cmark_mem:calloc_typedandrealloc_typed, that take the same arguments as their untyped counterparts, plus a newcmark_malloc_type_idthat denotes a type ID. When typed memory operations are enabled, this is typedef'd tomalloc_type_id_tfor compatibility with the system type-aware allocators; otherwise it is typedef'd tounsigned long long, which is the current underlying type on macOS and is selected to ensure they are the same size.In addition, there is a new
mem.hheader that defines five functions. These are freestanding functions that do nothing but call their associated method in the givencmark_meminstance, but thecallocandreallocfunctions have an annotation to allow for the typed-memory allocator substitution to occur so that the type-aware allocators can be called when available. (They also introduce a handful of convenience macros to make calling them easier, and ensures that type information is available since they always use the functions with asizeofand a cast.)To facilitate the use of these methods and to increase the security of swift-cmark, i've updated the CMake configuration and the Swift package spec to include the
-ftyped-memory-operationsflag when available, as well as the-Wallocator-wrappersflag (which still triggers on these allocator wrappers due to a bug, heh). I limited the SwiftPM updates to a new package spec that requires Swift 6.3, since my testing with 6.2 indicated that its associated Clang didn't have both flags available. I was able to build it with the Xcode 26.4 beta and it seems like everything works, so unless there's something wrong on non-Apple platforms we should be able to keep it.Finally, the biggest thing this PR does is replace all the calls to
cmark_memmethods to use themem.hfreestanding functions, so that the type-aware memory allocators can be used throughout the library. (I added a freestanding function forfreeeven though we don't have a type-aware implementation of that - there is amalloc_type_freein the headers on macOS but i'm not sure how to wire that one up. 😅)