On 06/03/19 22:27 +0000, Pádraig Brady wrote:
On 03/06/2019 01:44 AM, Jonathan Wakely wrote:
On 06/03/19 09:20 +0000, Pádraig Brady wrote:
On 03/06/2019 12:50 AM, Jonathan Wakely wrote:
On 06/03/19 02:43 +0000, Pádraig Brady wrote:
On 02/26/2019 04:23 PM, Padraig Brady wrote:
Note jemalloc >= 5.1 is required to fix a bug with 0 sizes.
How serious is the bug? What are the symptoms?
I've updated the commit summary to say it's a crash.
Arguably that's better than mem corruption.
It looks like 5.1.0 is less than a year old, so older versions are
presumably still in wide use.
We could potentially workaround it by making
new_allocator::allocate(0) call ::operator new(1) when
__cpp_sized_deallocation is defined, and deallocate(p, 0) call
::operator delete(p, 1). Obviously I'd prefer not to do that,
because
the default operator new already has an equivalent check, and only
programs linking to jemalloc require the workaround.
Right the jemalloc fix was released May 2018.
It would be great to avoid the extra workarounds.
Given this would be released about a year after the jemalloc fix was
released,
and that this would only manifest upon program rebuild,
I would be inclined to not add any workarounds.
For reference tcmalloc and glibc malloc were seen to work fine with
this.
Actually the jemalloc issue will only be fixed with the release of 5.2
(a few weeks away).
I've updated the commit message in the attached accordingly.
Hmm, I'm a bit nervous about making a change that will cause crashes
unless using an unreleased version (I know it will be released by the
time GCC 9.1 is released, but some people might upgrade GCC without
upgrading jemalloc).
Yes it's not ideal. It does make it a lot less risky that one has to
rebuild programs to get the new functionality, so existing programs
will be unaffected. Also -fsized-deallocation is only enabled by
default on gcc with -std >= c++14.
The default is -std=gnu++14 so it's enabled unless you explicitly
choose an older dialect or add -fno-sized-deallocation.
Good point :)
On the other hand, zero sized allocations should be rare in practice.
Yes they were rare in testing here
So programs _rebuilt_ against the following would need to update
to jemalloc 5.2:
zero sized allocs, jemalloc<5.2, c++>=14, GCC>=9.1
Hopefully that's a small enough set.
Which versions of jemalloc replace operator delete(void*, size_t) ?
Was that something new in 5.0 or did older versions already provide a
replacement for the sized operator delete?
If it was introduced in 5.0 then there won't be a problem for 3.x and
4.x because they'll use the default definition from libstdc++ which
just calls ::operator delete(p).
Again very good point. The replacements were only added in 5.0:
https://github.com/jemalloc/jemalloc/commit/2319152d
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).
