2

I am trying to write a simple game in C and I'm getting a SEGFAULT and have no idea why!

Here is the code for the program:

#include <stdio.h>
#include <string.h>

#define MAX_PLYS_PER_GAME (1024)
#define MAX_LEN (100)

typedef struct {
   char positionHistory[MAX_PLYS_PER_GAME][MAX_LEN];
} Game;

void getInitialGame(Game * game) {
    memset(game->positionHistory, 0, MAX_PLYS_PER_GAME*MAX_LEN*sizeof(char));
}

void printGame(Game game) {
    printf("Game -> %p (%d)\n", &game, sizeof(game));
    fflush(stdout);
}

int hasGameEnded(Game game) {
    printGame(game);
    return 0;
}

int main(int argc, char *argv[]) {
    Game game;
    getInitialGame(&game);

    if (hasGameEnded(game))
        return -1;

    return 0;
}

I tried debugging with gdb but the results didn't get me too far:

C:\Users\test>gdb test.exe
GNU gdb 5.1.1 (mingw experimental)
<snip>
This GDB was configured as "mingw32"...
(gdb) run
Starting program: C:\Users\test/test.exe

Program received signal SIGSEGV, Segmentation fault.
0x00401368 in main (argc=1, argv=0x341c88) at fast-chess-bug.c:29
29              if (hasGameEnded(game))
(gdb) bt
#0  0x00401368 in main (argc=1, argv=0x341c88) at fast-chess-bug.c:29
cxw
  • 16,685
  • 2
  • 45
  • 81
  • I can't reproduce it. for efficient function calls, printGame and hasGameEnded should take a pointer to a Game (like getInitialGame). – Bonan Oct 17 '16 at 19:42
  • You should change the print specifier in the `printGame()` function from `%d` to `%lu`. – ad absurdum Oct 17 '16 at 19:42
  • Welcome! Thank you for including details in your question --- not everyone does. Check out the [tour](https://stackoverflow.com/tour) for more about how the community works, and enjoy your stay! – cxw Oct 17 '16 at 19:47
  • Thanks for the feedback, guys! – Frederico Jordan Oct 17 '16 at 19:54
  • Well, your program compiles and runs without any problem (just a warning about using `%d` format instead of `%ld` for the `sizeof(game)`. Apart of the fact of passing the complete structure by value to `hasGameEnded()` and to `printGame()` functions, I don't see anything irregular in your code (you are copying a 100Kb structure on the stack to pass it to either function) – Luis Colorado Oct 19 '16 at 06:55

2 Answers2

5

It is probably a stack overflow (really!), although I'm not sure.

  1. You are declaring Game game; in main(). That means all 102400 bytes of game are going on the stack.
  2. Both printGame and hasGameEnded take a Game game, NOT a Game * game. That is, they are getting a copy of the Game, not a pointer to the existing Game. Therefore, you dump another 102400 bytes on the stack whenever you call either one.

I am guessing that the call to printGame is clobbering the stack in a way that causes problems with the hasGameEnded call.

The easiest fix I know of (without getting into dynamic memory allocation, which may be better long-term) is:

  1. Move Game game; outside of main(), e.g., to the line just above int main(...). That way it will be in the data segment and not on the stack.
  2. Change printGame and hasGameEnded to take Game *:

    void printGame(Game * game) {
        printf("Game -> %p (%d)\n", game, sizeof(Game));
        fflush(stdout);
    }
    
    int hasGameEnded(Game * game) {
        printGame(game);
        return 0;
    }
    

That should get you moving forward.

cxw
  • 16,685
  • 2
  • 45
  • 81
2

You're likely running out of stack space.

C is pass-by-value. So this code

int hasGameEnded(Game game)

creates a copy of the entire struct {} Game, most likely on the stack.

If the following code works, you ran out of stack space:

...

void printGame(Game *game) {
    printf("Game -> %p (%zu)\n", game, sizeof(*game));
    fflush(stdout);
}

int hasGameEnded(Game *game) {
    printGame(game);
    return 0;
}

int main(int argc, char *argv[]) {
    Game game;
    getInitialGame(&game);

    if (hasGameEnded(&game))
        return -1;

    return 0;
}

Note carefully the changes. Instead of passing the entire structure to hasGameEnded, it's now passing just the address of the structure. That change flows down the call stack, culminating in changes to printGame().

Note also that the proper format specifier for sizeof includes a z modifier. And I took the liberty of making it u for unsigned since a size can't be negative.

Community
  • 1
  • 1
Andrew Henle
  • 32,625
  • 3
  • 24
  • 56