On 20/05/19 10:44 +0100, Jonathan Wakely wrote:
On 20/05/19 09:17 +0000, Pádraig Brady wrote:
On 04/02/2019 07:33 PM, Padraig Brady wrote:
On 03/07/2019 03:43 AM, Jonathan Wakely wrote:
OK, that makes me feel better about it. It's presumably much easier to
upgrade to 5.2 from 5.0 or 5.1 than it would be from 4.x.
How complicated is the fix to prevent the crashes? Would it be
feasible for distros to backport that fix? I see that RHEL8 has
jemalloc 5.0.1 for example, but if the fix could be backported to that
release then it's less of a problem.
The patch set is simple enough:
https://github.com/jemalloc/jemalloc/pull/1341/commits
Thanks. That does seem reasonable for distros and other packagers to
backport, if they want to support 5.0 or 5.1 for their users.
I'm leaning towards accepting the patch for gcc-9 (and if not, we
should do it early in the gcc-10 cycle).
FYI jemalloc 5.2 is released with the fix for zero sized deallocations:
https://github.com/jemalloc/jemalloc/releases/tag/5.2.0
Friendly ping.
I can create a bug to track if you prefer.
No thanks, patches belong on these lists.
Now that we're in GCC 10 development stage 1 I'm happy to apply the
patch. I think by the time GCC 10 is released it will be reasonable to
expect people to use the fixed version of jemalloc.
I'll do the change today.
Tested powerpc64le-linux (without jemalloc) and committed to trunk.
I also had to fix a latent testcase bug that this change revealed.
commit 77cfde812ee7d83610f2b53fe63f7ad2a7a36c3e
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Mon May 20 11:46:57 2019 +0100
std::allocator::deallocate support sized-deallocation
Pass the size to the allocator so that it may optimize deallocation.
This was seen to significantly reduce the work required in jemalloc,
with about 40% reduction in CPU cycles in the free path.
Note jemalloc >= 5.2 is required to fix a crash with 0 sizes.
2019-05-20 P??draig Brady <pbr...@fb.com>
* libstdc++-v3/include/ext/new_allocator.h (deallocate): Pass the size
to the deallocator with -fsized-deallocation.
diff --git a/libstdc++-v3/include/ext/new_allocator.h b/libstdc++-v3/include/ext/new_allocator.h
index e24539100f9..f1ff7da530b 100644
--- a/libstdc++-v3/include/ext/new_allocator.h
+++ b/libstdc++-v3/include/ext/new_allocator.h
@@ -116,16 +116,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// __p is not permitted to be a null pointer.
void
- deallocate(pointer __p, size_type)
+ deallocate(pointer __p, size_type __t)
{
#if __cpp_aligned_new
if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
{
- ::operator delete(__p, std::align_val_t(alignof(_Tp)));
+ ::operator delete(__p,
+# if __cpp_sized_deallocation
+ __t * sizeof(_Tp),
+# endif
+ std::align_val_t(alignof(_Tp)));
return;
}
#endif
- ::operator delete(__p);
+ ::operator delete(__p
+#if __cpp_sized_deallocation
+ , __t * sizeof(_Tp)
+#endif
+ );
}
size_type
commit d2327fd8f69ab126f73b5f65c46845f4cdc14cd8
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Mon May 20 12:10:15 2019 +0100
Fix test bug with mismatched alignment in allocate/deallocate
* testsuite/experimental/memory_resource/new_delete_resource.cc: Fix
test by passing correct alignment to deallocate function.
diff --git a/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc b/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
index 5c5a0a4d7e3..41232b6fc67 100644
--- a/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
+++ b/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
@@ -103,7 +103,6 @@ test02()
void
test03()
-
{
using std::max_align_t;
using std::size_t;
@@ -123,7 +122,8 @@ test03()
p = r1->allocate(2, alignof(char));
VERIFY( bytes_allocated == 2 );
VERIFY( aligned<max_align_t>(p) );
- r1->deallocate(p, 2);
+ r1->deallocate(p, 2, alignof(char));
+ __builtin_printf("%d\n", (int)bytes_allocated);
VERIFY( bytes_allocated == 0 );
p = r1->allocate(3, alignof(short));
