Project segfaults as soon as i link against libbump.a

Hi,

Do you perhaps know why my project segfaults as soon as i link against the created libbump.a ? I don't even have to call any functions from it, just linking it in segfaults the project.

I ran the project through valgrind and right before the segfault it reported these memory issues but i don't know if they are being caused due to something i did wrong like with CXXFLAGS or so or if there is an issue with the project ?


==23746==    at 0x117494: std::_Function_base::~_Function_base() (std_function.h:243)
==23746==    by 0x10D262: function (std_function.h:395)
==23746==    by 0x10D262: plugin::physics::bump::response::Cross::Resolve(plugin::physics::bump::CollisionResolution&, plugin::physics::bump::Collision*, plugin::physics::bump::Rectangle const&, math::vec2, std::function<bool (plugin::physics::bump::Item*, plugin::physics::bump::Item*, plugin::physics::bump::Collision*)>) [clone .cold] (CollisionResponse.cpp:28)
==23746==  Address 0x10 is not stack'd, malloc'd or (recently) free'd
==23746==
==23746==
==23746== Process terminating with default action of signal 11 (SIGSEGV)
==23746==  Access not within mapped region at address 0x10
==23746==    at 0x117494: std::_Function_base::~_Function_base() (std_function.h:243)
==23746==    by 0x10D262: function (std_function.h:395)
==23746==    by 0x10D262: plugin::physics::bump::response::Cross::Resolve(plugin::physics::bump::CollisionResolution&, plugin::physics::bump::Collision*, plugin::physics::bump::Rectangle const&, math::vec2, std::function<bool (plugin::physics::bump::Item*, plugin::physics::bump::Item*, plugin::physics::bump::Collision*)>) [clone .cold] (CollisionResponse.cpp:28)```

Hello

Sorry you are having trouble with it. Bump.hpp has been tested on windows and linux using GCC

Which compiler and OS are you using?

I have not seen this type of error before unfortunately. Note that the flags I used are in https://github.com/Polynominal/bump.hpp/blob/main/.gcc-flags.json

Could you provide me with any other data regarding your setup? Are you using CMake to link Bump or through some other means?

I suspect I know what might be behind this and its likely is a oversight on my part, could you remove: https://github.com/Polynominal/bump.hpp/blob/main/CMakeLists.txt#L19 and the test directory then try compiling? I am hoping maybe the testing setup is interfering with it.

I should have added a flag for that but I must have forgot to do so, let me know if it works and I will add the flag.

Hi,

I'm still investigating but the error i showed was from running the compiled program through valgrind. Valgrid is a program that can show memory issues like (re)using already free'd memory or showing memory leaks etc. I tried compiling just the vanilla sources using cmake and it compiles fine and the test suite program also runs fine, however if i run the compiled test program with all the tests through valgrind it seems to report all kinds of issues related to memory as well like reading wrong sized data from memory etc but i'm not sure why it was reporting this, maybe it's related to the use of the shared pointer or there really might be memory issues in the code. I'm no expert on this but valgrind usually should not really report any major problems but if i run the compiled test program through valgrind it seems to report all kind of issues.

this is the version of my cmake & g++ on kubuntu

What i initially tried by just linking against the library (compiled using mingw64-g++ ) by linking in the 4 cpp files directly or linking against libbump.a without ever calling any code from it and it seemed to segfault my program but that problem might be on my end as i was mixing c & c++ code all compiled through g++, i still need to investigate this further and i'm also no export on these kind of things. Usually if i see such problems i run the code through valgrind in a linux vm to see if no memory issues appear (like reading beyond array boundries or reading already freed memory etc). but even if i just compile everything normally using cmake as intended an run the test program compiled through valgrind i get a lot of potential problems mentioned. Things like reading outside of boundries as well as if conditions based on unitialized memory. If you got access to a linux machine try installing valgrind and running the compiled test program through valgrind and see all the things it reports (maybe limit the tests a bit so you don't get flooded as certain things keep getting repeated).

here is an example of some things that get reported by valgrind when running the test program (i limited tests)

I also don't know if the use of the shared pointer things might be related to all of this

By minimum you need C++ 17 features as I use std::function and smart pointers extensively, this is not a clean C style library.

I have used valgrind before and I am not super familiar with it, so I cannot say how many of these are directly related to STL or the code itself. I did test it with valgrind in the past but mostly to detect major memory leaks not related to STL eg two shared pointers owning each other or inaccessible memory.

The other software that I use usually reported many issues on valgrind so I presumed that's somewhat the norm. If you can point out something that I can take action on then I will consider it, as much as I think valgrind is a useful tool I find that many of its errors are very hard to interpret and trace back in C++ code.

Additionally I suggest you create one central cmake project which links everything for you instead of doing it manually such that you have a one click build. Originally Bump.hpp was part of such a project.

So I assume that you managed to compile the tests and run them individually and they ran OK? as in no SIGILL or SIGSEGV?

I have used valgrind before and I am not super familiar with it, so I cannot say how many of these are directly related to STLor the code itself.

I'm also not sure of this

If you can point out something that I can take action on then I will consider it, as much as I think valgrind is a useful tool I find that many of its errors are very hard to interpret and trace back in C++ code.

it similar for me but if i can find anything or make the errors go away in valgrind i'll let you know i'll also try upgrading my software as i had not done so in a long time on this vm to see if things improve

So I assume that you managed to compile the tests and run them individually and they ran OK? as in no SIGILL or SIGSEGV?

indeed i did and the compiled test program ran fine, no segfaults or so, so whatever was causing the issue must be unrelated to the library (or does not manifest itself in it).

If i can find anything more related to the valgrind detect issues i'll let you know but i assume what caused the segfault initially must have been unrelated to the library but due to my messing about on trying to mix c & c++ code, although linked through g++ c++ compiler. i might have to try and stop that. But i'll still try seeing if i can make something out of things valgrind reports and i'm not sure i'll be able to so i think since the test program runs completely fine it can be ignored for now

Would be interesting to know what valgrind outputs outside of gtests as it appears that gtests themselves can create garbage output potentially eg: https://github.com/google/googletest/issues/108

Please note that I have used Bump.hpp in a project but I am not currently actively using it for anything so I am not likely to provide major refactors unless I get back into it.

I'd like to stress the point regarding manual linking of libraries, it's quite error prone. Here is a example of a large project with each of its module set up as individual libraries that are then linked together: https://github.com/DiligentGraphics/DiligentEngine

Anyway, best of luck.

Hi @Polynominal i tested some more, one of the reported issues is real however this reported issue only appears in the test binary...

This was the reported issue:

==7698==    at 0x10DC3C: plugin::physics::bump::World::Project(std::vector<std::shared_ptr<plugin::physics::bump::Collision>, std::allocator<std::shared_ptr<plugin::physics::bump::Collision> > >&, plugin::physics::bump::Item*, plugin::physics::bump::Rectangle const&, math::vec2, std::function<bool (plugin::physics::bump::Item*, plugin::physics::bump::Item*, plugin::physics::bump::Collision*)>) (World.cpp:27)
==7698==    by 0x12390A: plugin::physics::bump::response::Slide::Resolve(plugin::physics::bump::CollisionResolution&, plugin::physics::bump::Collision*, plugin::physics::bump::Rectangle const&, math::vec2, std::function<bool (plugin::physics::bump::Item*, plugin::physics::bump::Item*, plugin::physics::bump::Collision*)>) (CollisionResponse.cpp:57)
==7698==    by 0x10A600: slide(float, float, float, float, float, float, float, float, float, float) (test.cpp:16)
==7698==    by 0x10AD6B: main (test.cpp:41)

you define for instance this code here https://github.com/Polynominal/bump.hpp/blob/main/tests/response_spec.cpp#L118 col value, however this is defined inside a function so col is not being intialized. so Col.Item is undefined. It then calls plugin::physics::bump::response::Slide::Resolve where (here https://github.com/Polynominal/bump.hpp/blob/main/src/CollisionResponse.cpp#L57) it will call the world project function. and world project function does a check on col.item not being a nullptr on line 27 here (https://github.com/Polynominal/bump.hpp/blob/main/src/World.cpp#L27) so that if condition is using a not initialized value from our initial col value where col.item is undefined

The error goes away in valgrind if i change the code to this, basically setting col->item to a nullptr so the check in world project function does not check on unitialized value.

thats at least one issue i found but as said only appears in test binary itselve. There are other functions that have a similar Col local variabele defined that has the same issue

yeah I can see it now, looks like I forgot to initialize my pointers in Collision.hpp Good work, thank you. That's a oversight on my part, any raw pointers that are unitialized will give UB on nullptr comparison.

col.item should be default initialized to nullptr as was my intention but I must have forgot to do so in the class Unfortunately I have a knack for forgetting to initialize raw pointers to null pointer. this one should make sure they are initialized so at least some of the errors should probably go away

I did a quick commit just now: https://github.com/Polynominal/bump.hpp/commit/8067e41d01466c4e5519f2348254f3e780bdb7ef

thanks i will do a new test, with the new code, as valgrind also reports other things i still have to look into (like invalid reads) but thats a very big stacktrace i get there so need to analyze it.

Oh i forgot to mention i got the library working now in my own project as well now i still mix c & c++ but i have only 1 extern C function now and i also no longer get any problems or segfaults, the library and code using the library is compiled as c++ code though so should be fine

I'll still test and look into other potential issues valgrind reports (if any) and if i find the cause, i will report here

i did the test and it indeed reports much less issues now in valgrind. There only seems to be a problem with freeing memory using remove and removeclean but i have yet to figure out by what it's being caused as the stacklists are rather long. Valgrind does mention where the memory was initially allocated though and where the actual delete / free happened that was wrong so it could give us some pointers I've posted a snipet the reported issues below. In reality there a lot more of these reported but i think they are probably all caused by the same issue. I also enabled debug builds so we can see the line numbers now


[ RUN      ] BumpWorldTest.Remove
==8608== Invalid read of size 4
==8608==    at 0x1512E1: plugin::physics::bump::Grid::ToCell(math::vec2 const&) (Grid.cpp:13)
==8608==    by 0x1517BA: plugin::physics::bump::Grid::ToCellRect(plugin::physics::bump::Rectangle const&) (Grid.cpp:82)
==8608==    by 0x153CD6: plugin::physics::bump::World::Remove(plugin::physics::bump::Item*) (World.cpp:230)
==8608==    by 0x124794: BumpWorldTest_Remove_Test::TestBody() (world_spec.cpp:149)
==8608==    by 0x19E222: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2589)
==8608==    by 0x196400: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2625)
==8608==    by 0x16D6B7: testing::Test::Run() (gtest.cc:2664)
==8608==    by 0x16E118: testing::TestInfo::Run() (gtest.cc:2842)
==8608==    by 0x16EA07: testing::TestSuite::Run() (gtest.cc:2996)
==8608==    by 0x17BBFD: testing::internal::UnitTestImpl::RunAllTests() (gtest.cc:5708)
==8608==    by 0x19F2A9: bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (gtest.cc:2589)
==8608==    by 0x197652: bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (gtest.cc:2625)
==8608==  Address 0x4e034f4 is 36 bytes inside a block of size 48 free'd
==8608==    at 0x484BB6F: operator delete(void*, unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==8608==    by 0x136560: std::default_delete<plugin::physics::bump::Item>::operator()(plugin::physics::bump::Item*) const (unique_ptr.h:85)
==8608==    by 0x135911: std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >::~unique_ptr() (unique_ptr.h:361)
==8608==    by 0x134CE2: void std::_Destroy<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > >(std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*) (stl_construct.h:151)
==8608==    by 0x13391E: void std::_Destroy_aux<false>::__destroy<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*>(std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*) (stl_construct.h:163)
==8608==    by 0x131DC1: void std::_Destroy<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*>(std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*) (stl_construct.h:196)
==8608==    by 0x1304DE: void std::_Destroy<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > >(std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > >&) (alloc_traits.h:848)
==8608==    by 0x15FA00: std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > >::_M_erase_at_end(std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*) (stl_vector.h:1796)
==8608==    by 0x15CCFE: std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > >::_M_erase(__gnu_cxx::__normal_iterator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > > >, __gnu_cxx::__normal_iterator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > > >) (vector.tcc:191)
==8608==    by 0x15ADC7: std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > >::erase(__gnu_cxx::__normal_iterator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > const*, std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > > >, __gnu_cxx::__normal_iterator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > const*, std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > > >) (stl_vector.h:1461)
==8608==    by 0x155551: bool removeIf<std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > >, plugin::physics::bump::World::Remove(plugin::physics::bump::Item*)::{lambda(auto:1&)#1}>(std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > >&, plugin::physics::bump::World::Remove(plugin::physics::bump::Item*)::{lambda(auto:1&)#1}) (World.cpp:18)
==8608==    by 0x153CB2: plugin::physics::bump::World::Remove(plugin::physics::bump::Item*) (World.cpp:224)
==8608==  Block was alloc'd at
==8608==    at 0x4849013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==8608==    by 0x159810: std::_MakeUniq<plugin::physics::bump::Item>::__single_object std::make_unique<plugin::physics::bump::Item, plugin::util::UserData&, plugin::physics::bump::Rectangle&>(plugin::util::UserData&, plugin::physics::bump::Rectangle&) (unique_ptr.h:962)
==8608==    by 0x153B7E: plugin::physics::bump::World::Add(plugin::util::UserData, plugin::physics::bump::Rectangle) (World.cpp:208)
==8608==    by 0x12F57E: plugin::physics::bump::Item* plugin::physics::bump::World::Add<TestItem>(std::shared_ptr<TestItem>, plugin::physics::bump::Rectangle) (World.hpp:113)
==8608==    by 0x12DFEC: BumpWorldTest::AddItem(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, float, float, float, float) (BumpWorldTest.hpp:83)
==8608==    by 0x1245FE: BumpWorldTest_Remove_Test::TestBody() (world_spec.cpp:140)
==8608==    by 0x19E222: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2589)
==8608==    by 0x196400: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2625)
==8608==    by 0x16D6B7: testing::Test::Run() (gtest.cc:2664)
==8608==    by 0x16E118: testing::TestInfo::Run() (gtest.cc:2842)
==8608==    by 0x16EA07: testing::TestSuite::Run() (gtest.cc:2996)
==8608==    by 0x17BBFD: testing::internal::UnitTestImpl::RunAllTests() (gtest.cc:5708)
==8608== 
==8608== 
==8608== Invalid read of size 4
==8608==    at 0x1517C8: plugin::physics::bump::Grid::ToCellRect(plugin::physics::bump::Rectangle const&) (Grid.cpp:83)
==8608==    by 0x153CD6: plugin::physics::bump::World::Remove(plugin::physics::bump::Item*) (World.cpp:230)
==8608==    by 0x124794: BumpWorldTest_Remove_Test::TestBody() (world_spec.cpp:149)
==8608==    by 0x19E222: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2589)
==8608==    by 0x196400: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2625)
==8608==    by 0x16D6B7: testing::Test::Run() (gtest.cc:2664)
==8608==    by 0x16E118: testing::TestInfo::Run() (gtest.cc:2842)
==8608==    by 0x16EA07: testing::TestSuite::Run() (gtest.cc:2996)
==8608==    by 0x17BBFD: testing::internal::UnitTestImpl::RunAllTests() (gtest.cc:5708)
==8608==    by 0x19F2A9: bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (gtest.cc:2589)
==8608==    by 0x197652: bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (gtest.cc:2625)
==8608==    by 0x17A368: testing::UnitTest::Run() (gtest.cc:5291)
==8608==  Address 0x4e034f0 is 32 bytes inside a block of size 48 free'd
==8608==    at 0x484BB6F: operator delete(void*, unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==8608==    by 0x136560: std::default_delete<plugin::physics::bump::Item>::operator()(plugin::physics::bump::Item*) const (unique_ptr.h:85)
==8608==    by 0x135911: std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >::~unique_ptr() (unique_ptr.h:361)
==8608==    by 0x134CE2: void std::_Destroy<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > >(std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*) (stl_construct.h:151)
==8608==    by 0x13391E: void std::_Destroy_aux<false>::__destroy<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*>(std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*) (stl_construct.h:163)
==8608==    by 0x131DC1: void std::_Destroy<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*>(std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*) (stl_construct.h:196)
==8608==    by 0x1304DE: void std::_Destroy<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > >(std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > >&) (alloc_traits.h:848)
==8608==    by 0x15FA00: std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > >::_M_erase_at_end(std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*) (stl_vector.h:1796)
==8608==    by 0x15CCFE: std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > >::_M_erase(__gnu_cxx::__normal_iterator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > > >, __gnu_cxx::__normal_iterator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >*, std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > > >) (vector.tcc:191)
==8608==    by 0x15ADC7: std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > >::erase(__gnu_cxx::__normal_iterator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > const*, std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > > >, __gnu_cxx::__normal_iterator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > const*, std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > > >) (stl_vector.h:1461)
==8608==    by 0x155551: bool removeIf<std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > >, plugin::physics::bump::World::Remove(plugin::physics::bump::Item*)::{lambda(auto:1&)#1}>(std::vector<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> >, std::allocator<std::unique_ptr<plugin::physics::bump::Item, std::default_delete<plugin::physics::bump::Item> > > >&, plugin::physics::bump::World::Remove(plugin::physics::bump::Item*)::{lambda(auto:1&)#1}) (World.cpp:18)
==8608==    by 0x153CB2: plugin::physics::bump::World::Remove(plugin::physics::bump::Item*) (World.cpp:224)
==8608==  Block was alloc'd at
==8608==    at 0x4849013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==8608==    by 0x159810: std::_MakeUniq<plugin::physics::bump::Item>::__single_object std::make_unique<plugin::physics::bump::Item, plugin::util::UserData&, plugin::physics::bump::Rectangle&>(plugin::util::UserData&, plugin::physics::bump::Rectangle&) (unique_ptr.h:962)
==8608==    by 0x153B7E: plugin::physics::bump::World::Add(plugin::util::UserData, plugin::physics::bump::Rectangle) (World.cpp:208)
==8608==    by 0x12F57E: plugin::physics::bump::Item* plugin::physics::bump::World::Add<TestItem>(std::shared_ptr<TestItem>, plugin::physics::bump::Rectangle) (World.hpp:113)
==8608==    by 0x12DFEC: BumpWorldTest::AddItem(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, float, float, float, float) (BumpWorldTest.hpp:83)
==8608==    by 0x1245FE: BumpWorldTest_Remove_Test::TestBody() (world_spec.cpp:140)
==8608==    by 0x19E222: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2589)
==8608==    by 0x196400: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2625)
==8608==    by 0x16D6B7: testing::Test::Run() (gtest.cc:2664)
==8608==    by 0x16E118: testing::TestInfo::Run() (gtest.cc:2842)
==8608==    by 0x16EA07: testing::TestSuite::Run() (gtest.cc:2996)
==8608==    by 0x17BBFD: testing::internal::UnitTestImpl::RunAllTests() (gtest.cc:5708)
==8608==

Hi,

If i change the code for World::Remove to the following there are no more errors in valgrind reported

what i think what the issue is with the original code (here https://github.com/Polynominal/bump.hpp/blob/main/src/World.cpp#L224 ) is, is that removeIf is being called which in turn calls std::removeIf and somehow this seems to destroy item already but item is still being used to remove the items from the cells, this was a guess but swapping around the functionality (first removing the item from the cells and then calling removeIf instead of the other way around) seems to make the errors go completely away in valgrind, which might somehow suggest this is actually the case. The stacktraces in previous post also seem to suggest this as it shows where the allocated memory was deleted (middle part) and that seems to originate from removeIf so by the time you use item after calling removeif to remove it from the celltables item seems to be already invalid.

Now the change i did in my screenshot above was only for testing as i understand it's not ideal as we loop the tables and the item might not exist, but i'm not sure there is a way to only loop the celltables if the item actually exists (before calling removeif) as otherwise it seems to have been destroyed already with how the code currently is

this is a better workaround...

The issue indeed is that after calling removeif the item gets destroyed because it's nowhere in use anymore (i think but not sure no more pointers pointing to it like there are no references anymore) but you can actually go around this issue by explicitly keeping a reference to the item before calling removeif and then using that pointer for removing it from the table cells. calling removeif will then no longer erase item and you can safely use the item reference for removing it from the cells like the original code. When the function ends the reference goes out scope and only then i think item gets destroyed, i did not actually check if that is the case but adding this code also produced no more errors in valgrind and i think this is at least a better workaround than in my previous post

edit: i checked, at the end of the program there actually indeed no memory leaks and allocs = free's. But i'm not sure how i can actually check if item already gets destroyed after World::Remove returns.

I think its a bit misleading, the data that the pointer keepItem is referencing to will be unavailable after the removeIf call removeIf call constructed this way in order to optimize the code such that we check if the item is already known to us before removing it. In some instances this may be OK provided we never touch the data that the pointer is pointing towards but I rather play on the safe side so I have adjusted the code such that we do the removeIf operation as the very last thing instead.

https://github.com/Polynominal/bump.hpp/commit/37eaf8d165692ea9683a90b3752d177fddfec9e6

Additionally regarding references, keepItem in this case is just a address to the data that's it., It won't increment the reference count of smart pointers, and in this case Item is a unique_ptr so it doesn't have reference counting anyway.

Items are owned by World.hpp as they are contained in unique_ptr that means you cannot share ownership. Once the item is removed from the vector it will be automatically deleted.

I think its a bit misleading, the data that the pointer keepItem is referencing to will be unavailable after the removeIf call removeIf call constructed this way in order to optimize the code such that we check if the item is already known to us before removing it. In some instances this may be OK provided we never touch the data that the pointer is pointing towards but I rather play on the safe side so I have adjusted the code such that we do the removeIf operation as the very last thing instead.

Hi yes, you are right, better to keep it clear..

Additionally regarding references, keepItem in this case is just a address to the data that's it., It won't increment the reference count of smart pointers, and in this case Item is a unique_ptr so it doesn't have reference counting anyway.

Items are owned by World.hpp as they are contained in unique_ptr that means you cannot share ownership. Once the item is removed from the vector it will be automatically deleted.

yeah i'm not familiar with unique_ptr i only noticed it got removed from the vector and immediatly free'd afterwards, while it would still be used afterwards. It was this that valgrind was reporting also, but i'm happy to see that with the latest commits and version i build. Valgrind no longer reports any issues at all when running bump_test through it. So thats really nice. Thanks again for your help and porting the bump.ai library, i hope i can manage to use it for what i want it to use. As far as i'm concerned the issue can be closed so i will close it now ... thanks again

Polynominal / bump.hpp

Project segfaults as soon as i link against libbump.a #1