[PATCH 2/2] msvcp140_1: Implement the DLL.

[Arkadiusz Hiler]

On Fri, Apr 02, 2021 at 06:18:28PM +0200, Piotr Caban wrote:
> > +DEFINE_RTTI_DATA0(base_memory_resource, 0, ".?AVmemory_resource at pmr@std@@")
> > +DEFINE_RTTI_DATA1(aligned_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_Aligned_new_delete_resource at pmr@std@@")
> > +DEFINE_RTTI_DATA1(unaligned_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_Unligned_new_delete_resource at pmr@std@@")
> > +DEFINE_RTTI_DATA1(null_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_null_resource at 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 at pmr@std@@
.?AV_Aligned_new_delete_resource_impl at pmr@std@@
.?AV_Unaligned_new_delete_resource_impl at pmr@std@@
.?AV_Null_resource@?1??null_memory_resource@@YAPAVmemory_resource at pmr@std@@XZ@

Is there anything else wrong with this RTTI?

> > +DEFINE_RTTI_DATA0(base_memory_resource, 0, ".?AVmemory_resource at pmr@std@@")
> > +DEFINE_RTTI_DATA1(aligned_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_Aligned_new_delete_resource at pmr@std@@")
> > +DEFINE_RTTI_DATA1(unaligned_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_Unligned_new_delete_resource at pmr@std@@")
> > +DEFINE_RTTI_DATA1(null_resource, 0, &base_memory_resource_rtti_base_descriptor, ".?AV_null_resource at 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?

-- 
Cheers,
Arek