Jump to content
  • Advertisement
Sign in to follow this  
kaprikawn

Help with debugging segfault

This topic is 900 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi, I'm trying to learn OOP / C++ and am following a book called SDL Game Development by Shaun Mitchell.  I'm at a stage where I'm managing the game state, essentially just transitioning from a menu state to a play state, but I'm getting a segmentation fault which I'm really struggling to debug.

 

I have read the guidelines for new posters and would like to try and post a snippet of my code, but I don't think that's possible.  I've put my code here for viewing :

 

https://www.dropbox.com/sh/4gaw5pyyipeohq8/AAAbCSvINc76aRpyKAGE8vrra?dl=0

 

I hope this is an acceptable way of doing this, please let me know if there's a better way of doing it.

 

The segfault occurs after you click the 'Play' button, what happens is :

 

- Game.h contains a pointer to an instance of the GameStateMachine class (m_pGameStateMachine)

- GameStateMachine contains a vector of GameStates, the last element of which is always the current gamestate (in my current code it never has more than one element in).  The menu gamestate gets loaded into this initially.

- When you click the 'Play' button, triggering s_menuToPlay from the MenuState.cpp file (line 55), that calls changeState in GameStateMachine.cpp line 18.

- This deletes the MenuState instance from the vector and replaces it with a PlayState instance.

- In Game.cpp is an update function which is in the main game loop which calls the update function of the current gamestate.  Initially this calls the update function of the MenuState gamestate, after the click and transition from MenuState to PlayState, it should then call the update function of the PlayState instance.

 

However, from performing a backtrace on the segfault, it seems that the update function of the MenuState is still being called after it has been deleted and replaced with the PlayState instance.  It seems pretty likely that this is the cause of the segfault, but I can't for the life of me figure out why that code is being called by that point.

 

I know this is a big ask, but any help on debugging this would be greatly appreciated.  If going through my code is a bit much, if there's any specific advice on debugging segfaults that you could recommend that I couldn't get from googling 'How to debug segfaults' then that might help.

 

For reference, the code is C++ / SDL2, being compiled using clang (also tried g++) on latest version of Mint Linux.  The output of running gdb on my file is :

SDL successfully initialised
window created successfully
Renderer created successfully
Initialised 0 joystick(s)
Changing game state to MENU
entering MenuState
game init success
gameState currently MENU
Changing game state to PLAY
exiting MenuState
entering PlayState
gameState is now PLAY

Program received signal SIGSEGV, Segmentation fault.
0x000000000040c579 in MenuState::update (this=0x9b9660) at MenuState.cpp:13
13	    m_gameObjects[i] -> update();
(gdb) backtrace
#0  0x000000000040c579 in MenuState::update (this=0x9b9660) at MenuState.cpp:13
#1  0x000000000040af5e in GameStateMachine::update (this=0x9a7370) at GameStateMachine.cpp:49
#2  0x000000000040c1b9 in Game::update (this=0x617070) at Game.cpp:68
#3  0x000000000040e874 in main (argc=1, args=0x7fffffffe118) at Main.cpp:20

As you can see, the update loop is going

Main -> Game -> GameStateMachine -> MenuState

The console output above that shows that the PlayState has successfully initialized (and I've done all the checks I can think of to make sure that is the case).  Therefore it, at that point, should be going

Main -> Game -> GameStateMachine -> PlayState

 

If someone could point me in the right direction that would be great, thanks in advance.

Share this post


Link to post
Share on other sites
Advertisement

Contents of m_gameObjects:

  GameObject* button1 = new MenuButton( new LoaderParams( 100, 100, 400, 100, "playbutton" ), s_menuToPlay );
  GameObject* button2 = new MenuButton( new LoaderParams( 100, 300, 400, 100, "exitbutton" ), s_exitFromMenu );
  
  m_gameObjects.push_back( button1 );
  m_gameObjects.push_back( button2 );

Process of MenuState::update()

void MenuState::update() {
  for( int i = 0; i < m_gameObjects.size(); i++ ) {
    m_gameObjects[i] -> update();
  }
}

So that's going to check them in order, yes?

 

But when you click the first button there you delete the MenuState object. So the loop goes:

 

i = 0

m_gameObjects[0]->update(); //check the s_menuToPlay button - delete the menu state

i = 1

m_gameObjects[1]->update(); //m_gameObjects doesn't exist anymore. segfault

 

-

 

Please get much more cozy with constructors and destructors and try to avoid singletons.

 

Incidentally, you're leaking memory there since you're using new to allocate the buttons and then never deleting them.

 

In theory it could also fail on .size() in the loop header, but we're in undefined behavior land, so anything can happen.

 

There are a few approaches that could clean this up, but the most direct one would be to have an activated button place its callback in an event queue and execute the queue after you're outside of the objects that it may manipulate. If you're around later check in the chat and see if I'm around. We can talk about ctors/dtors and a better memory management strategy for this that may save you some trouble.

Share this post


Link to post
Share on other sites

Thanks for the reply.  I've now fixed the issue but I don't understand how what I've done has fixed it.

 

As I mentioned in my post, the current gamestate is held as the last element in a vector.  In the changestate function, my segfaulting code did this as part of changing one gamestate to the other :

delete m_gameStates.back();
m_gameStates.pop_back();

I didn't know what 'delete' did initially so I read up on it, and my understanding was that it basically got rid of the instance of the class, and freed up the memory.

 

The fix was to comment out the 'delete' line, so now my code just does the pop_back and now it works as intended.  I'm not 100% sure why this fixed it, and I'm not really that concerned about that specifically for now.  What I am concerned about is the potential for a memory leak (if my understanding of memory leaks is correct).  Is this concern founded?  Is doing the 'pop_back' function to delete the position in the vector sufficient to also delete whatever was in there too?  Or is my menustate instance still floating around in memory somewhere not having been properly cleaned up?

Share this post


Link to post
Share on other sites

Oh, something weird is going on here.  I thought I read your reply on my phone this morning which had significantly different content if I remember correctly.  And I posted the reply above not having re-read your reply.  But now I've gone back over it and it seems a lot more specific (did you edit your reply or something?  Or maybe it was a different reply that has since been deleted?).

 

Anyway, thanks for your help, I'll try to take on board your feedback.  I think I'm ok with constructors (at least for the level I'm currently at).  But yes, destructors is definately something I need to look at.

 

Why should I avoid singletons?  The book I'm following seemed to like them, and in the little I've done so far they seem to make sense.  Are they considered bad practice generally?

Edited by kaprikawn

Share this post


Link to post
Share on other sites

There are many reasons to avoid singletons, but the specific one that made me mention it here is that the singleton spaghetti was the route that you used to pull the rug out from under yourself (there are certainly other ways of doing this).

 

Simply not deleting the state is going to add another (more significant) leak. You're fixing one problem by introducing another one.

 

Apparently you're not coming to the chat? I was going to suggest that if you want to be able to pull the rug out from under yourself like this you could use shared_ptr, but honestly it would be better to just correct the structure.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!