CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 12 of 12

Hybrid View

  1. #1
    Join Date
    Nov 2004
    Location
    Lincoln, NE
    Posts
    516

    [Solved] Completely stumped by stack smash.

    For some reason, I'm getting a segmentation fault when main() exits with the following code:

    Code:
    #include <iostream>
    #include <cstring>
    #include <ctime>
    #include <cstdlib>
    #include <unistd.h>
    
    using std::cout;
    using std::endl;
    using std::flush;
    using std::fill;
    
    void moveTortoise(int *);
    void moveHare(int *);
    void race(char *);
    void checkCollision(int *,int * ,char *);
    bool isWinner(int *, int *);
    
    const int TICK_MS = 10000;
    const int TRACK_LENGTH = 70;
    const int FINISH = TRACK_LENGTH - 1;
    const char HARE_CHAR = 'H';
    const char TORTOISE_CHAR = 'T';
    
    int main() {
    	char track[TRACK_LENGTH + 1] = {0};
    
    	srand(time(0));
    
    	std::fill(track, track + FINISH, 45);
    
    	track[0] = HARE_CHAR;
    	cout << "BANG !!!!!\nAND THEY'RE OFF !!!!!" << endl;
    	race(track);
    	return 0;
    }
    
    void race(char *track) {
    	int hare_position = 0;
    	int tortoise_position = 0;
    
    	 while (!isWinner(&hare_position, &tortoise_position)) {
    		usleep(TICK_MS);
    		track[tortoise_position] = '-';
    		track[hare_position] = '-';
    		moveTortoise(&tortoise_position);
    		moveHare(&hare_position);
    		track[tortoise_position] = TORTOISE_CHAR;
    		track[hare_position] = HARE_CHAR;
    		checkCollision(&hare_position, &tortoise_position, track);
    		cout << "\r" << track << flush;
    	}
    }
    
    void moveTortoise(int *position) {
    	int move  = 0;
    	move = 1 + rand() % 10;
    	move <= 5 ? *position += 3 : (move <= 7 ? *position -= 6 : *position++);
    	*position = *position > 0 ? (*position > FINISH ? FINISH : *position) : 0;
    }
    
    void moveHare(int *position) {
    	int move = 1 + rand() % 10;
    	move == 1 ? *position -= 12 : (move <= 3 ? *position -= 2 : (move <= 6 ? *position++ : (move <= 8 ? *position += 9 : 0)));
    	*position = *position > 0 ? (*position > FINISH ? FINISH : *position) : 0;
    }
    
    void checkCollision(int *hare, int *tortoise, char *track) {
    	static int start = 0;
    
    	if (start > 0)	{
    	   std::fill(track + start, track + start + 7, 45);
    		start = 0;
    	}
    
    	if (*hare == *tortoise) {
    		start = *hare > 15 ? *hare - 8 : *hare + 2;
    		strncpy(track + start, "OUCH!!!", 7);
    	}
    }
    
    bool isWinner(int *hare, int *tortoise) {
    	if ((*hare == FINISH) && (*tortoise == FINISH)) {
    		cout << endl << "It's a tie." << endl;
    		return true;
    	}
    	if (*hare == FINISH) {
    		cout << endl << "Hare wins. Yuch." << endl;
    		return true;
    	}
    	if (*tortoise == FINISH) {
    		cout << endl << "TORTOISE WINS!!! YAY!!!" << endl;
    		return true;
    	}
    	return false;
    }
    I can't find anywhere that I would be overwriting the array bound, and everything runs perfectly until the program attempts to exit. Valgrind reports:
    Code:
    ==5972== Invalid read of size 8
    ==5972==    at 0x400A69: main (in /[path]/race)
    ==5972==  Address 0x6fffffff8 is not stack'd, malloc'd or (recently) free'd
    ==5972== 
    ==5972== 
    ==5972== Process terminating with default action of signal 11 (SIGSEGV)
    ==5972==  Access not within mapped region at address 0x6FFFFFFF8
    ==5972==    at 0x400A69: main (in /[path]/race)
    ==5972==  If you believe this happened as a result of a stack
    ==5972==  overflow in your program's main thread (unlikely but
    ==5972==  possible), you can try to increase the size of the
    ==5972==  main thread stack using the --main-stacksize= flag.
    ==5972==  The main thread stack size used in this run was 8388608.
    ==5972== 
    ==5972== HEAP SUMMARY:
    ==5972==     in use at exit: 0 bytes in 0 blocks
    ==5972==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
    ==5972== 
    ==5972== All heap blocks were freed -- no leaks are possible
    ==5972== 
    ==5972== For counts of detected and suppressed errors, rerun with: -v
    ==5972== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 2 from 2)
    Segmentation fault (core dumped)
    I realize that using vectors instead of pointers would be the definative solution, but I'm a noob taking a C++ class and my assignment calls for use of char arrays and pointer passing. My environment is Linux Mint 13 using g++ 4.6.3-1 with default compile options (g++ file.cpp). I suspect I might be passing a bad array to cout, in that when I display the array positions returned by moveTortoise() and moveHare(), it doesn't seg fault on exit:
    Code:
                    //SNIP
    		moveTortoise(&tortoise_position);
    		moveHare(&hare_position);
    		//Adding the line below equals no segmentation fault.
    		cout << tortoise_position << " " << hare_position;
    		track[tortoise_position] = TORTOISE_CHAR;
    		track[hare_position] = HARE_CHAR;
                    //SNIP
    Any ideas?
    Last edited by Comintern; February 10th, 2013 at 11:07 AM. Reason: Mark as solved.

  2. #2
    Join Date
    Apr 1999
    Posts
    27,449

    Re: Completely stumped by stack smash.

    Quote Originally Posted by Comintern View Post
    For some reason, I'm getting a segmentation fault when main() exits with the following code:
    Mishandling pointers, buffer overrun, etc.
    I realize that using vectors instead of pointers would be the definative solution,
    It would not be a definitive solution, since overstepping the boundaries of an array and overstepping the boundaries of a vector result in the same thing -- undefined behaviour.

    The only thing that vector takes care of is handling dynamically allocated memory in the form of an array (which of course is a big advantage). However, if you go and try to access element, say 10, of a vector that only can hold 9 items, then you get the same types of bugs as if you used an array. Unless the vector has extra checking in some "debug" mode, then you're no better off.

    Nowhere in your code do you check to see if those indices inside your arrays or offsets into your arrays are in-bounds. You can't do this by sight -- actually write the code to test for a boundary conditions. You could have just checked right when the functions are called to check if the values passed are valid. If you started to check your indices carefully when you run the program, then maybe you will find the error after debugging.
    Code:
    void checkCollision(int *hare, int *tortoise, char *track) 
    {
    	static int start = 0;
    
    	if (start > 0)	{
    	   std::fill(track + start, track + start + 7, 45);
    		start = 0;
    	}
    
    	if (*hare == *tortoise) {
    		start = *hare > 15 ? *hare - 8 : *hare + 2;
    		strncpy(track + start, "OUCH!!!", 7);
    	}
    }
    Where is the check to make sure that "start" and "start + 7" are in-bounds? I'm sure that you can't check this by sight -- the best way to know and to ensure it doesn't go over the edge is to write the code to test and adjust to make sure "start" is within bounds.

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; February 10th, 2013 at 05:19 AM.

  3. #3
    Join Date
    Nov 2004
    Location
    Lincoln, NE
    Posts
    516

    Re: Completely stumped by stack smash.

    Quote Originally Posted by Paul McKenzie View Post
    Mishandling pointers, buffer overrun, etc.

    Code:
    void checkCollision(int *hare, int *tortoise, char *track) 
    {
    	static int start = 0;
    
    	if (start > 0)	{
    	   std::fill(track + start, track + start + 7, 45);
    		start = 0;
    	}
    
    	if (*hare == *tortoise) {
    		start = *hare > 15 ? *hare - 8 : *hare + 2;
    		strncpy(track + start, "OUCH!!!", 7);
    	}
    }
    Where is the check to make sure that "start" and "start + 7" are in-bounds? I'm sure that you can't check this by sight -- the best way to know and to ensure it doesn't go over the edge is to write the code to test and adjust to make sure "start" is within bounds.

    Regards,

    Paul McKenzie
    That function was the first culprit that I looked at, but removing the function entirely gives the same crash. I check bounds on the parameters before they are passed to the function, which should limit start to a range of 2-61. These two lines should keep the memory fills in bounds:
    Code:
    *position = *position > 0 ? (*position > FINISH ? FINISH : *position) : 0;
    ...
    start = *hare > 15 ? *hare - 8 : *hare + 2; //*hare is the passed parameter that gets set by *position above.

  4. #4
    Join Date
    Jan 2006
    Location
    Singapore
    Posts
    6,765

    Re: Completely stumped by stack smash.

    Quote Originally Posted by Paul McKenzie
    The only thing that vector takes care of is handling dynamically allocated memory in the form of an array (which of course is a big advantage). However, if you go and try to access element, say 10, of a vector that only can hold 9 items, then you get the same types of bugs as if you used an array. Unless the vector has extra checking in some "debug" mode, then you're no better off.
    Or you switch to using the at() member function instead of overloaded operator[], but then you still have the problem, except that you might be able to recover from it (and avoid undefined behaviour), at the cost of checks that would be unnecessary if you ensured that you stay within the bounds.
    C + C++ Compiler: MinGW port of GCC
    Build + Version Control System: SCons + Bazaar

    Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
    Kindly rate my posts if you found them useful

  5. #5
    2kaud's Avatar
    2kaud is offline Super Moderator Power Poster
    Join Date
    Dec 2012
    Location
    England
    Posts
    7,822

    Re: Completely stumped by stack smash.

    Does the program fail every time it runs or just sometimes? I've tried it under Windows VS (only changing usleep) and it works OK. Also, in main I've put a loop around race(..) and let it run a few thousand times with a very small TICK_MS and it again works OK. I've added assertions for bounds checking and again it just works OK!

    Just as an experiment, try replacing cout with printf(...) and see what happens.

  6. #6
    Join Date
    Nov 2004
    Location
    Lincoln, NE
    Posts
    516

    Re: Completely stumped by stack smash.

    Quote Originally Posted by 2kaud View Post
    Does the program fail every time it runs or just sometimes? I've tried it under Windows VS (only changing usleep) and it works OK. Also, in main I've put a loop around race(..) and let it run a few thousand times with a very small TICK_MS and it again works OK. I've added assertions for bounds checking and again it just works OK!

    Just as an experiment, try replacing cout with printf(...) and see what happens.
    It crashes every time it runs. Looping around race() doesn't cause it to crash any earlier for me either -- it's always when main exits.

    Changing the printf() gives the same segmentation fault in Linux. Oddly, I just compiled it in Visual Studio and the Windows executable was just fine for me too. It only seems to be the Linux compile that has issues. I tried compiling with -O0 to disable optimizations, but I get the same result (with albeit a larger binary).

    I already turned the assignment in (quiting with exit(0); instead of return 0; to keep it from crashing), I'd mainly just like to figure out what is going wrong.

  7. #7
    Join Date
    Oct 2008
    Posts
    1,456

    Re: Completely stumped by stack smash.

    I've not read all the code, but surely this will cause a crash

    Code:
    move <= 5 ? *position += 3 : (move <= 7 ? *position -= 6 : *position++);
    the part in red increment the pointer not the pointee. Curiously, your bound checking code hides the effect by silently correcting the position value in memory; in turn, this could give an invalid read/write from some protected memory area or could simply bite you later when the corrupted memory is accessed by something else ( in your case the program termination code ).

    BTW, if you had used an "if()" instead of the "?:" operator it would have been easier to spot ...
    Last edited by superbonzo; February 10th, 2013 at 10:32 AM.

  8. #8
    Join Date
    Apr 1999
    Posts
    27,449

    Re: Completely stumped by stack smash.

    Quote Originally Posted by Comintern View Post
    It crashes every time it runs. Looping around race() doesn't cause it to crash any earlier for me either -- it's always when main exits.
    To add to what superbonzo discovered, why are you writing code like this anyway?
    Code:
    void moveTortoise(int *position) {
    	int move  = 0;
    	move = 1 + rand() % 10;
    move <= 5 ? *position += 3 : (move <= 7 ? *position -= 6 : *position++);
    	*position = *position > 0 ? (*position > FINISH ? FINISH : *position) : 0;
    }
    
    void moveHare(int *position) {
    	int move = 1 + rand() % 10;
    	move == 1 ? *position -= 12 : (move <= 3 ? *position -= 2 : (move <= 6 ? *position++ : (move <= 8 ? *position += 9 : 0)));
    	*position = *position > 0 ? (*position > FINISH ? FINISH : *position) : 0;
    }
    What's wrong with writing this in a clear, unobfuscated manner, so that it is understood what is being done? You aren't saving any time writing things on one line like that, and to add to the confusion, throwing in the ternary operator into the whole mix. It doesn't cost anything to make things clear by writing 3 or 4 if-then statements.

    I don't know what your goal was to write code this way, but a lot of people who do write like this believe they can outsmart the compiler by believing "one line means speed". Instead, as in your case, it did nothing except introduce bugs.

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; February 10th, 2013 at 10:41 AM.

  9. #9
    Join Date
    Nov 2004
    Location
    Lincoln, NE
    Posts
    516

    Re: Completely stumped by stack smash.

    Quote Originally Posted by Paul McKenzie View Post
    To add to what superbonzo discovered, why are you writing code like this anyway?
    What's wrong with writing this in a clear, unobfuscated manner, so that it is understood what is being done? You aren't saving any time writing things on one line like that, and to add to the confusion, throwing in the ternary operator into the whole mix. It doesn't cost anything to make things clear by writing 3 or 4 if-then statements.

    I don't know what your goal was to write code this way, but a lot of people who do write like this believe they can outsmart the compiler by believing "one line means speed". Instead, as in your case, it did nothing except introduce bugs.

    Regards,

    Paul McKenzie
    I had actually never used the ternary operators before this assignment (and I agree with your assessment of how those lines are written). The prof had two requirements for the assignment -- design the program around the given prototypes and demonstrate using ? : syntax. I have no doubt I'll learn more hanging around forums like this than the class.

    That said, changing *position++ to *position += 1 solves the problem. Thanks all, guess I'll have to go over my operator precedence again.

  10. #10
    Join Date
    Apr 1999
    Posts
    27,449

    Re: Completely stumped by stack smash.

    Quote Originally Posted by Comintern View Post
    I had actually never used the ternary operators before this assignment (and I agree with your assessment of how those lines are written). The prof had two requirements for the assignment -- design the program around the given prototypes and demonstrate using ? : syntax.
    The ternary operator used as you used it is highly discouraged. If anything, it should be used in a simple, clear manner -- a simple left hand side of the ?, and simple true/false conditions on the right side of the ?.

    Not only is it hard to decipher what you wrote, the ternary operator used multiple times on one line is a precedence nightmare. Things you think are being done in some order are not being done.

    I don't understand why professors think it's worthwhile to have students write obfuscated code like this -- those code lines would never pass a real code review, as the reviewer themselves would need to look up, down, and sideways at those lines, and probably have to run the code to be convinced it really works.

    Regards,

    Paul McKenzie

  11. #11
    2kaud's Avatar
    2kaud is offline Super Moderator Power Poster
    Join Date
    Dec 2012
    Location
    England
    Posts
    7,822

    Re: Completely stumped by stack smash.

    those code lines would never pass a real code review,
    I Totally agree with Paul regarding this. Many, many years ago when I started out as a programmer my code was often rejected by reviewers because they couldn't easily understand it - not because it didn't work. I was writing obfuscated code like this because this is how I had been taught at University - indeed I had exam questions about rewriting code to be the shortest possible! Once I jetisoned this way of writing and started writing working code that was simple to read then I had no further problems with reviewers. If you ever go for a programming job interview and they ask you to write some code, producing code like this will not endear you to the interviewers! Your professor should read what Donald Knuth (a world renowed computer scientist) has to say about program readability.
    Knuth says. “Algorithms are not made to be packaged
    into black boxes, but kept readable so that anyone using the code can understand how it is meant to work.
    http://eandt.theiet.org

    A further thought on your code. If the function parameters had been better defined then the error with *position++ would have been picked up by the compiler. As you are passing parameters by value and only changing the data pointed to and not the value itself then a better function definition would be

    Code:
    void moveTortoise(int *const position)
    This says that position is a const and cannot be changed (position is a const pointer to an int). So any attempt in the function code to change position (especially accidential!) would be caught by the complier and reported as an error. Bugs caught by the compiler are much easier to fix than having to debug code later.

    With Microsoft VS I get: error C2166: l-value specifies const object if I use const in the function definitions using *position++ but no error using *position +=1

  12. #12
    Join Date
    Jul 2005
    Location
    Netherlands
    Posts
    2,042

    Re: Completely stumped by stack smash.

    Quote Originally Posted by 2kaud View Post
    A further thought on your code. If the function parameters had been better defined then the error with *position++ would have been picked up by the compiler. As you are passing parameters by value and only changing the data pointed to and not the value itself then a better function definition would be

    Code:
    void moveTortoise(int *const position)
    Since the pointer is assumed to be non-NULL in the function anyway, it would be even better to use a reference.
    Code:
    void moveTortoise(int& position)
    Cheers, D Drmmr

    Please put [code][/code] tags around your code to preserve indentation and make it more readable.

    As long as man ascribes to himself what is merely a posibility, he will not work for the attainment of it. - P. D. Ouspensky

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured