Search code examples
cpointersmemorymallocfree

Error in ./thrash: free(): invalid pointer


I realize this has been asked several times but none of the solutions offer any help for me. I am writing a lab program that allocates a large amount of memory in C specifically an array of char pointers, each of which have allocated the size of a page in memory which is 4096 bytes.

char** pgs =(char**) malloc(sizeof(char *) * pages);
if(pgs == NULL){
    printf("Failed to allocate memory");
    exit(0);
}
int i;
for(i = 0; i < pages; i++){
    pgs[i] = malloc(4096);
    /*if(pgs[i] == NULL){
    printf("Failed to allocate memory");
    exit(0);
    }*/
*pgs[i] = "\0";
/*if(pgs[i] == NULL){
    printf("Failed to allocate memory");
    exit(0);
}*/
}

In the middle of the program elements of this array are accessed and modified at random so as to induce thrashing (as part of the lab):

while(time(NULL) - startTime < seconds){
    long rando = rand() % pages;
    if(modify > 0){
        *pgs[rando]++;
    }
    else{
        long temp = *pgs[rando];
    }

At the end of the program I attempt to free this memory:

for(i = 0; i < pages; i++){
    free(pgs[i]);
}


free(pgs);

I am however getting the dread "invalid pointer" error. If anyone has any advice or knowledge on how this might be fixable, please share.


Solution

  • The program fragments you show exhibit numerous problems, some of which were identified in comments:

    • Programs should report errors on standard error, not standard output.
    • Programs should exit with a non-zero status if they fail.
    • Programs should compile without warnings.
    • Messages in general and error messages in particular should end with a newline.
    • The program only attempts to modify one byte of each page.

    However, the primary problem is that the code in the question uses *pgs[rando]++ which is intended to modify the memory that's allocated. This is equivalent to *(pgs[rando]++) which increments the pointer and then reads the value and discards it — rather than being equivalent to (*pgs[rando])++ which would modify the byte pgs[rando][0]. The code in the question should generate a warning about value computed is not used (or an error if you make sure you compile with all warnings are treated as errors). Because your code is incrementing the pointers, the values returned to to the memory allocation system with free() are not, in general, the same as the ones that the memory allocation system returned to you, so you are indeed passing invalid pointers to free().

    This code avoids the problems described above. It does a fix number of iterations and doesn't use time(). It prints the sum so that the optimizer cannot optimize away the read accesses to the memory.

    /* SO 4971-2352 */
    
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    enum { PAGESIZE = 4096 };
    
    int main(void)
    {
        int pages = PAGESIZE;
        char **pgs = (char **)malloc(sizeof(char *) * pages);
        if (pgs == NULL)
        {
            fprintf(stderr, "Failed to allocate memory\n");
            exit(EXIT_FAILURE);
        }
        for (int i = 0; i < pages; i++)
        {
            pgs[i] = malloc(PAGESIZE);
            if (pgs[i] == NULL)
            {
                fprintf(stderr, "Failed to allocate memory\n");
                exit(EXIT_FAILURE);
            }
            memset(pgs[i], '\0', PAGESIZE);     // Or use calloc()!
        }
    
        size_t sum = 0;
        for (int i = 0; i < PAGESIZE * PAGESIZE; i++)
        {
            int pagenum = rand() % pages;
            int offset = rand() % PAGESIZE;
            int modify = i & 2;
            if (modify != 0)
            {
                pgs[pagenum][offset]++;
            }
            else
            {
                sum += pgs[pagenum][offset];
            }
        }
    
        printf("Sum: 0x%.8zX\n", sum);
    
        for (int i = 0; i < pages; i++)
            free(pgs[i]);
        free(pgs);
    
        return 0;
    }
    

    I called that code thrash31.c and compiled it into thrash31 using:

    $ gcc -O3 -g -std=c11 -Wall -Wextra -Werror thrash31.c -o thrash31
    $
    

    When run with a timing program, I got the output:

    $ timecmd -u -- thrash31
    2018-04-07 15:48:58.546809 [PID 9178] thrash31
    Sum: 0x001FE976
    2018-04-07 15:48:59.355508 [PID 9178; status 0x0000]  -  0.808699s
    $
    

    So, it took about 0.8 seconds to run. The sum it generates is the same each time because the code doesn't seed the random number generator.