floooh / oryol

A small, portable and extensible C++ 3D coding framework
MIT License
2k stars 200 forks source link

Bug in elementBuffer #271

Closed Frogmellon closed 7 years ago

Frogmellon commented 7 years ago

I get an (intermittent) crash when inserting into an empty elementBuffer that's previously been used. Repeatable (intermittent) with...

Map<int32_t, String> testMap;

for (int32_t counter = 0; counter < 20; ++counter) {
        testMap.Add(counter, "testString");
        testMap.Erase(counter);
}

Crashes when counter == 8.

I believe this goes wrong in elementBuffer::prepareInsert when index = 0, size = 0 buf->start = 16 buf->end = 16 buf->cap = 16 and then elementBuffer::moveInsertFront accesses out of bounds on this line...

new(&this->buf[this->start-1]) TYPE(std::move(this->buf[start]));

...and sometime later it all unravels.

floooh commented 7 years ago

Whoops interesting, thanks for the simplified test case! I'll have a look tomorrow, first thing :)

floooh commented 7 years ago

I should run more often with the clang Address Sanitizer, this is also triggering in the Buffer class test. I'll fix that first and then move on to the elementBuffer problem.

floooh commented 7 years ago

clang address sanitizer is also catching the error from your test case 100%, fixing this now...

Frogmellon commented 7 years ago

Thanks. I've actually found the code remarkably solid for a pretty ambitious side project.

floooh commented 7 years ago

Ok, the fix is in commit 2d2f9f467e14adebb7b335ca27738241c11d76bd.

I have added your minimal test to the Map unit test, and I also checked all tests and samples with clang address- and undefined-behaviour sanitizer (next I'll do the same for the more complex oryol-samples and my 8-bit emu).

Thanks a lot for the bug report!

Frogmellon commented 7 years ago

Thanks a lot for the prompt fix!