1

Hello I seem to be having an issue with freeing some of my variables after creating them.

Little background on my program. My program is reading MatrixMarket files and building a Matrix Structure from the information read in. I am then using the matrices to do scalar and vector multiplication.

This is my structure for my matrix.

typedef struct {
    double *values;       // Value of each non-zero element.
    unsigned int *col_indices; // Corresponding column coordinate.
    unsigned int *row_ptr; // Initial position of each row.
    unsigned int nz;  // Total non-zero elements.
    unsigned int row_count; // Total row_count of matrix.
    unsigned int cols_count; // Total columns of matrix.
} CSRMatrix;

My goal here is to be able to manipulate my matrix and be able to multiply a vector to the CSR Matrix. (CSR is Compressed Sparse Row Matrix).

Here is how I am initializing my memory allocation.

    /**
    * Set the correct values to the struct and create the memory allocation.
    */
    csrMatrix.row_count = numberOfRows;
    csrMatrix.cols_count = numberOfColumn;
    csrMatrix.nz = numberOfNonZeros;
    csrMatrix.row_ptr = calloc(numberOfRows, 1);
    csrMatrix.values = (double *)malloc(numberOfRows * numberOfRows * 1);
    csrMatrix.col_indices = calloc(numberOfRows * numberOfRows, 1);

Here is where I think my issue is, I may be wrong. I'm terrible at troubleshooting memory issues.

Before I go more in-depth here is where the main multiplication and iteration to the matrix happens:

// Initialize vectors for multiplication

int x_size = numberOfRows * numberOfColumn;
int csr_x[x_size];
memset(csr_x, 0, sizeof(csr_x));
for(int h = 0; h < x_size; h++){
    csr_x[h] = 1;
}
double* csr_y;

printf("\nCalculating matrix-vector product of the CSR for %u iterations...\n", iterations);
clock_t begin = clock();
for(i=0; i < iterations; i++){
    csr_y = (double *)malloc(csrMatrix.row_count * csrMatrix.cols_count * 1);
    csrMVP(&csrMatrix, csr_x,csr_y);
    free(csr_y);
}
clock_t end = clock();
double time_spent = (double)(end - begin) / CLOCKS_PER_SEC;
printf("for loop used %f seconds to complete the execution\n", time_spent);
fclose(file);
free(csr_x);
free(csr_y);
exit(EXIT_SUCCESS);

When I run my program strange things being to happen. The program will run for a few loops (total loops is 1000 iterations) then either exit incomplete with Process finished with exit code 0 or with exit code 11. This is where my troubleshooting started.

Since intellij is not throwing any memory issues or errors I have to go deeper.

I decided to ask a friend of mine who has used Valgrind to do a memcheck on my program. This was the result:

==5607== Memcheck, a memory error detector
==5607== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==5607== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==5607== Command: ./code
/home/tron/src/workspace/classes/cda5110/matrix/mat/ibm32/ibm32.mtx
==5607==
==5607== Invalid write of size 4
==5607==    at 0x109537: loadCSRMatrix (mainCSR.c:173)
==5607==    by 0x108F3A: main (mainCSR.c:72)
==5607==  Address 0x51f23b0 is 0 bytes after a block of size 128 alloc'd
==5607==    at 0x4C2EEF5: calloc (vg_replace_malloc.c:711)
==5607==    by 0x108EC5: main (mainCSR.c:64)
==5607==
==5607== Invalid read of size 4
==5607==    at 0x109300: csrMVP (mainCSR.c:140)
==5607==    by 0x109085: main (mainCSR.c:92)
==5607==  Address 0x51f23b0 is 0 bytes after a block of size 128 alloc'd
==5607==    at 0x4C2EEF5: calloc (vg_replace_malloc.c:711)
==5607==    by 0x108EC5: main (mainCSR.c:64)
==5607==
==5607== Invalid free() / delete / delete[] / realloc()
==5607==    at 0x4C2E10B: free (vg_replace_malloc.c:530)
==5607==    by 0x10910C: main (mainCSR.c:99)
==5607==  Address 0x1ffefff5a0 is on thread 1's stack
==5607==  in frame #1, created by main (mainCSR.c:24)
==5607==
==5607== Invalid free() / delete / delete[] / realloc()
==5607==    at 0x4C2E10B: free (vg_replace_malloc.c:530)
==5607==    by 0x10911B: main (mainCSR.c:100)
==5607==  Address 0x59d3bc0 is 0 bytes inside a block of size 8,192 free'd
==5607==    at 0x4C2E10B: free (vg_replace_malloc.c:530)
==5607==    by 0x109094: main (mainCSR.c:93)
==5607==  Block was alloc'd at
==5607==    at 0x4C2CEDF: malloc (vg_replace_malloc.c:299)
==5607==    by 0x109064: main (mainCSR.c:91)
==5607==
==5607==
==5607== HEAP SUMMARY:
==5607==     in use at exit: 12,416 bytes in 3 blocks
==5607==   total heap usage: 1,006 allocs, 1,005 frees, 8,213,160 bytes
allocated
==5607==
==5607== LEAK SUMMARY:
==5607==    definitely lost: 0 bytes in 0 blocks
==5607==    indirectly lost: 0 bytes in 0 blocks
==5607==      possibly lost: 0 bytes in 0 blocks
==5607==    still reachable: 12,416 bytes in 3 blocks
==5607==         suppressed: 0 bytes in 0 blocks
==5607== Rerun with --leak-check=full to see details of leaked memory
==5607==
==5607== For counts of detected and suppressed errors, rerun with: -v
==5607== ERROR SUMMARY: 1003 errors from 4 contexts (suppressed: 0 from 0)

There are few things that popped out at me. There are a few invaild write and read sizes. These are the following lines for each code called out by the errors:

  • at 0x109537: loadCSRMatrix (mainCSR.c:173): m->row_ptr[row_index + 1] = pairIndex;
  • by 0x108F3A: main (mainCSR.c:72): loadCSRMatrix(&csrMatrix, file);
  • by 0x108EC5: main (mainCSR.c:64): csrMatrix.row_ptr = calloc(numberOfRows, 1);
  • at 0x109300: csrMVP (mainCSR.c:140): y[i] += val[j] * x[col[j]];
  • by 0x109085: main (mainCSR.c:92): free(csr_y);
  • by 0x108EC5: main (mainCSR.c:64): csrMatrix.row_ptr = calloc(numberOfRows, 1);
  • by 0x10910C: main (mainCSR.c:99): free(csr_y);
  • by 0x10911B: main (mainCSR.c:100): exit(EXIT_SUCCESS);
  • by 0x109094: main (mainCSR.c:93): }
  • by 0x109064: main (mainCSR.c:91): csrMVP(&csrMatrix, csr_x,csr_y);

    From what I bring it down to is that I am not correctly utilizing malloc and calloc for my memory allocation.

So here's where I ask my question, I am not sure if malloc or calloc are used correctly. If anyone knows where I can start looking to fix this issue.

In case you're wondering what loadCSRMatrix and csrMVP functions are I will attach them below.

csrMVP

/**
 * Stores on vector y the product of matrix M and vector x:
 *   y = Mx
 */
void csrMVP(CSRMatrix *m, int x[], double *y){
    printf("\n\n\nValue of Y: \n ");

    unsigned int i,j,end;
    double *val = m->values;
    unsigned int *col = m->col_indices;
    unsigned int *ptr = m->row_ptr;
    end = 0;
    for(i = 0; i < m->row_count; i++){
        y[i] = 0.0;
        j = end;
        end = ptr[i+1];
        for( ; j < end; j++){
            y[i] += val[j] * x[col[j]];
            printf("%.f\t",  y[i]);
        }
    }printf("\n");

}

loadCSRMatrix

/**
 * Loads the matrix
 */
void loadCSRMatrix(CSRMatrix *m, FILE *f){
    unsigned int pairIndex = 0;
    for (int row_index = 0; row_index < m->row_count; row_index++) {
        for (unsigned int column_index = 0; column_index < m->row_count; column_index++) {
            double value;
            if(fscanf(f, "%lg", &value) != EOF){
                m->values[pairIndex] = value;
                m->col_indices[pairIndex] = column_index+1;
                //printf("Column: %d\t", matrixCSR.col_indices[pairIndex]);
                printf("%.f\t", m->values[pairIndex]);
                pairIndex++;
            }
            else {
                m->values[pairIndex] = 0;
                m->col_indices[pairIndex] = column_index+1;
                //printf("Column: %d\t", matrixCSR.col_indices[pairIndex]);
                printf("%.1f\t", m->values[pairIndex]);
                pairIndex++;
            }
        }
        if(pairIndex != 0) {
            m->row_ptr[row_index + 1] = pairIndex;
        }
        //printf("%zu\n", matrixCSR.row_indices[row_index]);
        printf("\n");
    }
}
JeanP
  • 406
  • 9
  • 27
  • 2
    Are you using C or C++? This looks like C code – NathanOliver Mar 29 '18 at 21:02
  • Added wrong tag. Typed too fast. – JeanP Mar 29 '18 at 21:03
  • 6
    `srMatrix.values = (double *)malloc(numberOfRows * numberOfRows * 1);` is not correct. You need a sizeof(double) on that instead of * 1. – Michael Dorgan Mar 29 '18 at 21:04
  • 3
    `int csr_x[x_size]; ... free(csr_x);` You're `free`ing memory you didn't `malloc`. You're also double-freeing `csr_y` on the next line. – Miles Budnek Mar 29 '18 at 21:06
  • `srMatrix.values = (double *)malloc(numberOfRows * numberOfRows * sizeof(double));` ? Im not sure what you meant by instead of * 1 – JeanP Mar 29 '18 at 21:07
  • 1
    There is no need to cast the return of `malloc`, it is unnecessary. See: [Do I cast the result of malloc?](http://stackoverflow.com/q/605845/995714). Also, keep in mind it isn't recommended to mix `new/delete` and `malloc/free` (you don't, but do keep that in mind) – David C. Rankin Mar 29 '18 at 21:08
  • 1
    Because `malloc` must be told the the size of each element. It does not know about types. – Weather Vane Mar 29 '18 at 21:08
  • 1
    And also understand if `unsigned int *row_ptr;` will just be a pointer pointing to the current row -- there is no need to allocate storage for it. A pointer can always hold an address, you just allocate storage if you need to hold actual values which you will reference with the pointer. If the values are already stored elsewhere, you can simply use the pointer to point to the existing storage. – David C. Rankin Mar 29 '18 at 21:10
  • Trying to answer every comment. @MilesBudnek I updated the malloc statement. DavidCRankin I removed the cast and *row_ptr is holding the initial position of every row. More on this here: https://people.sc.fsu.edu/~jburkardt/data/cr/cr.html – JeanP Mar 29 '18 at 21:11
  • 1
    Adding the `sizeof(double)` and removed the `double free(y)` allowed me to get closer to the correct result but I am still not running to completions – JeanP Mar 29 '18 at 21:17
  • @JeanP: did you remove `free(csr_x);`? Since `csr_x` is not the result of a `malloc` call, you can't `free` it. – John Bode Mar 29 '18 at 23:45
  • @JohnBode Yes I did and the program seems to be almost there. I just need to investigate some more. – JeanP Mar 29 '18 at 23:58
  • 1
    @JeanP - if your run into problems finalizing your code, drop another comment. I went though the process at the link you provided and have sorted out the fun. To output the sparse matrix, using pointers to both `col_indices` and `values` simplifies the process a great deal. – David C. Rankin Mar 30 '18 at 01:29
  • 1
    `csrMatrix.values = (double *)malloc(numberOfRows * numberOfRows * 1);` is suspicious. I'd expect `csrMatrix.values = malloc(sizeof *csrMatrix.value * numberOfRows * numberOfColumns);` --> columns * rows inside of rows * rows – chux - Reinstate Monica Mar 30 '18 at 02:43

0 Answers0