Why Valgrind complains about fgets?

Question

I have a simple C code.

main.c

int main() {
    FILE *stream = fopen("../input.txt", "r");
    cube test_cube;
    cube_initialization(&test_cube, stream);

/* and some code with free memory which is not needed to understand the situation */

}

cube.c

/* just the function valgrind complains about */

void cube_initialization(cube *cube, FILE *input_file) {
    int side_length;
    char buffer[BUF_SIZE];
    char waste_buffer[BUF_SIZE];


    fgets(buffer, BUF_SIZE, input_file);
    fgets(waste_buffer, BUF_SIZE, input_file); /* empty line */
    side_length = (int) strtol(buffer, NULL, 10);
    cube->side_length = side_length;
    cube->cube_array = malloc(side_length * sizeof(int **));
    int z;
    for (z = 0; z < side_length; z++) {
        cube->cube_array[z] = malloc(side_length * sizeof(int *));
        int y;
        for (y = 0; y < side_length; y++) {
            cube->cube_array[z][y] = malloc(side_length * sizeof(int));
        }
    }
}

and valgrind output

==8251== Invalid read of size 4
==8251==    at 0x48F3727: fgets (iofgets.c:47)
==8251==    by 0x1093C2: cube_initialization (cube.c:11)
==8251==    by 0x10928D: main (main.c:11)
==8251==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==8251== 
==8251== 
==8251== Process terminating with default action of signal 11 (SIGSEGV)
==8251==  Access not within mapped region at address 0x0
==8251==    at 0x48F3727: fgets (iofgets.c:47)
==8251==    by 0x1093C2: cube_initialization (cube.c:11)
==8251==    by 0x10928D: main (main.c:11)
==8251==  If you believe this happened as a result of a stack
==8251==  overflow in your program's main thread (unlikely but
==8251==  possible), you can try to increase the size of the
==8251==  main thread stack using the --main-stacksize= flag.
==8251==  The main thread stack size used in this run was 8388608.
==8251== 
==8251== HEAP SUMMARY:
==8251==     in use at exit: 0 bytes in 0 blocks
==8251==   total heap usage: 1 allocs, 1 frees, 488 bytes allocated
==8251== 
==8251== All heap blocks were freed -- no leaks are possible
==8251== 
==8251== For lists of detected and suppressed errors, rerun with: -s
==8251== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Segmentation fault (core dumped)

I don't understand why Valgrind complains about fgets(). I read data from file and use fgets with pointer to big buffer(256) but it needs me just for read short lines about 1-6 symbols (I need fgets because it stops when it finds ' ' at the end of line). Maybe the problem is in the fact fgets() tries to read line of 256 symbols and it stops after 1-5 and ' '?

user_11336762 · Answer

Valgrind complains about illegal address 0x0 access:

Address 0x0 is not stack'd, malloc'd or (recently) free'd

In your fgets() call

fgets(buffer, BUF_SIZE, input_file);

buffer parameter is fine because it is allocated in the stack. The only responsible can be input_file parameter, coming from main(). That parameter is a stream obtained from fopen() call. You didn't check it before passing to cube_initialization()!

Check it, and then understand why it returns NULL (probably you try to open a path that doesn't exist).

int main() {
    FILE *stream = fopen("../input.txt", "r");
    cube test_cube;
    if( stream != NULL )
    {
        cube_initialization(&test_cube, stream);

        /* Continue execution*/
    }
}

user_4593267 · Answer

Valgrind complains about an invalid read in fgets(). The problem unlikely to be the size of the destination array, which could trigger an invalid write, but is indeed an array of BUF_SIZE bytes, which you document to be a reasonable value of 256. The problem is more likely with the stream pointer, which may be null as reported by Valgrind:

Address 0x0 is not stack'd, malloc'd or (recently) free'd`

You should fix potential undefined behavior in your code:

verify if fopen() returned a valid FILE*, to avoid undefined behavior when you try to read from input_file.
verify that fgets() succeeds (does not return NULL), to avoid undefined behavior when you read from the destination array.

Why Valgrind complains about fgets?

2 Answers