2

im new to C and memory-management and tried to find memory leaks with valgrind, and i found all problems in lexer->src_size I do not know why memory leaks when I use strlen()

valgrind:

==49058== Memcheck, a memory error detector
==49058== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==49058== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==49058== Command: ./loop -s
==49058== 
==49058== Invalid read of size 1
==49058==    at 0x483FF54: strlen (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==49058==    by 0x1093D1: Lexer_Init (lexer.c:11)
==49058==    by 0x109A44: Loop_Compile (loop.c:6)
==49058==    by 0x109AD4: main (in /home/ali/Desktop/loop/loop)
==49058==  Address 0x4a502be is 0 bytes after a block of size 30 alloc'd
==49058==    at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==49058==    by 0x109366: IO_ReadFile (io.c:17)
==49058==    by 0x109ACC: main (in /home/ali/Desktop/loop/loop)
==49058== 
==49058== Conditional jump or move depends on uninitialised value(s)
==49058==    at 0x483FC57: strcat (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==49058==    by 0x10955A: Lexer_LexID (lexer.c:54)
==49058==    by 0x109691: Lexer_NextToken (lexer.c:82)
==49058==    by 0x109A80: Loop_Compile (loop.c:8)
==49058==    by 0x109AD4: main (in /home/ali/Desktop/loop/loop)
==49058==  Uninitialised value was created by a heap allocation
==49058==    at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==49058==    by 0x1093B6: Lexer_Init (lexer.c:9)
==49058==    by 0x109A44: Loop_Compile (loop.c:6)
==49058==    by 0x109AD4: main (in /home/ali/Desktop/loop/loop)
==49058== 
void: 0
main: 0
): 8
{: 9
==49058== Conditional jump or move depends on uninitialised value(s)
==49058==    at 0x483FC57: strcat (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==49058==    by 0x1095F3: Lexer_LexNumber (lexer.c:69)
==49058==    by 0x1096DD: Lexer_NextToken (lexer.c:86)
==49058==    by 0x109A80: Loop_Compile (loop.c:8)
==49058==    by 0x109AD4: main (in /home/ali/Desktop/loop/loop)
==49058==  Uninitialised value was created by a heap allocation
==49058==    at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==49058==    by 0x1093B6: Lexer_Init (lexer.c:9)
==49058==    by 0x109A44: Loop_Compile (loop.c:6)
==49058==    by 0x109AD4: main (in /home/ali/Desktop/loop/loop)
==49058== 
1: 2
+: 4
2: 2
==49058== Invalid read of size 1
==49058==    at 0x109454: Lexer_Advance (lexer.c:22)
==49058==    by 0x109480: Lexer_AdvanceWithToken (lexer.c:28)
==49058==    by 0x10991A: Lexer_NextToken (lexer.c:115)
==49058==    by 0x109A80: Loop_Compile (loop.c:8)
==49058==    by 0x109AD4: main (in /home/ali/Desktop/loop/loop)
==49058==  Address 0x4a502be is 0 bytes after a block of size 30 alloc'd
==49058==    at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==49058==    by 0x109366: IO_ReadFile (io.c:17)
==49058==    by 0x109ACC: main (in /home/ali/Desktop/loop/loop)
==49058== 
}: 10
==49058== 
==49058== HEAP SUMMARY:
==49058==     in use at exit: 234 bytes in 15 blocks
==49058==   total heap usage: 28 allocs, 13 frees, 5,884 bytes allocated
==49058== 
==49058== LEAK SUMMARY:
==49058==    definitely lost: 168 bytes in 10 blocks
==49058==    indirectly lost: 66 bytes in 5 blocks
==49058==      possibly lost: 0 bytes in 0 blocks
==49058==    still reachable: 0 bytes in 0 blocks
==49058==         suppressed: 0 bytes in 0 blocks
==49058== Rerun with --leak-check=full to see details of leaked memory
==49058== 
==49058== For lists of detected and suppressed errors, rerun with: -s
==49058== ERROR SUMMARY: 12 errors from 4 contexts (suppressed: 0 from 0)

io.c:


char *IO_ReadFile(const char *filename)
{
    char *buffer = 0;
    long length;
    FILE *f = fopen(filename, "rb");

    if (f)
    {
        fseek(f, 0, SEEK_END);
        length = ftell(f);
        fseek(f, 0, SEEK_SET);
        buffer = malloc(length);
        if (buffer)
        {
            fread(buffer, 1, length, f);
        }
        fclose(f);
    }

    return buffer;
}

loop.c

void Loop_Compile(char *src)
{
    lexer_t *lexer = Lexer_Init(src);
    token_t *token = 0;
    while ((token = Lexer_NextToken(lexer))->kind != TOKEN_EOF)
    {
        printf("%s: %d\n", token->value, token->kind);
    }
}

lexer.c

lexer_t *Lexer_Init(char *src)
{
    lexer_t *lexer = malloc(sizeof(lexer_t));
    lexer->src = src;
    lexer->src_size = strlen(src); // <----- Here
    lexer->index = 0;
    lexer->cc = src[lexer->index];
    return lexer;
};

and lexer struct type:

typedef struct LEXER_STRUCT
{
    char *src;
    size_t src_size;
    char cc; // cc: current char
    unsigned int index;
} lexer_t;

main.c

int main(int argc, char* argv[]) {
    if (argc < 2) {
        printf("Please Add A File\n");
        return 1;
    }
    Loop_Compile(IO_ReadFile("./examples/main.loop"));
    return 0;
}
Ali Chraghi
  • 613
  • 6
  • 16
  • 2
    I can't see anything in your posted code that could explain the valgrind error message. Please provide a [mre]. – Andreas Wenzel Apr 10 '21 at 13:56
  • I know what the problem is. I just do not know how to fix it. – Ali Chraghi Apr 10 '21 at 13:57
  • 2
    `"I know what the problem is."` -- I do not have that impression. My guess is that the problem is in some code that you did not post. I cannot verify this, because you have not provided a [mre]. – Andreas Wenzel Apr 10 '21 at 14:00
  • @chux: The line `lexer->src = src;` should not be a problem if, as OP claims, `src` indeed is a pointer to a string literal. In that case, a copy should not be necessary, unless the string is to be written. However, I have strong doubts whether OP's claim that `src` points to a string literal is accurate. Without a [mre], we cannot verify this. – Andreas Wenzel Apr 10 '21 at 14:10
  • I really do not know what you want more than that. If so I have to write the whole code here – Ali Chraghi Apr 10 '21 at 14:12
  • 3
    @AliC: Have you read the instructions on how to create a [mre]? If you are unable to reproduce the error without the full code of your program, then that is a sign that the problem lies somewhere else in your code. – Andreas Wenzel Apr 10 '21 at 14:14
  • 3
    If valgrind reports an `invalid read` in `strlen`, then that is a sign that the function argument `src` is not pointing to a valid string, or that the the string's memory has not been properly allocated (e.g. buffer overflow). This has nothing to do with a memory "leak". You might want to inspect what `src` is pointing to in a [debugger](https://stackoverflow.com/q/25385173/12149471). – Andreas Wenzel Apr 10 '21 at 14:17
  • @AndreasWenzel now you cannot help? – Ali Chraghi Apr 10 '21 at 14:31
  • 2
    @AliC: I hope you now understand the importance of providing a [mre]. Until then, your question was unanswerable. What made it especially hard to help you was that in [revision 3](https://stackoverflow.com/revisions/67034995/3) of your question, you had provided false code on how `Lexer_Init` was being called. This false code could not explain the valgrind error, only your actual code could. – Andreas Wenzel Apr 10 '21 at 14:47

2 Answers2

4

strlen(src); fails as src does not certainly point to a string.

IO_ReadFile() does not return a pointer to a string as the function fails to allocate for nor append a null character.

Try

    //buffer = malloc(length);
    buffer = malloc(length + 1);
    if (buffer)
    {
        fread(buffer, 1, length, f);
        buffer[length] = 0; // add
    }

Other issue may exist.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
3

I do not know why memory leaks when I use strlen()

Valgrind is not reporting a memory leak. It is reporting

==49058== Invalid read of size 1

and the location on which the read was requested is

==49058==  Address 0x4a502be is 0 bytes after a block of size 30 alloc'd
==49058==    at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==49058==    by 0x109366: IO_ReadFile (io.c:17)

This is an overrun of the allocated space, arising from the data whose length you are trying to compute not being null terminated. If you want the data provided by IO_ReadFile to be suitable for use with string functions then it must allocate enough space to include a terminator after the contents of the file.

It is plausible that the program behaved as you expected when not run under Valgrind because the behavior upon such an overrun is undefined. It may work in practice because the next location happens to be within the program's accessible memory space (likely) and it happens to contain a zero byte (not implausible, especially early in the run of the program). This is nevertheless a serious error.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157