On 4/5/21 6:36 PM, Arkadiusz Hiler wrote:
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?
The aligned_resource, unaligned_resource and null_resource are also inheriting '.?AV_Identity_equal_resource@pmr@std@@' class.
The definitions should look like this: DEFINE_RTTI_DATA0(base_memory_resource, 0, ".?AVmemory_resource@pmr@std@@") DEFINE_RTTI_DATA1(_Identity_equal_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_Identity_equal_resource@pmr@std@@") DEFINE_RTTI_DATA2(aligned_resource, 0, &_Identity_equal_resource_rtti_base_descriptor, &base_memory_resource_rtti_base_descriptor, ".?AV_Aligned_new_delete_resource_impl@pmr@std@@")
Another problem is that DEFINE_RTTI_DATA defines lots of unused structures in case of base_memory_resource and _Identity_equal_resource classes (e.g. virtual function tables should not be needed at all). I will experiment with it tomorrow.
+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.
It probably fails to free the memory if it's used interchangeable.
Do you want me to send this and your version of patch 1 together or are you going to upstream it on your own?
Please send it together with your patches.
Thanks, Piotr