Register to reply

Memory Leak with C

by tangodirt
Tags: leak, memory
Share this thread:
tangodirt
#1
Sep23-11, 09:01 AM
P: 47
I'm trying to plug up a memory leak in a simple C program that multiplies two matrices together. I've profiled my code with valgrind, and it reports leaked memory from within this function:

int ** AllocateMatrix(int size) {
    int i = 0;

    int *contiguousMemory = (int*) malloc(sizeof(int)*size*size);
    int **matrix = (int**) malloc(sizeof(int*)*size);

    for (i = 0; i < size; i++) {
        matrix[i] = (int*) contiguousMemory+(i*size);
    }

    return matrix;
}
I believe the issue is that the *contiguousMemory variable is malloced, but never freed. So, I tried adding a line before the return matrix; line that calls: free(contiguousMemory), which closes the memory leak, but instead returns:

==14898== Invalid read of size 4
==14898==    at 0x40102A: main (MatrixMultiply.c:252)
==14898==  Address 0x4cae140 is 0 bytes inside a block of size 262,144 free'd
==14898==    at 0x4A05A31: free (vg_replace_malloc.c:325)
==14898==    by 0x400DD4: main (MatrixMultiply.c:179)
==14898==
==14898==
==14898== More than 10000000 total errors detected.  I'm not reporting any more.
==14898== Final error counts will be inaccurate.  Go fix your program!
==14898== Rerun with --error-limit=no to disable this cutoff.  Note
==14898== that errors may occur in your program without prior warning from
==14898== Valgrind, because errors are no longer being displayed.
 
 
==17670== HEAP SUMMARY:
==17670==     in use at exit: 0 bytes in 0 blocks
==17670==   total heap usage: 6 allocs, 6 frees, 792,576 bytes allocated
==17670==
==17670== All heap blocks were freed -- no leaks are possible
==17670==
==17670== For counts of detected and suppressed errors, rerun with: -v
==17670== ERROR SUMMARY: 10000000 errors from 6 contexts (suppressed: 4 from 4)
Any ideas for fixing this?
Phys.Org News Partner Science news on Phys.org
FIXD tells car drivers via smartphone what is wrong
Team pioneers strategy for creating new materials
Team defines new biodiversity metric
fleem
#2
Sep23-11, 09:12 AM
P: 461
After you're done with those buffers you need to free them. google malloc and free. There is no garbage collection in C/C++ like there is in higher level languages.
tangodirt
#3
Sep23-11, 09:32 AM
P: 47
Right, so I have already tried to free(contiguousMemory), but it throws invalid read errors (see first post), probably because matrix is a pointer to an array of pointers, and not a true array structure in itself.

fleem
#4
Sep23-11, 09:46 AM
P: 461
Memory Leak with C

That's why I said "after you're done with those buffers". With your current design, neither block should be freed until you are done with the matrix. Then, when you are done, both blocks must be freed.
rcgldr
#5
Sep23-11, 09:50 AM
HW Helper
P: 7,131
When done with the matrix data try code like this:

int *pContMem;
int **pMatrix;
// ...
    pMatrix = AllocateMatrix(...);
// ...
    pContMem = pMatrix[0];
    free(pMatrix);
    free(pContMem);
// ...
tangodirt
#6
Sep23-11, 11:10 AM
P: 47
Here's what I've got so far (stripping out everything else):

// HEADER
int ** AllocateMatrix(int size);
int n = 512;
int **a;

int main() {
    a = AllocateMatrix(n);
   
    // DO stuff

    int i = 0;
    for (i = 0; i < n; i++) {
        free(a[i]);
    }

    free(a);
}
But, I'm getting invalid free()'s:

==26080== Memcheck, a memory error detector
==26080== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==26080== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==26080== Command: ./MatrixMultiply -k optimized -n 256
==26080==

==26080== Invalid free() / delete / delete[]
==26080==    at 0x4A05A31: free (vg_replace_malloc.c:325)
==26080==    by 0x401129: main (in /home/tango/MatrixMultiplyProfile/MatrixMultiply)
==26080==  Address 0x4c2d440 is 1,024 bytes inside a block of size 262,144 free'd
==26080==    at 0x4A05A31: free (vg_replace_malloc.c:325)
==26080==    by 0x401129: main (in /home/tango/MatrixMultiplyProfile/MatrixMultiply)
==26080==
==26080== Invalid free() / delete / delete[]
==26080==    at 0x4A05A31: free (vg_replace_malloc.c:325)
==26080==    by 0x401139: main (in /home/tango/MatrixMultiplyProfile/MatrixMultiply)
==26080==  Address 0x4c6dcc0 is 1,024 bytes inside a block of size 262,144 free'd
==26080==    at 0x4A05A31: free (vg_replace_malloc.c:325)
==26080==    by 0x401139: main (in /home/tango/MatrixMultiplyProfile/MatrixMultiply)
==26080==
==26080== Invalid free() / delete / delete[]
==26080==    at 0x4A05A31: free (vg_replace_malloc.c:325)
==26080==    by 0x40114D: main (in /home/tango/MatrixMultiplyProfile/MatrixMultiply)
==26080==  Address 0x4cae540 is 1,024 bytes inside a block of size 262,144 free'd
==26080==    at 0x4A05A31: free (vg_replace_malloc.c:325)
==26080==    by 0x40114D: main (in /home/tango/MatrixMultiplyProfile/MatrixMultiply)
==26080==
==26080==
==26080== HEAP SUMMARY:
==26080==     in use at exit: 0 bytes in 0 blocks
==26080==   total heap usage: 6 allocs, 771 frees, 792,576 bytes allocated
==26080==
==26080== All heap blocks were freed -- no leaks are possible
==26080==
==26080== For counts of detected and suppressed errors, rerun with: -v
==26080== ERROR SUMMARY: 765 errors from 3 contexts (suppressed: 4 from 4)
Hurkyl
#7
Sep23-11, 11:18 AM
Emeritus
Sci Advisor
PF Gold
Hurkyl's Avatar
P: 16,091
Pay careful attention to what you're allocating and what you're freeing. They need to match up exactly.

You malloc exactly two times. So you should free exactly two times.

(Your latest try frees n+1 times, so it fails that check, even before we worry if you're freeing the right pointers)


You need to free the pointer value that, in the function AllocateMatrix, was allocated stored in contiguousMemory , and you need to free the pointer value that was allocated and stored in matrix

You can obtain both of those pointer values in your main routine. One should be obvious. The other should might require a bit of thought.
Pythagorean
#8
Sep23-11, 11:38 AM
PF Gold
Pythagorean's Avatar
P: 4,287
I'm afraid language of languages without garbage collection
tangodirt
#9
Sep23-11, 11:44 AM
P: 47
Quote Quote by Hurkyl View Post
Pay careful attention to what you're allocating and what you're freeing. They need to match up exactly.

You malloc exactly two times. So you should free exactly two times.

(Your latest try frees n+1 times, so it fails that check, even before we worry if you're freeing the right pointers)


You need to free the pointer value that, in the function AllocateMatrix, was allocated stored in contiguousMemory , and you need to free the pointer value that was allocated and stored in matrix

You can obtain both of those pointer values in your main routine. One should be obvious. The other should might require a bit of thought.
The reason I put it in a loop is because I thought the matrix was allocated as a 2D matrix. At least, when I wrote my function to perform the multiplication, I addressed each element in the matrix as a 2D array (or an array of arrays): a[j][i].

However, after looking at the code further, it appears that the first element of the matrix (0) is simply a reference to the contiguousMemory, right? Because i = 0, so i*size = 0, so you're left with only contiguousMemory.

So, in order to clear contiguousMemory, you have to clear the first element of the matrix: free(a[0]), like rcgldr mentioned with his code sample. That said, I still am not exactly sure why or how this works, only that it does work. Doesn't every other element in matrix also reference contiguousMemory?
Hurkyl
#10
Sep23-11, 01:25 PM
Emeritus
Sci Advisor
PF Gold
Hurkyl's Avatar
P: 16,091
Quote Quote by tangodirt View Post
Doesn't every other element in matrix also reference contiguousMemory?
Every element of the array pointed to by matrix points somewhere into the array whose address was in contiguousMemory.

However, most of those pointer values were not the return value of malloc, and so it would be incorrect to free them.

And since continguousMemory == matrix[0], calling free on the latter is identical to calling free on the former.
JulsSmile
#11
Dec9-11, 12:10 PM
P: 10
To find where the leak is necessary to use a debugger. For windows deleaker is good .
phiby
#12
Dec11-11, 02:32 PM
P: 77
Quote Quote by tangodirt View Post
Here's what I've got so far (stripping out everything else):

// HEADER
int ** AllocateMatrix(int size);
int n = 512;
int **a;

int main() {
    a = AllocateMatrix(n);
   
    // DO stuff

    int i = 0;
    for (i = 0; i < n; i++) {
        free(a[i]);
    }

    free(a);
}
But, I'm getting invalid free()'s:

This is wrong.
You do 2 mallocs in your code but you do (1 + n) frees in your code. That's obviously wrong.
You need 2 frees corresponding to 2 mallocs.

The correct sequence is already given in the post above yours - http://www.physicsforums.com/showpos...10&postcount=5
JulsSmile
#13
May10-12, 07:46 PM
P: 10
+5
Filip Larsen
#14
May11-12, 02:45 AM
PF Gold
Filip Larsen's Avatar
P: 960
Alternativly, instead of two mallocs, a = malloc(size1) and b = malloc(size2), you can allocate both with one malloc, like a = malloc(size1+size2) and b = a + size1. Then you only need to do free(a) when done.
chiro
#15
May11-12, 03:37 AM
P: 4,573
I see a number of problems.

It seems like you want to allocate some memory for a matrix. This implies that you want to return from your AllocateMatrix function, a pointer that corresponds to allocated memory that is your matrix.

If this is the case, you do not want to free this variable if you are going to use it later outside the scope of your function in whatever code. It also means if this ie the case that you have the responsibility to free the code later on when you do not need it.

If you know the dimensions of your matrix and just need to allocate an empty matrix (with some specific initialization like say a zero-matrix, identity and so on) then just allocate the memory and initialize the matrix.

The structure of your matrix operations will depend on how you index everything. You can either index it by having the memory reference the first row, then second row, and so on or you could represent the first column, then second column and so on. The important thing is that you reference each element the same way for every operation. Also if you pass your matrix to some external library, you need to make sure that the data format it expects is what it should expect.

For initializing a matrix will all zero's, you can just use a zero function after you allocate the matrix. One line will be for malloc, the other for zeroing the memory (either loop or just calling a standard zeroing function) and with all of this, you will only have three lines in your function (one for malloc, one for zero, one to return the pointer to your new matrix data structure).
JulsSmile
#16
Oct26-12, 07:04 AM
P: 10
I can recomend valgrind)) windetector is not bad


Register to reply

Related Discussions
The gulf oil leak General Physics 29
How to let the cold work shape memory alloy to acquire shape memory properties Materials & Chemical Engineering 2
The leak General Discussion 44
Verbal memory/semantic memory, also long-term memory Social Sciences 0