Matrix multiplication dynamically allocated matrix error: segmentation fault (core dump)

Question

I am writing a matrix multiplication program, and I need to check the running time of different matrices with different sizes. Then I need to parallelize my code with pthreads in main, therefore I created one header file, which includes the initialization and multiplication functions, and then my main in a separate .c file, but when I execute my program, I get the following error:

error: segmentation fault (core dump)

Please note that I'm trying a matrix multiplication of a square matrix with size 1000.

Here are my codes:

// mm.h

#ifndef MM_H
#define MM_H

#include <stdlib.h>
#include <time.h>

int SIZE, NTHREADS;
int **A, **B, **C;

void init(int **A, int **B, int **C)
{
    int i, j;

    A = (int**)malloc(SIZE * sizeof(int *));
    for(i = 0; i < SIZE; i++)
        A[i] = malloc(SIZE * sizeof(int));

    B = (int**)malloc(SIZE * sizeof(int *));
    for(i = 0; i < SIZE; i++)
        B[i] = malloc(SIZE * sizeof(int));

    C = (int**)malloc(SIZE * sizeof(int *));
    for(i = 0; i < SIZE; i++)
        C[i] = malloc(SIZE * sizeof(int));

    srand(time(NULL));

    for(i = 0; i < SIZE; i++) {
        for(j = 0; j < SIZE; j++) {
            A[i][j] = rand()%100;
            B[i][j] = rand()%100;
        }
    }
}

void mm(int **A, int **B, int **C)
{
    int i, j, k;

    for(i = 0; i < SIZE; i++) {
        for(j = 0; j < SIZE; j++) {
            C[i][j] = 0;
            for(k = 0; k < SIZE; k++) {
                C[i][j] += A[i][k] * B[k][j];
            }
        }
    }
}

#endif

Any my main function:

// mmserial.c

#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include "mm.h"

int main(int argc, char* argv[])
{
    int i;

    if(argc != 2)
    {
        printf("Usage: %s <size_of_square_matrix>\n", argv[0]);
        exit(1);
    }

    SIZE = atoi(argv[1]);
    init(A, B, C);

    clock_t begin, end;
    double time_spent;

    begin = clock();

    mm(A, B, C);

    end = clock();

    time_spent = (double)(end - begin) / CLOCKS_PER_SEC;

    printf("Elapsed time: %.2lf seconds.\n", time_spent);

    for(i = 0; i < SIZE; i++)
        free((void *)A[i]);
    free((void *)A);

    for(i = 0; i < SIZE; i++)
        free((void *)B[i]);
    free((void *)B);

    return 0;
}

If someone could help me overcome this problem, I would be glad. And more question, should I also dynamically allocate my final matrix, which will hold the resulting values of the multiplication?

Answer 1

There are multiple issues with your code. Here are solutions to your issues in your program.

First, you need to allocate space for C array. This is already been covered by tacp and hence, I am skipping the same.

The main issue is coming from init(A,B) function. There are couple of issues here. One, A and B are global pointers and hence, this is really not required to be passed as a function argument. However, since your code is designed for the same, you would have to incorporate the following changes

//main function invoking init
init(&A, &B); // Pass references to A and B

The foot print of init needs to be modified as

void init(int ***Aptr, int ***Bptr)
{
     int    **A; // Define a local pointer to keep rest of the code intact
     int    **B; // Define a local pointer to keep rest of the code intact
     .....................
     //Rest of your code

     // End of function
     *Aptr = A;
     *Bptr = B;
}

With these 2 changes, your code should be ready to work fine. In your earlier implementation, only the local copy of A and B were updated in the init function and it wasn't reflected on the actual pointers. Hence, when init completed, A and B were pointing to potentially NULL pointers which was the cause of the segmentation fault.

There is another solution to this problem. You could avoid passing the arguments to init i.e. convert init to init() and modify the implementation as void init(). This would work fine (again assuming C is also allocated)

However, there is still one more point. Lastly, you would require to take care to delete the memory for C array also (assuming it is allocated in init function) as

for(i = 0; i < SIZE; i++)
    free((void *)C[i]);
free((void *)C);

Postscript:

From a programming perspective, I feel you shouldn't mix global variables and passing these very global variables as function parameters. The scope is global which means the function can definitely access the variables and hence, don't require the same to be passed to the function.

Answer 2

I was looking for final code but no updates available, for those who are interested as well in matrix multiplication by dynamic allocation, here the final code:

#include <stdio.h>
#include <stdlib.h>
#include <time.h>
void allocate_mem(double*** arr, int rows, int cols);
void deallocate_mem(double*** arr, int n);
void printMatrix(double** a, int rows, int cols);
void printMatrixE(double** a, int rows, int cols);
void multMatrixp(double **A, double **B, double **C,int r1,int c1,int r2,int c2);
void init(double*** a, int rows,int cols);

int main(int argc, char* argv[])
{

   int ro1, co1, ro2, co2;
   double **a, **b, **c;    



//ro1=10;
//co1=10000;
//ro2=10000;
//co2=33;

ro1=atoi(argv[1]);
co1=atoi(argv[2]);
ro2=atoi(argv[3]);
co2=atoi(argv[4]);


init(&a,ro1,co1);
init(&b,ro2,co2);
allocate_mem(&c,ro1,co2);

clock_t begin, end;
double time_spent;
begin = clock();
multMatrixp(a, b, c, ro1, co1, ro2, co2);  
end = clock();
time_spent = (double)(end - begin) / CLOCKS_PER_SEC;
printf("Elapsed time: %.2lf seconds.\n", time_spent);

printMatrix(a,ro1,co1); 
printMatrix(b,ro2,co2); 
printMatrixE(c,ro1,co2); 


deallocate_mem(&a,ro1);
deallocate_mem(&b,ro2);
deallocate_mem(&c,ro1);

    return 0;
}

//______________________________________________________________________________
void allocate_mem(double*** arr, int rows, int cols)
{
  int i;
  *arr = (double**)malloc(rows*sizeof(double*));
  for( i=0; i<rows; i++)
    (*arr)[i] = (double*)malloc(cols*sizeof(double));
} 

//______________________________________________________________________________
void deallocate_mem(double*** arr, int rows){
 int i;
    for (i = 0; i < rows; i++)
        free((*arr)[i]);
    free(*arr); 
}

//______________________________________________________________________________

void init(double*** a, int rows,int cols)
{
   int i, j;

*a= (double**) malloc(rows*sizeof(double*));
for(i=0;i<rows;i++)
(*a)[i]=(double*)malloc(cols*sizeof(double));
    for(i=0;i<rows;i++)
        for(j=0;j<cols;j++)
        (*a)[i][j] = rand()%1000;
}


//______________________________________________________________________________
void printMatrix(double** a, int rows, int cols)
{
    int i, j;
   printf("Matrix[%d][%d]\n",rows,cols);    
   for(i=0;i<rows;i++){
      for(j=0;j<cols;j++)
      printf("%7.1lf ",a[i][j]);
      printf("\n");
   }
   printf("\n");   
}

//______________________________________________________________________________
void printMatrixE(double** a, int rows, int cols)
{
    int i, j;
   printf("Matrix[%d][%d]\n",rows,cols);    
   for(i=0;i<rows;i++){
      for(j=0;j<cols;j++)
      printf("%9.2e ",a[i][j]);
      printf("\n");
   } 
   printf("\n");     
}


//______________________________________________________________________________

void multMatrixp(double **A, double **B, double **C,int ro1,int co1,int ro2,int co2)
{
    int i, j, k;
    for(i = 0; i < ro1; i++) {
        for(j = 0; j < co2; j++) {
            C[i][j] = 0;
            for(k = 0; k < co1; k++) {
                C[i][j] += A[i][k] * B[k][j];
            }
        }
    }
}

Answer 3

C is also a dynamic matrix, but you never allocated memory for it, directly assign values to nonexist memory, so that's why you have core dump. You should also allocate memory for C.

 void mm(int **A, int **B, int **C)
 {
   int i, j, k;

   for(i = 0; i < SIZE; i++) {
      for(j = 0; j < SIZE; j++) {
        C[i][j] = 0;   ///^^^Need to first allocate space for C
        for(k = 0; k < SIZE; k++) {
            C[i][j] += A[i][k] * B[k][j];
        }
      }
   }
}