dougmencken / HeadOverHeels

The free and open source remake of the game “Head over Heels”
GNU General Public License v3.0
33 stars 10 forks source link

Taking an item out of the handbag make the game crash with SIGABRT (Linux) #47

Closed kiwifb closed 1 year ago

kiwifb commented 1 year ago

So, now that I have the handbag I want to use it to get to the next room. But taking the item out of the bag crash the game. Full backtrace

(gdb) bt full
#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=140661107476032) at ./nptl/pthread_kill.c:44
        tid = <optimized out>
        ret = 0
        pd = 0x7fee37584640
        old_mask = {__val = {0 <repeats 16 times>}}
        ret = <optimized out>
        pd = <optimized out>
        old_mask = <optimized out>
        ret = <optimized out>
        tid = <optimized out>
        ret = <optimized out>
        resultvar = <optimized out>
        resultvar = <optimized out>
        __arg3 = <optimized out>
        __arg2 = <optimized out>
        __arg1 = <optimized out>
        _a3 = <optimized out>
        _a2 = <optimized out>
        _a1 = <optimized out>
        __futex = <optimized out>
        resultvar = <optimized out>
        __arg3 = <optimized out>
        __arg2 = <optimized out>
        __arg1 = <optimized out>
        _a3 = <optimized out>
        _a2 = <optimized out>
        _a1 = <optimized out>
        __futex = <optimized out>
        __private = <optimized out>
        __oldval = <optimized out>
        result = <optimized out>
#1  __pthread_kill_internal (signo=6, threadid=140661107476032) at ./nptl/pthread_kill.c:78
No locals.
#2  __GI___pthread_kill (threadid=140661107476032, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
No locals.
#3  0x00007fee504c3476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
        ret = <optimized out>
#4  0x00007fee504a97f3 in __GI_abort () at ./stdlib/abort.c:79
        save_stage = 1
        act = {__sigaction_handler = {sa_handler = 0x0, sa_sigaction = 0x0}, sa_mask = {__val = {0 <repeats 13 times>, 10, 
              140661528049312, 140661107476032}}, sa_flags = 1347454650, sa_restorer = 0x7fee04086458}
        sigs = {__val = {32, 0 <repeats 15 times>}}
#5  0x00007fee50852b9e in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
No symbol table info available.
#6  0x00007fee5085e20c in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
No symbol table info available.
#7  0x00007fee5085e277 in std::terminate() () from /lib/x86_64-linux-gnu/libstdc++.so.6
No symbol table info available.
#8  0x00007fee5085e4d8 in __cxa_throw () from /lib/x86_64-linux-gnu/libstdc++.so.6
No symbol table info available.
#9  0x00007fee50855344 in std::__throw_logic_error(char const*) () from /lib/x86_64-linux-gnu/libstdc++.so.6
No symbol table info available.
#10 0x00005556578b378c in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*> (__end=0x1 <error: Cannot access memory at address 0x1>, __beg=0x0, this=<optimized out>)
    at /usr/include/c++/11/bits/basic_string.tcc:206
        __dnew = <optimized out>
        __dnew = <optimized out>
#11 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string<std::allocator<char> > (__a=..., 
    __s=0x0, this=<optimized out>) at /usr/include/c++/11/bits/basic_string.h:539
        __end = 0x1 <error: Cannot access memory at address 0x1>
#12 iso::UserControlled::dropItem (this=<optimized out>, player=...) at behaviors/UserControlled.cpp:517
        freeItem = {pointer = 0x7fee04039ea0}
#13 0x00005556578a4e5f in iso::PlayerHeels::update (this=0x555658166430) at behaviors/PlayerHeels.cpp:137
        playerItem = @0x55565ac7a380: {<iso::FreeItem> = {<iso::Item> = {<iso::Mediated> = {
                _vptr.Mediated = 0x55565790f960 <vtable for iso::PlayerItem+16>, mediator = 0x5556597fab80}, <iso::Shady> = {
                _vptr.Shady = 0x55565790fa00 <vtable for iso::PlayerItem+176>, wantShadow = true}, 
              static poolOfPictures = 0x5556580d0750, descriptionOfItem = 0x5556598e0db0, 
              uniqueName = "character heels @ blacktooth27fish.xml", originalLabel = "heels", processedImage = {
    pointer = 0x55565bb4faa0}, height = 24, orientation = "west", currentFrame = 4, backwardsMotion = false, offset = {first = -185, 
                second = 110}, collisionDetector = true, motion = std::vector of length 16, capacity 16 = {{
                  pointer = 0x7fee04036670}, {pointer = 0x7fee04037d60}, {pointer = 0x7fee04039450}, {pointer = 0x7fee04022740}, {
                  pointer = 0x7fee04022b50}, {pointer = 0x7fee04022f60}, {pointer = 0x7fee04032440}, {pointer = 0x7fee04032850}, {
                  pointer = 0x7fee04032c60}, {pointer = 0x7fee04033070}, {pointer = 0x7fee04033480}, {pointer = 0x7fee04037f80}, {
                  pointer = 0x7fee04038390}, {pointer = 0x7fee040387a0}, {pointer = 0x7fee04038bb0}, {pointer = 0x7fee04038fc0}}, 
              shadows = std::vector of length 16, capacity 16 = {{pointer = 0x7fee04023780}, {pointer = 0x7fee04049030}, {
                  pointer = 0x7fee0404a830}, {pointer = 0x7fee0404bf40}, {pointer = 0x7fee0404d630}, {pointer = 0x7fee0404da50}, {
                  pointer = 0x7fee0404de70}, {pointer = 0x7fee0404e290}, {pointer = 0x7fee04029800}, {pointer = 0x7fee0402aef0}, {
                  pointer = 0x7fee0402c5e0}, {pointer = 0x7fee0402dcd0}, {pointer = 0x7fee0402f3c0}, {pointer = 0x7fee04030ab0}, {
                  pointer = 0x7fee040321a0}, {pointer = 0x7fee04073b30}}, motionTimer = std::unique_ptr<Timer> = {
                get() = 0x55565a60dc30}, behavior = 0x555658166430, anchor = "drum 1st", xYet = 51, yYet = 143, 
              zYet = 48}, <Drawable> = {_vptr.Drawable = 0x55565790fa20 <vtable for iso::PlayerItem+208>}, 
            static farFarAway = -1024, originalCellX = -1024, originalCellY = -1024, originalCellZ = -1024, wantMask = {
              value = 11}, transparency = 0 '\000', frozen = false, partOfDoor = false, shadedNonmaskedImage = {
              pointer = 0x55565bfdafc0}}, lives = 8 '\b', highSpeed = 0, highJumps = 0, 
          tools = std::vector of length 1, capacity 1 = {"handbag"}, howManyDoughnuts = 0, wayOfExit = "no exit", 
          wayOfEntry = "via second teleport", shieldTimer = std::unique_ptr<Timer> = {get() = 0x55565bf0d420}, 
          shieldRemaining = 0, descriptionOfTakenItem = 0x55565916f860, 
          behaviorOfTakenItem = "behavior of thing able to move by pushing"}
#14 0x000055565782ea9b in iso::Mediator::update (this=0x5556597fab80) at Mediator.cpp:123
        f = {pointer = 0x55565ac7a380}
        vanishedGridItems = std::vector of length 0, capacity 0
        vanishedFreeItems = std::vector of length 0, capacity 0
        playersInRoom = std::vector of length -1073585200782584444, capacity 1 = {{
            pointer = 0x55565790e430 <vtable for allegro::Pict+16>}, {pointer = 0x7fee04045930}, {pointer = 0x7fee50004500}, {
            pointer = 0x35}, {pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {pointer = 0x0}, {
            pointer = 0xffffffff00000000}, {pointer = 0x0}, {pointer = 0x0}, {pointer = 0x35}, {
            pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {pointer = 0x0}, {pointer = 0xffffffff00000000}, {
            pointer = 0x0}, {pointer = 0x0}, {pointer = 0x35}, {pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {
            pointer = 0x0}, {pointer = 0xffffffff00000000}, {pointer = 0x0}, {pointer = 0x2000000000}, {pointer = 0x35}, {
            pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {pointer = 0x0}, {pointer = 0xffffffff00000000}, {
            pointer = 0x10000000000}, {pointer = 0x3f4ececf3f800000}, {pointer = 0x35}, {
            pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {pointer = 0x0}, {pointer = 0xffffffff00000000}, {
            pointer = 0x0}, {pointer = 0x0}, {pointer = 0x35}, {pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {
            pointer = 0x0}, {pointer = 0xffffffff00000000}, {pointer = 0x55565bff6a00}, {pointer = 0x0}, {pointer = 0x25}, {
            pointer = 0x7fee04045}, {pointer = 0x40e33cf4b4e6c948}, {pointer = 0x7fee50a8b2b0 <al_destroy_bitmap>}, {
            pointer = 0x55}, {pointer = 0x7fee00000001}, {pointer = 0x7fee04046070}, {pointer = 0x7fee04035590}, {pointer = 0x0}, {
            pointer = 0x7fee04045560}, {pointer = 0x7fee04045478}, {pointer = 0xf}, {pointer = 0x7267656c6c61374e}, {
            pointer = 0x4574636950346f}, {pointer = 0x55}, {pointer = 0x700000000}, {pointer = 0x55565a60e5d0}, {pointer = 0x0}, {
            pointer = 0x0}, {pointer = 0x55565a615ed0}, {pointer = 0x7fee04044ea0}, {pointer = 0x7fee04044ea0}, {
            pointer = 0x7fee04044ea8}, {pointer = 0x0}, {pointer = 0x55}, {pointer = 0x7fee00000000}, {pointer = 0x5556598c7ea0}, {
            pointer = 0x0}, {pointer = 0x0}, {pointer = 0x55565a615ed0}, {pointer = 0x7fee04045518}, {pointer = 0xf}, {
            pointer = 0x7267656c6c61374e}, {pointer = 0x4574636950346f}, {pointer = 0x35}, {pointer = 0x55565bf02478}, {
            pointer = 0x7fee040000b0}, {pointer = 0x0}, {pointer = 0x55565991e430}, {pointer = 0x30}, {pointer = 0x25}, {
            pointer = 0x55565b877f38}, {pointer = 0x0}, {pointer = 0x7fee5000456d}, {pointer = 0x35}, {
            pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {pointer = 0x0}, {pointer = 0xffffffff00000000}, {
            pointer = 0x0}, {pointer = 0x4000000000}, {pointer = 0x35}, {
            pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {pointer = 0x0}, {pointer = 0xffffffff00000000}, {
            pointer = 0x4000000000}, {pointer = 0x0}, {pointer = 0x35}, {
            pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {pointer = 0x0}, {pointer = 0xffffffff00000000}, {
            pointer = 0x4}, {pointer = 0x3f800000}, {pointer = 0x35}, {pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {
            pointer = 0x0}, {pointer = 0xffffffff00000000}, {pointer = 0x3f800000}, {pointer = 0x0}, {pointer = 0x35}, {
            pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {pointer = 0x0}, {pointer = 0xffffffff00000000}, {
            pointer = 0x0}, {pointer = 0x0}, {pointer = 0x35}, {pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {
            pointer = 0x0}, {pointer = 0xffffffff00000000}, {pointer = 0x0}, {pointer = 0x0}, {pointer = 0x35}, {
            pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {pointer = 0x0}, {pointer = 0xffffffff00000000}, {
            pointer = 0x3f800000}, {pointer = 0x3f800000}, {pointer = 0x35}, {
            pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {pointer = 0x0}, {pointer = 0xffffffff00000000}, {
            pointer = 0x0}, {pointer = 0x0}, {pointer = 0x35}, {pointer = 0x55565790e450 <vtable for allegro::Sample+16>}, {
            pointer = 0x0}, {pointer = 0xffffffff00000000}, {pointer = 0x55565bfd28c0}, {pointer = 0x0}, {pointer = 0x1b5}, {
            pointer = 0x0}, {pointer = 0x2190000000a}, {pointer = 0x0}, {pointer = 0x0}, {pointer = 0x4000000000}, {
            pointer = 0x10000000028}, {pointer = 0x4000000000}, {pointer = 0x2800000000}, {pointer = 0x0}, {
            pointer = 0x4000000000}, {pointer = 0x28}, {pointer = 0x7fee0409b730}, {pointer = 0x1}, {pointer = 0x7fee0409b730}, {
            pointer = 0x1000000000a}, {pointer = 0x4}, {pointer = 0x3f800000}, {pointer = 0x0}, {pointer = 0x3f80000000000000}, {
            pointer = 0x0}, {pointer = 0x0}, {pointer = 0x3f800000}, {pointer = 0x0}, {pointer = 0x3f80000000000000}, {
            pointer = 0x3f800000}, {pointer = 0x0}, {pointer = 0x3f80000000000000}, {pointer = 0x0}, {pointer = 0x0}, {
            pointer = 0x3f800000}, {pointer = 0x0}, {pointer = 0x3f80000000000000}, {pointer = 0x3d00000000000000}, {
            pointer = 0x0}, {pointer = 0x0}, {pointer = 0xbd4ccccd}, {pointer = 0x0}, {pointer = 0x3f80000000000000}, {
            pointer = 0xbf80000000000000}, {pointer = 0x3f800000}, {pointer = 0x3f800000}, {pointer = 0x0}, {pointer = 0x0}, {
            pointer = 0x0}, {pointer = 0x0}, {pointer = 0x0}, {pointer = 0x0}, {pointer = 0x0}, {pointer = 0x0}, {
            pointer = 0x7fee0409b730}, {pointer = 0x0}, {pointer = 0x55565bfc7710}, {pointer = 0x0}, {pointer = 0x55}, {
            pointer = 0x700000001}, {pointer = 0x55565a793400}, {pointer = 0x55565a796140}, {pointer = 0x55565a795fe0}, {
            pointer = 0x55565a795f70}, {pointer = 0x7fee04046ad0}, {pointer = 0x7fee04046ad0}, {pointer = 0x7fee04046ad8}...}
#15 0x000055565782f128 in iso::Mediator::updateThread (mediatorAsVoid=0x5556597fab80) at Mediator.cpp:196
        mediator = 0x5556597fab80
#16 0x00007fee50515b43 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
        ret = <optimized out>
        pd = <optimized out>
        out = <optimized out>
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140725039088960, -1007450437748141918, 140661107476032, 11, 140661526452304, 
                140725039089312, 997757987058995362, 997672148870353058}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 
              0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
        not_first_call = <optimized out>
#17 0x00007fee505a7a00 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
No locals.
kiwifb commented 1 year ago

I have a suspicion that taking the object out and lifting Heels in the process is not properly handled. But that's just a gut feeling at this point.

dougmencken commented 1 year ago

can’t see any other string copying within UserControlled::dropItem but this one

https://github.com/dougmencken/HeadOverHeels/blob/d870b90238afd61012338c2aebf38efce45f893e/src/behaviors/UserControlled.cpp#L496

may it be that player.getDescriptionOfTakenItem() is not nil, but player.getDescriptionOfTakenItem()->getLabel() is nil ?

kiwifb commented 1 year ago

Commenting it out did not fix the issue. But I should have posted the terminal output with it.

took item "drum 1st"
free item "drum 1st" is to be gone
removing free item "drum 1st" from room "blacktooth27fish.xml"
drop item "drum"
creation of behavior "behavior of thing able to move by pushing" for free item "drum.cVhbtyvXsc1P" ( "drum" ) at x=52 y=143 z=24 with orientation "nowhere"
free item "drum 2nd" is yet in room "blacktooth27fish.xml"
terminate called after throwing an instance of 'std::logic_error'
  what():  basic_string::_M_construct null not valid
Aborted (core dumped)

So, we can see the line you quote is displaying without issue.

kiwifb commented 1 year ago

The last line before "terminate" is in https://github.com/dougmencken/HeadOverHeels/blob/321bcbd0aa9196adbb8631a49290f50d64eb2250/src/Room.cpp#L731

kiwifb commented 1 year ago

We do not get past that line https://github.com/dougmencken/HeadOverHeels/blob/79a341e0df078709380f2f522e64b0c39eebc44e/src/behaviors/UserControlled.cpp#L512 and it is strange. player.placeItemInBag? OK I get it, it is trying to reset the content of the bag to empty. But it is not working.

kiwifb commented 1 year ago

And if I comment the line, I get multiple copies of the object :) until it crashes because I go through the roof. I was thinking that may be it did not like the nilPointer in this line https://github.com/dougmencken/HeadOverHeels/blob/79a341e0df078709380f2f522e64b0c39eebc44e/src/PlayerItem.cpp#L573 but it looks like we do not get there because it is trying and failing to cast the nilPointer to const std::string& when entering the method https://github.com/dougmencken/HeadOverHeels/blob/79a341e0df078709380f2f522e64b0c39eebc44e/src/PlayerItem.cpp#L571

dougmencken commented 1 year ago

news from my side : with https://github.com/dougmencken/HeadOverHeels/commit/af1192edfef573e3c98a9605bd3f67a12b1dbd54 I finally got it to compile (with gcc 12.3.0 currently here on ubuntu), but it doesn't link pouring out with errors for each symbol

that's for plain default autoreconf -f -i && ./configure && make

currently I am pushing patches without really trying them, but it's blind, thus may I ask you how do you build it on Linux

kiwifb commented 1 year ago

I am currently building on kubuntu 22.04 which is gcc-11.4. I could try on my gentoo boxes (gcc-12.3.1 and gcc-13.2.0). I have installed liballegro5-dev and configure against that.

autoreconf -fi
./configure --prefix=/home/fbissey/Work/HeadOverHeels/_rootdir --with-allegro5 --enable-debug
make
make install
./_rootdir/bin/headoverheels

I can also build using the linux-build.sh script, slightly patched

@@ -80,7 +80,7 @@ gameInstallPath="${buildFolder}"/_rootdir

 if [ ! -f src/Makefile ]
 then
-        LDFLAGS="-L${allegro4installpath}/lib" \
+        LDFLAGS="-L${allegro4installpath}/lib -Wl,-rpath ${allegro4installpath}/lib" \
         CFLAGS="-I${allegro4installpath}/include" \
         CXXFLAGS="${CFLAGS}" \
         ./configure \

More controversially, I am using the following

diff --git a/configure.ac b/configure.ac
index 869f30f..eedcd9b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -84,6 +84,7 @@ AM_CONDITIONAL([DARWIN], [test x"$on_darwin" = x"true"])
 AC_OUTPUT( \
             Makefile \
             src/Makefile \
+            src/config_exe.h \
             gamedata/Makefile \
             gamedata/gfx/Makefile \
             gamedata/gfx.2009/Makefile \
diff --git a/src/Ism.cpp b/src/Ism.cpp
index 93cb48f..109e74a 100644
--- a/src/Ism.cpp
+++ b/src/Ism.cpp
@@ -1,5 +1,6 @@

 #include "Ism.hpp"
+#include "config_exe.h"

 #include <algorithm>

@@ -115,7 +116,7 @@ void setPathToGame ( const char * pathToGame )
 #if defined ( __CYGWIN__ )
                 FullPathToGame = "/bin/headoverheels" ;
 #else
-                FullPathToGame = "/usr/bin/headoverheels" ;  /* hard-coded */
+                FullPathToGame = ConfigureExecutablePatch ;  /* hard-coded */
                                                              /* for other paths like /usr/local
                                                                 and~or custom names of game
                                                                 it needs more sophiscated logic */

and src/config_exe.h.in

// Location of the executable of the game

#define ConfigureExecutablePatch "@prefix@/bin/headoverheels"

It needs a bit of work, because there are plenty of way that can break, but the game knows where to find its stuff without issues, so long as you do not use more exotic configure flags like --bindir.

kiwifb commented 1 year ago

The easy way to fix the bag emptying problem is to have a special function to empty the bag.

diff --git a/src/PlayerItem.cpp b/src/PlayerItem.cpp
index 93043e4..01b5c3c 100644
--- a/src/PlayerItem.cpp
+++ b/src/PlayerItem.cpp
@@ -575,6 +575,12 @@ void PlayerItem::placeItemInBag ( const std::string& labelOfItem, const std::str
         this->behaviorOfTakenItem = behavior ;
 }

+void PlayerItem::emptyBag ()
+{
+        this->descriptionOfTakenItem = nilPointer ;
+        this->behaviorOfTakenItem = "still" ;
+}
+
 void PlayerItem::save ()
 {
         GameManager::getInstance().eatFish( *this, this->mediator->getRoom() );
diff --git a/src/PlayerItem.hpp b/src/PlayerItem.hpp
index 11727f6..3e78fcf 100644
--- a/src/PlayerItem.hpp
+++ b/src/PlayerItem.hpp
@@ -120,6 +120,8 @@ public:

         void placeItemInBag ( const std::string & labelOfItem, const std::string & behavior ) ;

+        void emptyBag () ;
+
         const DescriptionOfItem * getDescriptionOfTakenItem () const {  return descriptionOfTakenItem ;  }

         const std::string & getBehaviorOfTakenItem () const {  return this->behaviorOfTakenItem ;  }
diff --git a/src/behaviors/UserControlled.cpp b/src/behaviors/UserControlled.cpp
index c22e3cc..6efe50b 100644
--- a/src/behaviors/UserControlled.cpp
+++ b/src/behaviors/UserControlled.cpp
@@ -509,7 +509,7 @@ void UserControlled::dropItem( PlayerItem & player )

                         GameManager::getInstance().emptyHandbag();

-                        player.placeItemInBag( nilPointer, "still" );
+                        player.emptyBag();

                         // update activity
                         activity = ( activity == Activity::DropAndJump ? Activity::Jump : Activity::Wait );

Not elegant but it works.

dougmencken commented 1 year ago

Not elegant

emptyBag is much more elegant than placeInBag( nil ) imho, I really like your solution

dougmencken commented 1 year ago

I hope there're no any more nil references ;)

closing this?

kiwifb commented 1 year ago

Yes. I am moving on to some different issue. You should get a .gitignore file, let me know if you'd like me to compose one.