I created a program that is supposed to read two 3D matrices from a binary file, multiply them, and print the result to a binary file. However, while it successfully compiles, when I run it, it gives me a segmentation fault error.
Here is the code I used for the program:
#include <stdio.h>
#include <stdlib.h>
int main() {
int n;
int n2;
int matrix1[n][n][n], matrix2[n][n][n], result_matrix[n][n][n];
FILE *file1, *file2, *result;
file1 = fopen("matrix1.bin", "rb");
file2 = fopen("matrix2.bin", "rb");
fread(&n, sizeof(int), 1, file1);
fread(&n2, sizeof(int), 1, file2);
if (n != n2 || n > 100) {
printf("Error: Incompatible matrices or n greater than 100.");
return 1;
}
fread(matrix1, sizeof(int), n * n * n, file1);
fread(matrix2, sizeof(int), n * n * n, file2);
fclose(file1);
fclose(file2);
for (int i = 0; i < n; ++i) {
for (int j = 0; j < n; ++j) {
for (int k = 0; k < n; ++k) {
result_matrix[i][j][k] = 0;
for (int l = 0; l < n; l++) {
result_matrix[i][j][k] += matrix1[i][j][l] * matrix2[l][j][k];
}
}
}
}
result = fopen("result.bin", "wb");
fwrite(&n, sizeof(int), 1, result);
fwrite(result_matrix, sizeof(int), n * n * n, result);
fclose(result);
return 0;
}
This is specifically where the segmentation fault is happening:
Program received signal SIGSEGV, Segmentation fault.
0x0000555555555c3a in main () at 31504085_1.c:35
35 result_matrix[i][j][k] += matrix1[i][j][l] * matrix2[l][j][k];
What can you do to help fix this issue?
As the comments pointed out, you should really turn up your level of warnings in your compiler flags. This is what you can get if you do so. The warning messages clearly points out the issue,
<source>: In function 'main':
<source>:8:5: warning: 'n' is used uninitialized [-Wuninitialized]
8 | int matrix1[n][n][n], matrix2[n][n][n], result_matrix[n][n][n];
| ^~~
<source>:5:9: note: 'n' declared here
5 | int n;
| ^
Using an uninitialized variable produces an indeterminate value, which is not undefined behavior per se, but using an indeterminate value to declare an array (specifically, a VLA, more on that topic later) can lead to undefined behavior because the precondition that the size expression must evaluate to to a value greater than zero may be violated. When it does lead to undefined behavior, anything can happen, and a segfault is one of the many possible outcomes. Undefined behavior or not, using an indeterminate value to declare the array is semantically wrong and definitely what you want to achieve.
So the first attempt to fix would be:
int n;
/* ... */
fread(&n, sizeof(int), 1, file1);
/* ... */
int matrix1[n][n][n], matrix2[n][n][n], result_matrix[n][n][n];
When you declare an array like matrix1[n][n][n], where the extents for the dimensions are not integer constant expressions, you are using a feature in the C language that is called the variable-length arrays(VLAs for short). This feature is not required to be supported by a standard-conforming C compiler and by definition your program is not very portable. With that in mind and assume that you know for sure that your compiler does support VLAs, and since C23, this also implies the support for variably-modified types (VMs), let's further examine other issues of your code.
Assuming that your implementation does use a stack, it is up to the implmentation whether to put the VLA on the stack. On typical implementations, the stack size is limited, and if you try to use a VLA that takes up too much space the stack would blow up and result in a segfault again. Since we have already assumed that your implementation supports VMs, you should instead use a malloc to allocate the memory for the VLA in the free store. By taking advantage of the support for VM, you can do so without losing the convenience of the multidimensional array access syntax. To make things easier, we can use a typedef. I have also modified the code to check for the return value of fread to guard against an empty file. You should also always check the return value of other standard library functions in your program if they are available.
int n;
/* ... */
if(fread(&n, sizeof(int), 1, file1) != 1){
fputs("Failed to read matrix dimension.\n", stderr);
return EXIT_FAILURE;
}
/* ... */
typedef int (*matrix_t)[n][n][n]; //A variably-modified type
matrix_t matrix1 = malloc(sizeof *matrix1);
if(!matrix1){
fputs("Malloc failed: matrix1.\n", stderr);
return EXIT_FAILURE;
}
matrix_t matrix2 = malloc(sizeof *matrix2);
/* ... */
Then, inside the loops, you can dereference the pointer to perform the actual computation,
for (int i = 0; i < n; ++i) {
for (int j = 0; j < n; ++j) {
for (int k = 0; k < n; ++k) {
(*result_matrix)[i][j][k] = 0;
for (int l = 0; l < n; l++) {
(*result_matrix)[i][j][k] += (*matrix1)[i][j][l] * (*matrix2)[l][j][k];
}
}
}
}
At the end of the function, you should remember to free the allocated memories,
free(matrix1);
free(matrix2);
free(result_matrix);
return EXIT_SUCCESS;