realloc fails after multiple calls only when not debugging

Question

The below code occasionally fails on the buffer = (char*) realloc(buffer, allocated * sizeof(char)); call (marked down below) that I use to dynamically allocate space for a char*,by allocating 1 char initially, and doubling the allocated amount every time the memory I already have is insufficient to store the string.

I have very similar code in many other parts of my project, with the same memory allocation policy and calls (changing only the type of the void* I pass to realloc).

I am using VS2010 to debug the problem, and when I start the program on debug mode, the function always completes successfully.

However, when calling the program from the command line, there is a good chance that one of the calls to realloc will fail after some time with an "Access violation reading location" error - though it doesn't happen all the time, and only happens after the function below has been called multiple times, with many reallocations having already taken place.

What's weirder, I put some prints before and after the realloc call to assert if the pointer location was changed, and, when I did so and ran the program, the calls to realloc stopped failing randomly.

What am I doing wrong?

TOKEN
next_token_file(FILE* file, 
                STATE_MACHINE* sm, 
                STATE_MACHINE* wsssm)
{
    char* buffer = (char*) malloc(sizeof(char));
    size_t allocated = 1;
    size_t i = 0;
    while(1)
    {
    /*
    ... code that increments i by one and messes with sm a bit. Does nothing to the buffer.
    */
        // XXX: This fails when using realloc. Why?
        if(i + 1 >= allocated)
        {
            allocated = allocated << 1;
            buffer = (char*) realloc(buffer, allocated * sizeof(char));
        }
        buffer[i] = sm->current_state->state;
    /*
    ... more code that doesn't concern the buffer
    */
    }
    // Null-terminate string.
    buffer[++i] = 0;
    TOKEN t = {ret, buffer};
    return t;
}

Shouldn't this `size_t allocated = 1;` be `size_t allocated = 16;`? — alk, Aug 20 '13 at 15:16
That'd be my guess, either that or he really does want to go from 16 chars down to 2 on the first round (which would be ... strange). — WhozCraig, Aug 20 '13 at 15:21

alk · Accepted Answer · 2020-07-07T10:34:23.793

4

Due to these lines

char* buffer = (char*) malloc(16 * sizeof(char));
size_t allocated = 1;

the program shrinks buffer for the first 4 re-allocations. So the program writes to unallocated memory from i=16 on, which is undefined behaviour, so anything could happen. Also this most likely smashes the memory management which in turn makes realloc() fail.

You might like to change those two lines to be:

size_t allocated = 16; /* or = 1 if the 16 was a typo. */
char * buffer = malloc(allocated);

Other notes:

sizeof(char) is always 1.
Do not cast the result of malloc/calloc/realloc as it is not necessary nor recommended: https://stackoverflow.com/a/605858/694576.
Do check the result of system calls.

Refering the last note, the following modifications should be applied

char * buffer = malloc(allocated);

might become:

char * buffer = malloc(allocated); 
if (NULL == buffer)
{
  /* Error handling goes here. */
}

and

buffer = (char*) realloc(buffer, allocated * sizeof(char));

might become:

{
  char * pctmp = realloc(buffer, allocated);
  if (NULL == pctmp)
  {
    /* Error handling goes here. */
  }
  else
  {
    buffer = pctmp;
  }
}

edited Jul 07 '20 at 10:34

answered Aug 20 '13 at 15:20

alk

69,737
10
105
255

sizeof (char) is even grounded in since c99 as it has to be 1 Byte, so where your refering to that sizeof (char) is allways 0? – dhein Aug 20 '13 at 15:27
@Zaibis: Ouch ...`0` was a typo. Fixed. – alk Aug 20 '13 at 15:28
1

The realloc failure handling is such a common bug, using the same pointer results in a leak :) – paulm Aug 20 '13 at 15:33
Now, with this, one doubt still stands in my mind: why would that call fail only when I wasn't debugging it instead of crashing consistently? – Renan Gemignani Aug 20 '13 at 17:57
1

@RenanGemignani: You are just about to realise the irrationality of Undefined Behaviour. – alk Aug 20 '13 at 18:27
I have similar realloc() random problem, I'm trying to understand this example in order to learn where I'm doing wrong... but here I can't see the problem. It is true that allocated is initialized to 1 and buffer is initialized to a len of 16, but soon when (i + 1 > 1) then the realloc() align buffer len and "allocated" variable to the same value of 2. Isn't it? And when i + 1 > 16 then the code resize buffer to 32? Or not? I'm confused. Someone can explain please? – MaxC Jul 04 '20 at 16:54

score 1 · Answer 2 · edited Aug 20 '13 at 15:33

1

More of a comment than an answer but I don't have 50 points to comment.

This:

char* buffer = (char*) malloc(16 * sizeof(char));

should be

char* buffer = (char*) malloc(1 * sizeof(char));

or

allocated = 16.

edited Aug 20 '13 at 15:33

alk

69,737
10
105
255

answered Aug 20 '13 at 15:25

Charlie Burns

6,994
20
29

Do not cast the result of `malloc/calloc/realloc` as it is not necessary nor recommended: http://stackoverflow.com/a/605858/694576 – alk Aug 20 '13 at 15:33

dhein · Answer 3 · 2013-08-20T15:38:35.950

I dont know, when you are increasing or decreasing i. But I would bet, acording to this snippet, your problem is: your reallocating infinitly, and as your not checking for realloc is returning NULL, that will crash your programm ;)

As allready said, even the not well running pritf's are conforming it, your violating your memory block. this will happen by reallocing the memory adress which has been overwritten outside the range.(excepting its UB anyway)

Or if you try to work if an invalid return value (what is when NULL is returned, what could happen because u aren't checking it) Or if you request zerosized area(size parameter is 0) and you get returned an non zero pointer and you work with that one. But 2nd case probably wont happen in your programm ;)

realloc fails after multiple calls only when not debugging

3 Answers3