0

I've tried replicating this bug I'm having in a project, but all efforts have failed, with my replications compiling and testing perfectly.

Basically, I keep getting a seg fault that traces back to a call to strcpy in one of my files. I've managed to isolate it somewhat, and am inclined to believe that the issue is due to a call to memcpy earlier, as without that line, the code works fine.

void play(char * p1, char * p2, 
    struct result * final_res)
{
    enum icons my_board[WIDTH][HEIGHT];
    initialize(my_board);  /* potentially causing the seg fault */

    strcpy(final_res -> won, "Hello"); /* the seg fault */
    strcpy(final_res -> lost, "Goodbye"); 

    /* rest of my code, commented out */
}

Where in result is a struct:

struct result
{
    char won[21];
    char lost[21];
};

The function in question, which copies the state of a global enum, master:

void intialize(enum icons board[][WIDTH])
{
    int i, j;

    for(i=0; i < WIDTH; i++) {
        for(j=0; j < WIDTH; j++) {
            memcpy(&board[i][j], &master[i][j], WIDTH-1);
        }
    }

}

The master global variable:

static const enum icons master[WIDTH][HEIGHT] = 
{
    {CIRCLE, CIRCLE, CIRCLE, CIRCLE, CIRCLE}, 
    {CIRCLE, CIRCLE, CIRCLE, CIRCLE, CIRCLE}, 
    {SQUARE, SQUARE, SQUARE, SQUARE, SQUARE}, 
    {TRIANGLE, TRIANGLE, TRIANGLE, TRIANGLE, TRIANGLE}, 
    {TRIANGLE, TRIANGLE, TRIANGLE, TRIANGLE, TRIANGLE}, 
};

My valgrind output:

==11706== Invalid write of size 1
==11706==    at 0x343040: _platform_memmove$VARIANT$Nehalem (in /usr/lib/system/libsystem_platform.dylib)
==11706==    by 0x1547E2: stpcpy (in /usr/lib/system/libsystem_c.dylib)
==11706==    by 0x1C2A43: __strcpy_chk (in /usr/lib/system/libsystem_c.dylib)
==11706==    by 0x1000013CD: play (game.c:21)
==11706==    by 0x1000012EF: main (starter.c:63)
==11706==  Address 0x104000001 is not stack'd, malloc'd or (recently) free'd
==11706==
==11706==
==11706== Process terminating with default action of signal 11 (SIGSEGV)
==11706==  Access not within mapped region at address 0x104000001
==11706==    at 0x343040: _platform_memmove$VARIANT$Nehalem (in /usr/lib/system/libsystem_platform.dylib)
==11706==    by 0x1547E2: stpcpy (in /usr/lib/system/libsystem_c.dylib)
==11706==    by 0x1C2A43: __strcpy_chk (in /usr/lib/system/libsystem_c.dylib)
==11706==    by 0x1000013CD: play (game.c:21)
==11706==    by 0x1000012EF: main (starter.c:63)
==11706==  If you believe this happened as a result of a stack
==11706==  overflow in your program's main thread (unlikely but
==11706==  possible), you can try to increase the size of the
==11706==  main thread stack using the --main-stacksize= flag.
==11706==  The main thread stack size used in this run was 8388608.
==11706==
==11706== HEAP SUMMARY:
==11706==     in use at exit: 33,497 bytes in 377 blocks
==11706==   total heap usage: 455 allocs, 78 frees, 39,513 bytes allocated
==11706==
==11706== LEAK SUMMARY:
==11706==    definitely lost: 0 bytes in 0 blocks
==11706==    indirectly lost: 0 bytes in 0 blocks
==11706==      possibly lost: 0 bytes in 0 blocks
==11706==    still reachable: 8,192 bytes in 2 blocks
==11706==         suppressed: 25,305 bytes in 375 blocks
==11706== Rerun with --leak-check=full to see details of leaked memory
==11706==
==11706== For counts of detected and suppressed errors, rerun with: -v
==11706== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 133 from 34)
Segmentation fault: 11

Apologies for not providing a compilable example, or leaving out any pertinent info, please comment if there's anything else I need to provide to make anything clearer!

Any idea what's going on here?

Thank you!

Unihedron
  • 10,902
  • 13
  • 62
  • 72
  • Did you ensure that the memory for the "struct result" was allocated. I.e. to "final_res"? – SomethingSomething Aug 25 '14 at 15:41
  • I just declared it at the top of the `starter.c` file as `struct result current_game;` (passed into the `final_res` arg), although I did play around with using malloc, although I couldn't get it to work without declaring `current_game` as a pointer –  Aug 25 '14 at 15:46
  • How exactly did you pass it? You should pass its reference, i.e. `play(p1, p2, &your_local_struct);` – SomethingSomething Aug 25 '14 at 15:48
  • Yep, that's how I did it. Ended up not using malloc for the reasons I said above, so it's just declared at the top of the file (I didn't assign anything to it) and passed to the `play()` func. Is this where the problem might be? –  Aug 25 '14 at 15:51
  • 1
    `for(i=0; i < WIDTH; i++)`?? Should not `for(i=0; i < HEIGHT; i++)`? – chux - Reinstate Monica Aug 25 '14 at 16:06
  • `void intialize(enum icons board[][WIDTH])` --> `void intialize(enum icons board[][HEIGHT])` – BLUEPIXY Aug 25 '14 at 18:01
  • `memcpy(&board[i][j], &master[i][j], WIDTH-1);` : `WIDTH-1` --> `sizeof(enum icons)` – BLUEPIXY Aug 25 '14 at 18:07

3 Answers3

0

I think the problem is in yout initialization routine: You iterate through all the fields (rows and columns) and copy a complete row instead of a single element. When you get to the upper limits of the indice you copy beyond the limits of your array.

chrmue
  • 1,552
  • 2
  • 18
  • 35
0

Your memcpy in initialise is the bug. The third argument should be sizeof(master) not width-1. This assumes the size of elements in array 1 are the same size as those in master (array 2)

phil
  • 561
  • 3
  • 10
0

This block of code is not right:

void intialize(enum icons board[][WIDTH])
{
    int i, j;

    for(i=0; i < WIDTH; i++) {
        for(j=0; j < WIDTH; j++) {
            memcpy(&board[i][j], &master[i][j], WIDTH-1);
        }
    }
}

Type of board[i][j] is enum icons. When using memcpy, you need to use sizeof(enum icons) not WIDTH-1.

You can solve this problem two ways.

  1. Assign the values from master to board one element at a time.

    void intialize(enum icons board[][WIDTH])
    {
       int i, j;
    
       for(i=0; i < WIDTH; i++) {
          for(j=0; j < WIDTH; j++) {
             board[i][j] = master[i][j];
          }
       }
    }
    

    or

  2. Use memcpy to copy rows of data from master to board.

    void intialize(enum icons board[][WIDTH])
    {
       int i;
    
       for(i=0; i < WIDTH; i++) {
          // Copy WIDTH enum icons from master to board.
          memcpy(board[i], master[i], sizeof(enum icons)*WIDTH);
       }
    }
    
R Sahu
  • 204,454
  • 14
  • 159
  • 270