mattreecebentley / plf_hive

plf::hive is a fork of plf::colony to match the current C++ standards proposal.
https://plflib.org/colony.htm
zlib License
71 stars 7 forks source link

Invalid write in `recover_from_partial_skipblock_fill` #20

Closed Quuxplusone closed 2 years ago

Quuxplusone commented 2 years ago
#include "plf_hive.h"

int throwing = 0;

struct S {
    int x;
    S(int x) : x(x) {}
    S(const S& s) : x(s.x) { if (--throwing == 0) throw 42; }
};

int main()
{
    plf::hive<S> h = {1, 2};
    h.erase(std::next(h.begin()));
    throwing = 1;
    try {
        h.insert(2, 42);
    } catch (...) { }
}
$ valgrind ./a.out
==89089== Memcheck, a memory error detector
==89089== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==89089== Using Valgrind-3.19.0.GIT-lbmacos and LibVEX; rerun with -h for copyright info
==89089== Command: ./a.out
==89089== 
==89089== Invalid write of size 1
==89089==    at 0x1007CFDC9: _platform_bzero$VARIANT$Haswell (in /usr/lib/system/libsystem_platform.dylib)
==89089==    by 0x100007978: plf::hive<S, std::__1::allocator<S>, plf::hive_priority::performance>::recover_from_partial_skipblock_fill(std::__1::aligned_storage<4ul, 4ul>::type*, std::__1::aligned_storage<4ul, 4ul>::type*, unsigned short*, unsigned short) (plf_hive.h:2295)

It looks to me like the culprit is this line:

-   const skipfield_type elements_constructed_before_exception = static_cast<skipfield_type>((current_location - 1) - location);
+  const skipfield_type elements_constructed_before_exception = static_cast<skipfield_type>((current_location) - location);

Removing the -1 as shown seems to fix the issue. (current_location here points to the object we were trying to construct when the exception occurred, so that object wasn't constructed, but everything to the left of it was.)

mattreecebentley commented 2 years ago

On 2/05/2022 5:15 am, Quuxplusone wrote:

|#include "plf_hive.h" int throwing = 0; struct S { int x; S(int x) : x(x) {} S(const S& s) : x(s.x) { if (--throwing == 0) throw 42; } }; int main() { plf::hive h = {1, 2}; h.erase(std::next(h.begin())); throwing = 1; try { h.insert(2, 42); } catch (...) { } } |

|$ valgrind ./a.out ==89089== Memcheck, a memory error detector ==89089== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==89089== Using Valgrind-3.19.0.GIT-lbmacos and LibVEX; rerun with -h for copyright info ==89089== Command: ./a.out ==89089== ==89089== Invalid write of size 1 ==89089== at 0x1007CFDC9: _platform_bzero$VARIANT$Haswell (in /usr/lib/system/libsystem_platform.dylib) ==89089== by 0x100007978: plf::hive<S, std::1::allocator, plf::hive_priority::performance>::recover_from_partial_skipblock_fill(std::__1::aligned_storage<4ul, 4ul>::type*, std::1::aligned_storage<4ul, 4ul>::type, unsigned short, unsigned short) (plf_hive.h:2295) |

It looks to me like the culprit is this line:

|- const skipfield_type elements_constructed_before_exception = static_cast((current_location - 1) - location); + const skipfield_type elements_constructed_before_exception = static_cast((current_location) - location); |

Removing the |-1| as shown seems to fix the issue. (|current_location| here points to the object we were trying to construct when the exception occurred, so /that/ object wasn't constructed, but everything to the left of it was.)

Yes, that's correct - mistake on my part - fixed.