views:

270

answers:

3

Hello,

I wrote a program some time ago (Mac OS X, C++, SDL, FMOD) and it perfomed rather good. But lately I wanted to extend its functionality and added some more code to it. And now, when I run it and try to test the new functionality, the program crashes with SIGABRT.

Looking into debugger, on function stack I see:

  • _kill
  • kill$UNIX2003
  • raise
  • __abort
  • __stack_chk_fail
  • odtworz <-- my function that was was modified

As far as I know, "__stack_chk_fail" indicates a stack overflow. But that's not the weirdest thing about it. In this function "odtworz", I have some code like this:

...

koniec = 0;
while ( koniec == 0 ) {
    ...
    if (mode == 1) {
        ...
    }
    else if (mode == 2) {
        ...
    }
    else if (mode == 3) {
       piesniOrkiestrowe[0] = '\0'; 
       while ( piesniOrkiestrowe[0] == '\0' ) { 
           losowaPiesn(); 
           char * piesnOrkiestrowa = szukajPiesniOrkiestrowej(); 
           if ( piesnOrkiestrowa != NULL ) 
              strcpy(piesniOrkiestrowe, piesnOrkiestrowa); 
       } 
       char nowyPiesnPlik[25]; 
       sprintf(nowyPiesnPlik, "%sorch/%s", PIESNI_DIR.c_str(), piesniOrkiestrowe);
    }
}

mode is a global variable and is set to value "2" in a function before. And now imagine - if I delete the third if statement (mode == 3) which never gets executed in this mode, the program doesn't crash! Deleting code that doesn't even get to be executed helps the situation!

Now, I don't want to delete this code because it's for other mode of my program. And it works fine there. So any hints where I can search? What could be possibly wrong with this?

+11  A: 

It is not a stack overflow error. __stack_chk_fail is called when stack frame corruption is detected. The traditional way to smash the stack is a buffer overflow. The code that causes it is not in your snippet, it is in the dots.


After updating the question with code from a comment: both the strcpy and the sprintf calls are excellent candidates for stack corruption. The buffer overflow problem I mentioned in my original answer. Taking a guess: nowyPiesnPlik looks very small. The sprintf() function will write too many characters to the buffer and overwrite the "canary". When the canary gets stomped on, the runtime will whistle fowl :)

You could make the array bigger. Not a real solution, use safe alternatives for these functions, like snprintf(). I'll avoid mentioning strncpy().

Hans Passant
And do you have any ideas why does erasing the `mode == 3` part helps in his case?
Kotti
It doesn't, it just hides the fact that a buffer overflow is occuring.
wheaties
Are there any local variables in the ... in mode == 3? If so, removing that branch may affect the stack layout.
Arkadiy
The code in mode == 3 that I erase looks like this: piesniOrkiestrowe[0] = '\0'; while ( piesniOrkiestrowe[0] == '\0' ) { losowaPiesn(); char * piesnOrkiestrowa = szukajPiesniOrkiestrowej(); if ( piesnOrkiestrowa != NULL ) strcpy(piesniOrkiestrowe, piesnOrkiestrowa); } char nowyPiesnPlik[25]; sprintf(nowyPiesnPlik, "%sorch/%s", PIESNI_DIR.c_str(), piesniOrkiestrowe);
mav
@mav: Probably in `mode == 3` some variables are declared or such, so that the layout of the stack frame ends up differently if this code is included. And then some other part of the stack happens to be corrupted, not the part that the stack checking function verifies.
sth
A: 

Not strange at all. When it comes to stack overflows or heap corruption, you should expect strange. The stack pointer, program counter, or other program state has been corrupted, so the debugger or trace tool can not accurately report where the program was at the time of the crash. The bug is likely elsewhere in your code, far away from the snippet you posted. Start with the most recently modified code.

Edit: You've already written a nice example of stack corruption yourself, as you've since discovered. Here's one anyway:

void foo (){ 
    int x[0];
    x[-99] = -1;
}
John
Could you please give me a few example of code that can possibly cause stack corruption? My program makes heavy use of cstrings and Strings.
mav
A: 

Found it!

The culrpit was before code that I given, but Hans Passant gave me a clue what to look. It looked like this:

char piesnPlik[25];
if ( mode == TRYB_PIANINO )
    sprintf(piesnPlik, "%spiano/%s.mp3", PIESNI_DIR.c_str(), wybranaPiesn);
else if ( tryb == TRYB_ORKIESTRA )
    sprintf(piesnPlik, "%sorch/%s", PIESNI_DIR.c_str(), piesniOrkiestrowe);
else if ( tryb == TRYB_NAGRANIE )
    sprintf(piesnPlik, "%s/%s", NAGRANIA_DIR.c_str(), nazwaNagraniaMP3);

So, I added today the third if which user "piesnPlik" variable. But "nazwaNagraniaMP3" was longer than the two other variables that were copied there, so it corrupted the stack. But it's unbelievable that it managed to work with all SDL stuff after, only to crash after returning from the function.

Thanks all for Your suggestions!

mav