Add support for C++17 new/delete. These are listed here, for operator new http://en.cppreference.com/w/cpp/memory/new/operator_new void* operator new ( std::size_t count, std::align_val_t al); void* operator new[]( std::size_t count, std::align_val_t al); void* operator new ( std::size_t count, std::align_val_t al, const std::nothrow_t&); void* operator new[]( std::size_t count, std::align_val_t al, const std::nothrow_t&); And here http://en.cppreference.com/w/cpp/memory/new/operator_delete for delete void operator delete ( void* ptr, std::align_val_t al ); void operator delete[]( void* ptr, std::align_val_t al ); void operator delete ( void* ptr, std::size_t sz ); void operator delete ( void* ptr, std::size_t sz, std::align_val_t al ); void operator delete[]( void* ptr, std::size_t sz, std::align_val_t al ); I'm adding this report in parallel to https://bugs.kde.org/show_bug.cgi?id=372347 to separate the work for C++14 and C++17.
The LLVM libcxx implementation of aligned new does the following: void * operator new(std::size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC { set size to at least 1 set align to at least size of void* ::posix_memalign(&p, static_cast<size_t>(alignment), size) if failed throw bad_alloc return pointer } The other aligned operator news just call the above one. The aligned deletes just call ordinary delete. There is some conditional compilation for MSVCRT but that should not be a concern. The libstdc++ is quite similar operator new (std::size_t sz, std::align_val_t al) { set size to at least 1 round up size to a multiple of alignment allocate with 'aligned_alloc' which is a wrapper around posix_memalign or malloc if posix_memalign is not available if failed throw bad_alloc return pointer }
Testcase to reproduce this behaviour: # Makefile .PHONY: clean # GCC and Valgrind built from SVN/git head CXX=${HOME}/tools/gcc/bin/g++ VG=${HOME}/tools/valgrind/bin/valgrind test32: test.cpp ${CXX} -o $@ $< -m32 -Wl,-rpath,${HOME}/tools/gcc/lib -faligned-new -std=c++17 test64: test.cpp ${CXX} -o $@ $< -m64 -Wl,-rpath,${HOME}/tools/gcc/lib64 -faligned-new -std=c++17 vg32: test32 ${VG} --trace-malloc=yes --show-mismatched-frees=yes ./$< vg64: test64 ${VG} --trace-malloc=yes --show-mismatched-frees=yes ./$< clean: rm -f test32 test64 test32_clang test64_clang Source code: #include <cstdlib> #include <new> class alignas(64) MyClass { int i; }; int main() { MyClass* myClass = new MyClass(); delete myClass; MyClass* myClassNt = new (std::nothrow) MyClass(); delete myClassNt; MyClass* myClass5 = new MyClass[5]; delete [] myClass5; MyClass* myClass5Nt = new (std::nothrow) MyClass[5]; delete [] myClass5Nt; } There are no mismatches but the output is --4614-- memalign(al 64, size 64) = 0x4425AC0 --4614-- free(0x4425AC0) --4614-- memalign(al 64, size 64) = 0x4425B80 --4614-- free(0x4425B80) --4614-- memalign(al 64, size 320) = 0x4425C80 --4614-- free(0x4425C80) --4614-- memalign(al 64, size 320) = 0x4425E40 --4614-- free(0x4425E40) Since Valgrind does not pick up the aligned new/deletes but it does pick up the underlying memaligns and frees. Here are the mangled function signatures Linux 32bit U _ZdaPvSt11align_val_t@@CXXABI_1.3.11 U _ZdlPvjSt11align_val_t@@CXXABI_1.3.11 U _ZnajSt11align_val_t@@CXXABI_1.3.11 U _ZnajSt11align_val_tRKSt9nothrow_t@@CXXABI_1.3.11 U _ZnwjSt11align_val_t@@CXXABI_1.3.11 U _ZnwjSt11align_val_tRKSt9nothrow_t@@CXXABI_1.3.11 Unmangled U operator delete[](void*, std::align_val_t)@@CXXABI_1.3.11 U operator delete(void*, unsigned int, std::align_val_t)@@CXXABI_1.3.11 U operator new[](unsigned int, std::align_val_t)@@CXXABI_1.3.11 U operator new[](unsigned int, std::align_val_t, std::nothrow_t const&)@@CXXABI_1.3.11 U operator new(unsigned int, std::align_val_t)@@CXXABI_1.3.11 U operator new(unsigned int, std::align_val_t, std::nothrow_t const&)@@CXXABI_1.3.11 Linux 64bit U _ZdaPvSt11align_val_t@@CXXABI_1.3.11 U _ZdlPvmSt11align_val_t@@CXXABI_1.3.11 U _ZnamSt11align_val_t@@CXXABI_1.3.11 U _ZnamSt11align_val_tRKSt9nothrow_t@@CXXABI_1.3.11 U _ZnwmSt11align_val_t@@CXXABI_1.3.11 U _ZnwmSt11align_val_tRKSt9nothrow_t@@CXXABI_1.3.11 Unmangled U operator delete[](void*, std::align_val_t)@@CXXABI_1.3.11 U operator delete(void*, unsigned long, std::align_val_t)@@CXXABI_1.3.11 U operator new[](unsigned long, std::align_val_t)@@CXXABI_1.3.11 U operator new[](unsigned long, std::align_val_t, std::nothrow_t const&)@@CXXABI_1.3.11 U operator new(unsigned long, std::align_val_t)@@CXXABI_1.3.11 U operator new(unsigned long, std::align_val_t, std::nothrow_t const&)@@CXXABI_1.3.11
On Solaris the signatures are 32bit demangled U operator delete[](void*, std::align_val_t) U operator delete(void*, unsigned int, std::align_val_t) U operator new[](unsigned int, std::align_val_t) U operator new[](unsigned int, std::align_val_t, std::nothrow_t const&) U operator new(unsigned int, std::align_val_t) U operator new(unsigned int, std::align_val_t, std::nothrow_t const&) Mangled U _ZdaPvSt11align_val_t U _ZdlPvjSt11align_val_t U _ZnajSt11align_val_t U _ZnajSt11align_val_tRKSt9nothrow_t U _ZnwjSt11align_val_t U _ZnwjSt11align_val_tRKSt9nothrow_t 64 bit demangled U operator delete[](void*, std::align_val_t) U operator delete(void*, unsigned long, std::align_val_t) U operator new[](unsigned long, std::align_val_t) U operator new[](unsigned long, std::align_val_t, std::nothrow_t const&) U operator new(unsigned long, std::align_val_t) U operator new(unsigned long, std::align_val_t, std::nothrow_t const&) Mangled U _ZdaPvSt11align_val_t U _ZdlPvmSt11align_val_t U _ZnamSt11align_val_t U _ZnamSt11align_val_tRKSt9nothrow_t U _ZnwmSt11align_val_t U _ZnwmSt11align_val_tRKSt9nothrow_t
This is starting to look good. I now get --17711-- _ZnwmSt11align_val_t(size 64, al 64) = 0x5AB4CC0 --17711-- _ZdlPvSt11align_val_t(0x5AB4CC0) --17711-- _ZnamSt11align_val_t(size 320, al 64) = 0x5AB4DC0 --17711-- _ZdaPvSt11align_val_t(0x5AB4DC0) --17711-- _ZnwmSt11align_val_t(size 64, al 64) = 0x5AB4FC0 --17711-- _ZdlPvmSt11align_val_t(0x5AB4FC0) --17711-- _ZnamSt11align_val_t(size 320, al 64) = 0x5AB50C0 --17711-- _ZdaPvmSt11align_val_t(0x5AB50C0) --17711-- _ZnwmSt11align_val_tRKSt9nothrow_t(size 64, al 64) = 0x5AB52C0 --17711-- _ZdlPvSt11align_val_tRKSt9nothrow_t(0x5AB52C0) --17711-- _ZnamSt11align_val_tRKSt9nothrow_t(size 320, al 64) = 0x5AB53C0 --17711-- _ZdaPvSt11align_val_tRKSt9nothrow_t(0x5AB53C0)
Created attachment 113074 [details] Patch adding aligned new delete support A fairly big change as I added to the existing alloc/new functions and the ALLOC macros.
Created attachment 131600 [details] Updates the patch with redirs for libc++ and adds a regtest
That is a big patch indeed. But mostly because it also adds support for libcxx and solaris. It looks good in general I believe. The ALLOC_or_NULL_ALIGNED now probably needs a ENOMEM check. The indentation in coregrind/m_scheduler/scheduler.c looks off (tabs vs spaces?) I think we need a configure check for whether the c++ compiler supports -std=c++17 for the testcase. Don't we also need new builtin_delete_aligned and builtin_vec_delete_aligned to get the new/malloc/free/delete mismatch correct?
I'll fix the indentation, ENOMEM and feature detection. The patch also has some merge conflicts now. For the aligned delete operators, libcxx has _LIBCPP_WEAK void operator delete(void* ptr, std::align_val_t) _NOEXCEPT { std::__libcpp_aligned_free(ptr); } and all of the other variations just call the above overload. In turn that calls inline _LIBCPP_INLINE_VISIBILITY void __libcpp_aligned_free(void* __ptr) { #if defined(_LIBCPP_MSVCRT_LIKE) ::_aligned_free(__ptr); #else ::free(__ptr); #endif } For libstd++ there is _GLIBCXX_WEAK_DEFINITION void operator delete(void* ptr, std::align_val_t) noexcept { #if _GLIBCXX_HAVE_ALIGNED_ALLOC || _GLIBCXX_HAVE_POSIX_MEMALIGN \ || _GLIBCXX_HAVE_MEMALIGN std::free (ptr); #elif _GLIBCXX_HAVE__ALIGNED_MALLOC _aligned_free (ptr); #else if (ptr) std::free (((void **) ptr)[-1]); // See aligned_alloc in new_opa.cc #endif } and again everything seems to be implemented in terms of the above overload. I believe _aligned_free is only on Windows. So it looks like nothing special is done at the moment, but no guarantee that won't change so I will add builtins.
OK, I think that this is about done. The one feature that is not present is detection of mismatches between aligned and non-aligned new/delete. This is mostly a hypothetical problem, and will only arise with users that have replaced these global operators. The reason that I haven't made this change is that it will have a fairly significant impact on 32 bit binaries. In mc_include.h there is typedef enum { MC_AllocMalloc = 0, MC_AllocNew = 1, MC_AllocNewVec = 2, MC_AllocCustom = 3 } MC_AllocKind; If I add MC_AllocNewAligbed and MC_AllocNewVecAligned then this will require an extra bit of storage for this enum. That will mean reducing the range of MC_Chun szB, which is currently SizeT szB : (sizeof(SizeT)*8)-2; // Size requested; 30 or 62 bits. If I do this, then I'd rather only do it for 64 bit platforms. 61 bits for the size is still plenty, whilst reducing the size from 1G to 512M on 32 bit might have repercussions.
Created attachment 136085 [details] Update after feedback from Mark I think all items have been addressed except mismatch detection. I'd prefer open another bugzilla item for that.
I hadn't realized we were so short on bits. But I agree that reducing the szB is questionable, at least on 32 bit. On the other hand we might be keeping a stack of ExeContext pointers per allocation already. So maybe just making both the allockind and szB a full word isn't that dramatic an idea. But lets do that as a separate issue. Just one comment about the alignment being upgraded and being a power of 2 in ALLOC_or_NULL_ALIGNED and ALLOC_or_BOMB_ALIGNED. If I am reading the standard correctly then ff the value of an alignment argument is not a valid alignment value, then the behavior is undefined. So it might make sense to don't adjust/fixup the alignment in these cases, but let the mc_malloc_wrappers handle this, just like they handle bad sizes with record_fishy_value_error.
BTW. I think it is more important to have these wrappers in now then to make sure they are perfect. So if you think they are good enough now then please check them in. But then please open bug reports for tracking alignment new vs other allocation kinds, and one for reporting fishy alignment values. So we don't forget.
This was done a while back.