On Fri, Apr 02, 2021 at 06:18:28PM +0200, Piotr Caban wrote:
+DEFINE_RTTI_DATA0(base_memory_resource, 0, ".?AVmemory_resource@pmr@std@@") +DEFINE_RTTI_DATA1(aligned_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_Aligned_new_delete_resource@pmr@std@@") +DEFINE_RTTI_DATA1(unaligned_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_Unligned_new_delete_resource@pmr@std@@") +DEFINE_RTTI_DATA1(null_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_null_resource@pmr@std@@")
This doesn't match with native RTTI data. If you need help with that part please let me know
Those names should be correct:
.?AVmemory_resource@pmr@std@@ .?AV_Aligned_new_delete_resource_impl@pmr@std@@ .?AV_Unaligned_new_delete_resource_impl@pmr@std@@ .?AV_Null_resource@?1??null_memory_resource@@YAPAVmemory_resource@pmr@std@@XZ@
Is there anything else wrong with this RTTI?
+DEFINE_RTTI_DATA0(base_memory_resource, 0, ".?AVmemory_resource@pmr@std@@") +DEFINE_RTTI_DATA1(aligned_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_Aligned_new_delete_resource@pmr@std@@") +DEFINE_RTTI_DATA1(unaligned_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_Unligned_new_delete_resource@pmr@std@@") +DEFINE_RTTI_DATA1(null_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_null_resource@pmr@std@@")
+DEFINE_THISCALL_WRAPPER(aligned_do_allocate, 12) +void* __thiscall aligned_do_allocate(memory_resource *this, size_t bytes, size_t alignment) +{
- return MSVCRT_operator_new_aligned(bytes, alignment);
+}
This doesn't match the tests that are calling free (instead of _aligned_free) in some cases.
+DEFINE_THISCALL_WRAPPER(aligned_do_deallocate, 16) +void __thiscall aligned_do_deallocate(memory_resource *this, void *p, size_t bytes, size_t alignment) +{
- MSVCRT_operator_delete_aligned(p, alignment);
+}
Same here.
Ah yes. I've tweaked the tests to not crash on Windows, but forgot to change our impl. The tests are passing because our malloc()/free() and its aligned siblings are compatible wrt how they store the address of the original allocation making them interchangeable, so this was left unnoticed.
Now... thinking about it, it probably should use NEW_ALIGNMENT.
I'll add explicit tests for NEW_ALIGNMENT and 2 * NEW_ALIGNMENT to be 100% sure.
ACK on all the other comments and thanks for the review! :-)
Do you want me to send this and your version of patch 1 together or are you going to upstream it on your own?