CODEWITHSUNDEEP
cmemory-leaksvalgrind
Andrea Ciceri
Andrea Ciceri

Reputation: 471

Valgrind crashes and gives me this invalid realloc warning

t_syntaxTree is a struct defined as:

typedef struct t_syntaxTree {
    char nodeName[16];
    int nodesLen;
    struct t_syntaxTree** nodes;
} t_syntaxTree;

I wrote the function treeToStr to transformate a syntax tree in a string, the code should be self explanatory. The output string format is lisp-like, for example an output string can be (or (and true true) (> b 3)). The following code works but if I execute the program with valgrind it crashes with a segmentation fault. Furthermore, before the crash, valgrind tells me that some of my realloc calls are invalid.

int recTreeToStr(t_syntaxTree* t, char* str, int len) {
    if (t->nodesLen == 0) {
        int nLen = len + strlen(t->nodeName);
        str = realloc(str, sizeof(char) * nLen);
        strcat(str, t->nodeName);
        return nLen;
    }
    else {
        int nLen = len + strlen(t->nodeName) + 1;
        str = realloc(str, sizeof(char) * nLen);
        strcat(str, "(");
        strcat(str, t->nodeName);

        for (int i=0; i<t->nodesLen; i++) {
            nLen++;
            str = realloc(str, sizeof(char) * nLen);
            strcat(str, " ");
            nLen = recTreeToStr(t->nodes[i], str, nLen);
        }

        nLen++;
        str = realloc(str, sizeof(char) * nLen);
        strcat(str, ")");

        return nLen;
    }
}

char* treeToStr(t_syntaxTree* tree) {
    char* str=malloc(sizeof(char));
    str[0] = '\0';
    recTreeToStr(tree, str, 1);
    return str;
}

This is the valgrind report before the crash (immediatly after this message the program crash with a segmentation fault error):

==26561== Invalid free() / delete / delete[] / realloc()
==26561==    at 0x4839D7B: realloc (vg_replace_malloc.c:826)
==26561==    by 0x10B3C4: recTreeToStr (cooper.c:443)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B4C0: treeToStr (cooper.c:459)
==26561==    by 0x10B4F2: cooper (cooper.c:467)
==26561==    by 0x10922E: main (test.c:6)
==26561==  Address 0x4a6aee0 is 0 bytes inside a block of size 15 free'd
==26561==    at 0x4839D7B: realloc (vg_replace_malloc.c:826)
==26561==    by 0x10B310: recTreeToStr (cooper.c:431)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B4C0: treeToStr (cooper.c:459)
==26561==    by 0x10B4F2: cooper (cooper.c:467)
==26561==    by 0x10922E: main (test.c:6)
==26561==  Block was alloc'd at
==26561==    at 0x4839D7B: realloc (vg_replace_malloc.c:826)
==26561==    by 0x10B3C4: recTreeToStr (cooper.c:443)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B4C0: treeToStr (cooper.c:459)
==26561==    by 0x10B4F2: cooper (cooper.c:467)
==26561==    by 0x10922E: main (test.c:6)
==26561== 
==26561== Invalid read of size 1
==26561==    at 0x10B3DF: recTreeToStr (cooper.c:444)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B4C0: treeToStr (cooper.c:459)
==26561==    by 0x10B4F2: cooper (cooper.c:467)
==26561==    by 0x10922E: main (test.c:6)
==26561==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==26561== 
==26561== 
==26561== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==26561==  Access not within mapped region at address 0x0
==26561==    at 0x10B3DF: recTreeToStr (cooper.c:444)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B4C0: treeToStr (cooper.c:459)
==26561==    by 0x10B4F2: cooper (cooper.c:467)
==26561==    by 0x10922E: main (test.c:6)
==26561==  If you believe this happened as a result of a stack
==26561==  overflow in your program's main thread (unlikely but
==26561==  possible), you can try to increase the size of the
==26561==  main thread stack using the --main-stacksize= flag.
==26561==  The main thread stack size used in this run was 8388608.
==26561== 
==26561== HEAP SUMMARY:
==26561==     in use at exit: 1,249 bytes in 46 blocks
==26561==   total heap usage: 224 allocs, 178 frees, 21,257 bytes allocated
==26561== 
==26561== 17 bytes in 1 blocks are definitely lost in loss record 2 of 11
==26561==    at 0x4839D7B: realloc (vg_replace_malloc.c:826)
==26561==    by 0x10B310: recTreeToStr (cooper.c:431)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B420: recTreeToStr (cooper.c:445)
==26561==    by 0x10B4C0: treeToStr (cooper.c:459)
==26561==    by 0x10B4F2: cooper (cooper.c:467)
==26561==    by 0x10922E: main (test.c:6)
==26561== 
==26561== LEAK SUMMARY:
==26561==    definitely lost: 17 bytes in 1 blocks
==26561==    indirectly lost: 0 bytes in 0 blocks
==26561==      possibly lost: 0 bytes in 0 blocks
==26561==    still reachable: 1,232 bytes in 45 blocks
==26561==         suppressed: 0 bytes in 0 blocks
==26561== Reachable blocks (those to which a pointer was found) are not shown.
==26561== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==26561== 
==26561== For counts of detected and suppressed errors, rerun with: -v
==26561== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)

Upvotes: 0

Views: 382

Answers (2)

jwismar
jwismar

Reputation: 12258

In order to successfully reallocate your char* you're actually going to need to pass around a char**. The char* you have in your method is a copy of the real pointer you think you're reallocating.

Something like:

int recTreeToStr(t_syntaxTree* t, char** str, int len) {
...
    *str = realloc(*str, sizeof(char) * nLen);
...
}

and 

recTreeToStr(tree, &str, 1);

I'm also concerned that you're allocating memory for the existing string + the length of the node name, but then also including '(' and ')' and ' ' and not allocating space for those. (Unless I'm blind.)

Upvotes: 1

Zan Lynx
Zan Lynx

Reputation: 54325

After your first call to realloc you have freed str. It's been replaced with a new allocation. And then your function returns without storing the new value of str anywhere.

So yes you are using invalid values of str after recTreeToStr returns.

Upvotes: 2

Related Questions