Program with realloc behave differently in Valgrind

Question

I wrote a function to read a string with fgets that uses realloc() to make the buffer grow when needed:

char * read_string(char * message){
    printf("%s", message);
    size_t buffsize = MIN_BUFFER;
    char *buffer = malloc(buffsize);
    if (buffer == NULL) return NULL;
    char *p;
    for(p = buffer ; (*p = getchar()) != '\n' && *p != EOF ; ++p)
        if (p - buffer == buffsize - 1) {
            buffer = realloc(buffer, buffsize *= 2) ;
            if (buffer == NULL) return NULL;
        }
    *p = 0;
    p = malloc(p - buffer + 1);
    if (p == NULL) return NULL;
    strcpy(p, buffer);
    free(buffer);
    return p;
}

I compiled the program and tried it, and it worked like expected. But when I run it with valgrind, the function returns NULL when the read string is >= MIN_BUFFER and valgrind says:

(...)
==18076==  Invalid write of size 1
==18076==    at 0x8048895: read_string (programme.c:73)
==18076==    by 0x804898E: main (programme.c:96)
==18076==  Address 0x41fc02f is 0 bytes after a block of size 7 free'd
==18076==    at 0x402BC70: realloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==18076==    by 0x8048860: read_string (programme.c:76)
(...)
==18076== Warning: silly arg (-48) to malloc()
(...)

I added a printf statement between *p=0; and p=malloc... and it confirmed that the arg passed had a value of -48. I didn't know that programs don't run the same way when launched alone and with valgrind. Is there something wrong in my code or is it just a valgrind bug?

Sidenote: your code is leaking memory if `realloc()` returns `NULL` - use a temporary variable and free the previous pointer before returning `NULL` in this case. — , Dec 09 '12 at 21:27

score 7 · Accepted Answer · answered Dec 09 '12 at 21:27

7

When you realloc the buffer, your pointer 'p' still points at the old buffer.

That will stomp memory, and also cause future allocations to use bogus values.

answered Dec 09 '12 at 21:27

JasonD

16,464
2
29
44

Thanks to you all: @JasonD, @davak, @H2CO3, @MSN. Your answers were helpful. I changed the code to: `if (p - buffer == taillbuff - 1) { char *r = realloc(buffer, taillbuff *= 2) ; if (r == NULL) { free(buffer); return NULL; } else buffer = r; p = buffer + taillbuff/2 - 1; }` – nh2 Dec 10 '12 at 19:03

score 2 · Answer 2 · answered Dec 09 '12 at 22:26

realloc returns a pointer to a new buffer of the requested size with the same contents as the pointer passed in, assuming that the pointer passed in was previously returned by malloc or realloc. It does not guarantee that it's the same pointer. Valgrind very likely modifies the behavior of realloc, but keeps it within the specification.

Since you are resizing memory in a loop, you would be better served by tracking your position in buffer as an offset from the beginning of buffer rather than a pointer.

davak · Answer 3 · 2012-12-09T22:50:46.457

1

As man 3 realloc says

...The function may move the memory block to a new location.

What this means, is that

p = malloc(p - buffer + 1);

is the problem. If realloc() was called, buffer might be pointing to a new block of memory and expression

(p - buffer)

does not make any sense.

edited Dec 09 '12 at 22:50

answered Dec 09 '12 at 21:45

davak

313
3
6

1

The "EDITED" paragraph is incorrect: `p - buffer + 1` is not a pointer, it's a number of type `ptrdiff_t`. `ptrdiff_t` is a signed integral type, and as such can be cast to compatible integral types, such as `size_t`. Also, your choice of `char` type for `p` in the modified code limits the buffer to 127 bytes, after which it overflows and thrashes the heap by writing into unallocated memory. – user4815162342 Dec 09 '12 at 22:21
Thanks for updating the answer. In C-speak, `p - buffer` is *undefined behavior*, which means that not only do you get a nonsensical result, but a program can crash on some architectures or compilers. The behavior is undefined because one is only allowed to subtract pointers to objects in the same array; quoting §6.5.6.3.9: "When two pointers are subtracted, both shall point to elements of the same array object, or one past the last element of the array object." (The standard also explicitly defines in §4.2 that a violated "shall" constraint constitutes undefined behavior.) – user4815162342 Dec 10 '12 at 07:17

Program with realloc behave differently in Valgrind

3 Answers3