free(): invalid pointer Aborted (code dumped) (ubuntu C)

Question

I have a big project in C that has somewhere around 2 thousand lines of code.
I have in this project a linked list that I am using to store data of the program, at the end of the program I am calling a function I wrote to free the memory of the linked list. but for some reason I am getting this error:

free(): invalid pointer
Aborted (code dumped)

I have my linked list seperated in its own header file so it doesn't matter where in my code I am adding nodes to that lists they all just called insertSymbol to insert new list so I know what the nodes are allocated in the heap memory.

Can someone tell me what is wrong with the way I am doing this and also explain to me why does it happens??

this is my SymbolTable.h file

#ifndef SYMBOL_TABLE_H
#define SYMBOL_TABLE_H

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "constants.h"

/**
 *  a struct holding the SymbolTable information
*/
typedef struct SymbolTable {
    char* symbol;
    unsigned int identifier:2;
    int value;
    struct SymbolTable *next;
} SymbolTable;

extern SymbolTable* symbol_table;

/**
 * Function insertSymbol
 * ---------------------
 * gets the symbol information (e.g. symbol name, identefier and value)
 * and insert it to the symbol table
 * 
 * @param symbol: string holding the symbol name
 * @param identerfier: an integer holding the id of the symbol (e.g. .code, .mdefine or .data)
 * @param value: the value of the symbol in the memory
*/
void insertSymbol(const char* symbol, const int identifier, const int value);

/**
 * Function lookupSymbol
 * ----------------------
 * searching a symbol in the symbol table
 * 
 * @param symbol: a string holding the symbol name to search
 * 
 * @return a pointer to the symbol matching the name passed as an argument
*/
SymbolTable* lookupSymbol(const char* symbol);

/**
 * Function freeSymbolTable
 * ------------------------
 * releasing the memory that was allocated to the symbol_table data structure
*/
void freeSymbolTable();

#endif

and this is the source code for it:

#include "../headers/symbolTable.h"

SymbolTable* symbol_table = NULL;

void insertSymbol(const char *symbol, const int identifier, const int value)
{
    SymbolTable **temp = &symbol_table;
    while (*temp != NULL) {
        temp = &(*temp)->next;
    }
    *temp = (SymbolTable*)calloc(1, sizeof(SymbolTable));
    (*temp)->symbol = (char*)calloc(strlen(symbol) + 1, sizeof(char));
    strcpy((*temp)->symbol, symbol);
    (*temp)->identifier = identifier;
    (*temp)->value = value;
    (*temp)->next = NULL;
}

SymbolTable* lookupSymbol(const char *symbol)
{
    SymbolTable *temp = symbol_table;

    /* iterating over the symbol table and returning a pointer to the symbol if it was found otherwise returning null */
    while (temp != NULL) {
        if (strcmp(temp->symbol, symbol) == 0) {
            return temp;
        }
        temp = temp->next;
    }
    return NULL;
}

void freeSymbolTable()
{
    /* iterating over the symbol table and releasing the memory that was allocated for it */
    SymbolTable *temp = NULL;
    while (symbol_table != NULL)
    {
        temp = symbol_table;
        symbol_table = symbol_table->next;
        free(temp);
        temp = NULL;
    }
}

Note that all of the parts in my projects that are adding nodes to the list are just calling insertSymbol with the data that they want and not adding themself so the problem is somewhere around here.

Answer 1

You're not posting other parts of the code, so ...

A few issues ...

1. Your insertSymbol may work, but it's very convoluted (using ** pointers throughout). It looks buggy/error prone.
2. freeSymbolTable has a bug: It does not free symbol (allocated in insertSymbol).
3. strdup is easier to use.
4. lookupSymbol can be cleaned up a bit.
5. Using "non-standard" return statements (vs. a single return at the bottom) should be avoided where possible.
6. Using temp a lot (vs. more descriptive names)
7. It's easier [and, IMO, less error prone] to use for loops for traversing linked lists.
8. SymbolTable is misnamed. It isn't a table. It's a symbol table entry. [Although I didn't change it below], it's better renamed as SymbolEntry or just Symbol.
9. This would free up SymbolTable to be used for multiple tables with: typedef struct { Symbol *head; Symbol *tail; } SymbolTable;

Here is the refactored code. It is annotated. (coded but not compiled nor tested):

void
insertSymbol(const char *symbol, const int identifier, const int value)
{

    // find the end/tail of the list
    SymbolTable *prev = NULL;
    for (SymbolTable *cur = symbol_table;  cur != NULL;  cur = cur->next)
        prev = cur;

    // get space for new node
    SymbolTable *newsym = calloc(1, sizeof(SymbolTable));

    // fill in node
    // Note that strdup is much simpler to use
    newsym->symbol = strdup(symbol);
    newsym->identifier = identifier;
    newsym->value = value;

    // add to tail of non-empty list
    if (prev != NULL)
        prev->next = newsym;

    // add as first element of list
    else
        symbol_table = newsym;
}

SymbolTable *
lookupSymbol(const char *symbol)
{
    SymbolTable *match = symbol_table;

    /* iterating over the symbol table and returning a pointer to the symbol
        if it was found otherwise returning null */
    for (;  match != NULL;  match = match->next) {
        if (strcmp(match->symbol, symbol) == 0)
            break;
    }

    return match;
}

void
freeSymbolTable(void)
{
    /* iterating over the symbol table and releasing the memory that was
        allocated for it */

    SymbolTable *next;
    for (SymbolTable *cur = symbol_table;  cur != NULL;  cur = next) {
        // prevent use-after-free bug
        next = cur->next;

// NOTE/BUG: this was missing!
        free(cur->symbol);

        free(cur);
    }

    symbol_table = NULL;
}

---

UPDATE:

Here's the code generalized to accept a pointer to a symbol table instead of using a global symbol_table variable:

typedef struct {
    Symbol *head;
#if 0
    Symbol *tail;
#endif
} SymbolTable;

void
insertSymbol(SymbolTable *table,const char *symbol,
    const int identifier, const int value)
{

    // find the end/tail of the list
    Symbol *prev = NULL;
    for (Symbol *cur = table->head;  cur != NULL;  cur = cur->next)
        prev = cur;

    // get space for new node
    Symbol *newsym = calloc(1, sizeof(Symbol));

    // fill in node
    // Note that strdup is much simpler to use
    newsym->symbol = strdup(symbol);
    newsym->identifier = identifier;
    newsym->value = value;

    // add to tail of non-empty list
    if (prev != NULL)
        prev->next = newsym;

    // add as first element of list
    else
        table->head = newsym;
}

SymbolTable *
lookupSymbol(SymbolTable *table,const char *symbol)
{
    Symbol *match = table->head;

    /* iterating over the symbol table and returning a pointer to the symbol
        if it was found otherwise returning null */
    for (;  match != NULL;  match = match->next) {
        if (strcmp(match->symbol, symbol) == 0)
            break;
    }

    return match;
}

void
freeSymbolTable(SymbolTable *table)
{
    /* iterating over the symbol table and releasing the memory that was
        allocated for it */

    Symbol *next;
    for (Symbol *cur = table->head;  cur != NULL;  cur = next) {
        // prevent use-after-free bug
        next = cur->next;

// NOTE/BUG: this was missing!
        free(cur->symbol);

        free(cur);
    }

    table->head = NULL;
}

---

UPDATE #2:

Well, I didnt post other parts of my code is because the only time where the symbol table is modified or nodes are inserted to it is through these functions, other part of the code just call them – roee

Obviously, you have UB (undefined behavior) of some sort. The issue may be elsewhere in [seemingly] unrelated code (that could be corrupting the heap because you're overrunning an array (e.g.)).

That's why we want an MRE. For C, we would like a single .c file [with (e.g.) #include <stdio.h>]. But, no (e.g.) #include "myprogram.h". For runtime errors, it should compile cleanly. We should be able to download, build, and run it on our local systems [if we so choose].

I'm guessing that you're just starting out. In the grand scheme of things, 2000 lines isn't all that much for an experienced programmer. Does it fit within the 30,000 char posting limit here? Or, consider github or pastebin.

And I changed my code to what you wrote and it still not working for me and I get the same error, do you have any idea why? – roee

The code I posted has been used many times in many places before. So, I have a high degree of confidence in it.

So, at this point, I'd want to run the code locally. I would run it under gdb, compile with -fsanitize [as S.P.D. suggested], instrument the code with debug statements, etc.

Of course, you can do these things as well. What results did you get (particularly for the -fsanitize)?

There are a number of other tricks that can be used. How to proceed really depends on the exact code you have.

also I cannot use the strdup function and get implicit declaration of function, maybe it does not supported when using these flags when compling the code: -Wall -ansi -pedantic – roee

Yes, probably, these options prevent it.

1. Why are you forced to use -ansi -pedantic?
2. What C standard are you using (e.g.) what -std= are you using)?
3. As of C23, strdup is now part of the C standard but it's been part of posix for at least a decade.
4. Is this for a school assignment or is some [silly] company rule or did you just add these options in an attempt to debug your problem?

If all else fails, it's trivial to implement. Most big projects will probe for it during configuration (e.g autoconf). They will provide an implementation if the system does not as it's silly to replicate the code inline in multiple places:

char *
strdup(const char *src)
{
    size_t len = strlen(src) + 1;

    char *src = malloc(len);

    memcpy(dst,src,len);

    return src;
}