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
>
> 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
cheers,
Pádraig.
