realloc(): invalid next size

[Deephay]

Greetings all,

I have a program that used the realloc() function to change the
allocated size of a buffer, the program works with some arguments, but
with some other arguments, it will show me the error message like:

*** glibc detected *** realloc(): invalid next size: 0x0804c3a8 ***

and then I inserted a perror("realloc") to see what happend, it says that:

realloc: Illegal seek
the realloc() is in a loop:

for (m = 0; m < len; m++) {
if (isspace(data[m]) || ispunct(data[m]) ||
isdigit(data[m]))
printf("%c", data[m]);
else {
p = min(strcspn(&data[m], " "),
strcspn(&data[m], "\t"),
strcspn(&data[m], "\r"),
strcspn(&data[m], "\n"));
key = realloc(key, p);
strncpy(key, &data[m], p);
key[p] = '\0';
trans(key, p);
m = m + p - 1;
}
}

and the "key" is already malloced before the loop:

char *key = malloc(1);

Any suggestion could be helpful, thx very much!

Deephay

 

[Richard Bos]

I have a program that used the realloc() function to change the
allocated size of a buffer, the program works with some arguments, but
with some other arguments, it will show me the error message like:

*** glibc detected *** realloc(): invalid next size: 0x0804c3a8 ***

and then I inserted a perror("realloc") to see what happend, it says that:

realloc: Illegal seek
the realloc() is in a loop:

for (m = 0; m < len; m++) {
if (isspace(data[m]) || ispunct(data[m]) ||
isdigit(data[m]))
printf("%c", data[m]);
else {
p = min(strcspn(&data[m], " "),
strcspn(&data[m], "\t"),
strcspn(&data[m], "\r"),
strcspn(&data[m], "\n"));

Why not simply use strcspn(&data[m], " \t\r\n")?

key = realloc(key, p);
strncpy(key, &data[m], p);
key[p] = '\0';
trans(key, p);
m = m + p - 1;
}
}

and the "key" is already malloced before the loop:

char *key = malloc(1);

Hard to say what the real problem is without any object or function
definitions, but two questions spring to mind:
- does this occur immediately, or after a few iterations;
- what actually is the value of p (and if this code isn't complete, of
key) at the point where it produces the error?

Richard

 

[Deephay]

I have a program that used the realloc() function to change the
allocated size of a buffer, the program works with some arguments, but
with some other arguments, it will show me the error message like:

*** glibc detected *** realloc(): invalid next size: 0x0804c3a8 ***

and then I inserted a perror("realloc") to see what happend, it says that:

realloc: Illegal seek
the realloc() is in a loop:

for (m = 0; m < len; m++) {
if (isspace(data[m]) || ispunct(data[m]) ||
isdigit(data[m]))
printf("%c", data[m]);
else {
p = min(strcspn(&data[m], " "),
strcspn(&data[m], "\t"),
strcspn(&data[m], "\r"),
strcspn(&data[m], "\n"));

Why not simply use strcspn(&data[m], " \t\r\n")?

I have to get the index where the first "space" locates.

key = realloc(key, p);
strncpy(key, &data[m], p);
key[p] = '\0';
trans(key, p);
m = m + p - 1;
}
}

and the "key" is already malloced before the loop:

char *key = malloc(1);

Hard to say what the real problem is without any object or function
definitions, but two questions spring to mind:
- does this occur immediately, or after a few iterations;
- what actually is the value of p (and if this code isn't complete, of
key) at the point where it produces the error?

the error occurred after a few iterations, the first realloc will always
success.
the p is always larger than 0, I have tested. Actually it will be a
quite normal
value where the error produced, say, 7, 8, or whatever...

What I think now is that it might be a bug of glibc...
I searched with google and found some unsolved problem like the one I have.

What I did now is, declare a key[50] static, this works fine.
I still want to make sure what the problem is, though.

Richard

Thanks!

Deephay

 

[Michael Wojcik]

I have a program that used the realloc() function to change the
allocated size of a buffer, the program works with some arguments, but
with some other arguments, it will show me the error message like:

*** glibc detected *** realloc(): invalid next size: 0x0804c3a8 ***

glibc is off-topic for comp.lang.c, but this is glibc telling you
that you have invoked Undefined Behavior. Probably you have
corrupted glibc's housekeeping data by performing an illegal
operation on an object with dynamic storage duration, such as writing
past the end of an allocated area.

and then I inserted a perror("realloc") to see what happend, it says that:

What will happen is that you will get a meaningless message written
to stderr. realloc does not set errno, so perror will report whatever
happens to have already been in errno.

the realloc() is in a loop:

for (m = 0; m < len; m++) {
if (isspace(data[m]) || ispunct(data[m]) ||
isdigit(data[m]))
printf("%c", data[m]);
else {
p = min(strcspn(&data[m], " "),
strcspn(&data[m], "\t"),
strcspn(&data[m], "\r"),
strcspn(&data[m], "\n"));
key = realloc(key, p);

Wrong for two reasons. You failed to check whether realloc
succeeded, and if it failed, you just lost the old value of "key"
and so introduced a memory leak.

The result of realloc should always be stored in a temporary
variable and should be checked for null. If it is null, remember
to free the old value:

char *newkey;
newkey = realloc(key, p);
if (! newkey)
{
free(key);
[perform error handling]
}
key = newkey;

strncpy(key, &data[m], p);
key[p] = '\0';

You just wrote past the end of the allocated area. Since key is an
area of p bytes, valid indices are 0 through p-1, inclusive.

and the "key" is already malloced before the loop:

char *key = malloc(1);

Did you check to see whether malloc succeeded?

Any suggestion could be helpful, thx very much!

Read the comp.lang.c FAQ (http://www.c-faq.com is one source). Get a
copy of the C standard. C makes little effort to protect you from
yourself; you will only produce reliable, correct C code by learning
the language and its pitfalls.

--
Michael Wojcik (e-mail address removed)

The lecturer was detailing a proof on the blackboard. He started to say,
"From the above it is obvious that ...". Then he stepped back and thought
deeply for a while. Then he left the room. We waited. Five minutes
later he returned smiling and said, "Yes, it is obvious", and continued
to outline the proof. -- John O'Gorman

 

[Ralf Damaschke]

I have a program that used the realloc() function to
change the
allocated size of a buffer, the program works with some
arguments, but with some other arguments, it will show me
the error message like:

*** glibc detected *** realloc(): invalid next size:
0x0804c3a8 ***

Why not simply use strcspn(&data[m], " \t\r\n")?

I have to get the index where the first "space" locates.

Ok, that would be "strcspn(&data[m], " \t\r\n")+1" if there are
any spaces, but you did not add 1 in your code either.

key = realloc(key, p);
strncpy(key, &data[m], p);
key[p] = '\0';

I still want to make sure what the problem is, though.

The last assignment above invokes undefined behavior even if
realloc() did not return NULL.

Ralf

 

[Richard Bos]

p = min(strcspn(&data[m], " "),
strcspn(&data[m], "\t"),
strcspn(&data[m], "\r"),
strcspn(&data[m], "\n"));

Why not simply use strcspn(&data[m], " \t\r\n")?

I have to get the index where the first "space" locates.

That one call does the same thing as those four calls plus a properly
written min() function.

key = realloc(key, p);
strncpy(key, &data[m], p);
key[p] = '\0';
trans(key, p);
m = m + p - 1;
and the "key" is already malloced before the loop:

char *key = malloc(1);

Hard to say what the real problem is without any object or function
definitions, but two questions spring to mind:
- does this occur immediately, or after a few iterations;
- what actually is the value of p (and if this code isn't complete, of
key) at the point where it produces the error?

the error occurred after a few iterations, the first realloc will always
success.
the p is always larger than 0, I have tested. Actually it will be a
quite normal value where the error produced, say, 7, 8, or whatever...

What I think now is that it might be a bug of glibc...

That is the last thing I would consider.

I searched with google and found some unsolved problem like the one I have.

You can find a great many confused bug-writers on the 'web, yes, many of
them blaming their tools for their own mistakes.

What I did now is, declare a key[50] static, this works fine.

It probably works around a bug - using strncpy() without thorough
understanding is asking for one, btw - and leaves a time bomb in your
code. What happens if you encounter a token larger than 50 characters?
Worse, what if you encounter one of _precisely_ 50 characters?

I still want to make sure what the problem is, though.

Without more code which demonstrates the problem precisely, it is
impossible to be certain. If the above is your exact code, you have
several off-by-one errors; then again, if the above is your exact code,
it doesn't compile, because there is a lot missing.

If you want more help, what you should do now is the following:
- make a copy of your existing code;
- whittle it down to the smallest _compilable_ program which still
exhibits the same problem;
- post _exactly_ that code. Paste, do not retype it into your post,
otherwise you'll make typos.

Richard

 

[Pedro Graca]

The result of realloc should always be stored in a temporary
variable and should be checked for null. If it is null, remember
to free the old value:

char *newkey;
newkey = realloc(key, p);
if (! newkey)
{
free(key);
[perform error handling]
}
key = newkey;

Uh? Doesn't realloc(), when it doesn't fail, call free() all by itself
if there's a need for that? Also suppose the reallocated memory doesn't
'move' (newkey == key); doing a "free(key);" now could be disastrous.

 

[Chris Torek]

Uh? Doesn't realloc(), when it doesn't fail, call free() all by itself
if there's a need for that?

No!

With a few exceptions (noted below), realloc() is essentially an
optimized version of the following:

void *realloc(void *old, size_t newsize) {
size_t oldsize = __some_sort_of_magic_done_here(old);
void *new;

new = malloc(newsize);
if (new != NULL) {
memcpy(new, old, oldsize < newsize ? oldsize : newsize);
free(old);
}
return new;
}

(where the "magic" function obtains the size previously malloc()ed,
or the size of the previously malloc()ed region, whichever is
larger, based on "old", or returns 0 if old==NULL).

Note specifically that "old" is *not*, I repeat NOT, free()d if
"new" memory is not obtainable.

Also suppose the reallocated memory doesn't 'move' (newkey == key);

In this case, the return value from realloc() is necessarily not
NULL (well, except if old==NULL).

In both C89 and C99, realloc(p, 0) (where p!=NULL) is equivalent
to free(p). In at least C89 (and perhaps both C89 and C99),
realloc(NULL, 0) does something no one can quite explain.
(Seriously: C89 specifically said that realloc(NULL,n) was equivalent
to malloc(n), and malloc(0) *could* be equivalent to malloc(1); it
then also said that realloc(p,0) was equivalent to free(p); so what
then is realloc(NULL,0) -- is it like malloc(0) and hence like
malloc(1), or is it just free(NULL)?)

 

[Marc Thrun]

The result of realloc should always be stored in a temporary
variable and should be checked for null. If it is null, remember
to free the old value:

char *newkey;
newkey = realloc(key, p);
if (! newkey)
{
free(key);
[perform error handling]
}
key = newkey;

Uh? Doesn't realloc(), when it doesn't fail, call free() all by itself
if there's a need for that? Also suppose the reallocated memory doesn't
'move' (newkey == key); doing a "free(key);" now could be disastrous.

realloc() returns a null-pointer when it fails, hence the if branch is
only taken after a failure in realloc().

 

[Pedro Graca]

The result of realloc should always be stored in a temporary
variable and should be checked for null. If it is null, remember
to free the old value:

char *newkey;
newkey = realloc(key, p);
if (! newkey)
{
free(key);
[perform error handling]
}
key = newkey;

Uh? Doesn't realloc(), when it doesn't fail, call free() all by itself
if there's a need for that? Also suppose the reallocated memory doesn't
'move' (newkey == key); doing a "free(key);" now could be disastrous.

realloc() returns a null-pointer when it fails, hence the if branch is
only taken after a failure in realloc().

Ah! Of course. Thank you.
(sorry Michael)

 

[Pedro Graca]

No!

[snip comprehensive answer]

Thank you very much for the comprehensive answer.

 

[Rod Pemberton]

No!

With a few exceptions (noted below), realloc() is essentially an
optimized version of the following:

void *realloc(void *old, size_t newsize) {
size_t oldsize = __some_sort_of_magic_done_here(old);
void *new;

new = malloc(newsize);
if (new != NULL) {
memcpy(new, old, oldsize < newsize ? oldsize : newsize);
free(old);
}
return new;
}

Hmm, no NULL check for old?

(Seriously: C89 specifically said that realloc(NULL,n) was equivalent
to malloc(n), and malloc(0) *could* be equivalent to malloc(1); it
then also said that realloc(p,0) was equivalent to free(p); so what
then is realloc(NULL,0) -- is it like malloc(0) and hence like
malloc(1), or is it just free(NULL)?)

If your question wasn't rhetorical, this realloc probably answers your
question. It has undefined behavior since it doesn't determine the 'magic
mystery size' of s1...

void *my_realloc (void *s1, size_t size)
{
void *s2=NULL;

if (size!=0||s1==NULL)
s2 = malloc(size);
else
free(s1);
if (s1!=NULL)
memcpy(s2, s1, size);
return(s2);
}


Rod Pemberton

 

[tudoxxx]

I have a program that used the realloc() function to change the
allocated size of a buffer, the program works with some arguments, but
with some other arguments, it will show me the error message like:

*** glibc detected *** realloc(): invalid next size: 0x0804c3a8 ***

glibc is off-topic for comp.lang.c, but this is glibc telling you
that you have invoked Undefined Behavior. Probably you have
corrupted glibc's housekeeping data by performing an illegal
operation on an object with dynamic storage duration, such as writing
past the end of an allocated area.

and then I inserted a perror("realloc") to see what happend, it says that:

What will happen is that you will get a meaningless message written
to stderr. realloc does not set errno, so perror will report whatever
happens to have already been in errno.

the realloc() is in a loop:

for (m = 0; m < len; m++) {
if (isspace(data[m]) || ispunct(data[m]) ||
isdigit(data[m]))
printf("%c", data[m]);
else {
p = min(strcspn(&data[m], " "),
strcspn(&data[m], "\t"),
strcspn(&data[m], "\r"),
strcspn(&data[m], "\n"));
key = realloc(key, p);

Wrong for two reasons. You failed to check whether realloc
succeeded, and if it failed, you just lost the old value of "key"
and so introduced a memory leak.

The result of realloc should always be stored in a temporary
variable and should be checked for null. If it is null, remember
to free the old value:

char *newkey;
newkey = realloc(key, p);
if (! newkey)
{
free(key);
[perform error handling]
}
key = newkey;

strncpy(key, &data[m], p);
key[p] = '\0';

You just wrote past the end of the allocated area. Since key is an
area of p bytes, valid indices are 0 through p-1, inclusive.

and the "key" is already malloced before the loop:

char *key = malloc(1);

Did you check to see whether malloc succeeded?

Any suggestion could be helpful, thx very much!

Read the comp.lang.c FAQ (http://www.c-faq.com is one source). Get a
copy of the C standard. C makes little effort to protect you from
yourself; you will only produce reliable, correct C code by learning
the language and its pitfalls.

thx a lot for those suggestions!

 

[tudoxxx]

p = min(strcspn(&data[m], " "),
strcspn(&data[m], "\t"),
strcspn(&data[m], "\r"),
strcspn(&data[m], "\n"));

Why not simply use strcspn(&data[m], " \t\r\n")?

I have to get the index where the first "space" locates.

That one call does the same thing as those four calls plus a properly
written min() function.

key = realloc(key, p);
strncpy(key, &data[m], p);
key[p] = '\0';
trans(key, p);
m = m + p - 1;
and the "key" is already malloced before the loop:

char *key = malloc(1);

Hard to say what the real problem is without any object or function
definitions, but two questions spring to mind:
- does this occur immediately, or after a few iterations;
- what actually is the value of p (and if this code isn't complete, of
key) at the point where it produces the error?

the error occurred after a few iterations, the first realloc will always
success.
the p is always larger than 0, I have tested. Actually it will be a
quite normal value where the error produced, say, 7, 8, or whatever...

What I think now is that it might be a bug of glibc...

That is the last thing I would consider.

I searched with google and found some unsolved problem like the one I have.

You can find a great many confused bug-writers on the 'web, yes, many of
them blaming their tools for their own mistakes.

What I did now is, declare a key[50] static, this works fine.

It probably works around a bug - using strncpy() without thorough
understanding is asking for one, btw - and leaves a time bomb in your
code. What happens if you encounter a token larger than 50 characters?
Worse, what if you encounter one of _precisely_ 50 characters?

I still want to make sure what the problem is, though.

Without more code which demonstrates the problem precisely, it is
impossible to be certain. If the above is your exact code, you have
several off-by-one errors; then again, if the above is your exact code,
it doesn't compile, because there is a lot missing.

If you want more help, what you should do now is the following:
- make a copy of your existing code;
- whittle it down to the smallest _compilable_ program which still
exhibits the same problem;
- post _exactly_ that code. Paste, do not retype it into your post,
otherwise you'll make typos.

Richard

thx for the suggestions, Richard, I'll try to exam the code again.

 

[Pedro Graca]

If your question wasn't rhetorical, this realloc probably answers your
question. It has undefined behavior since it doesn't determine the 'magic
mystery size' of s1...

void *my_realloc (void *s1, size_t size)
{
void *s2=NULL;

if (size!=0||s1==NULL)
s2 = malloc(size);
else
free(s1);
if (s1!=NULL)
memcpy(s2, s1, size);
return(s2);
}

Hmm, there seems to be something wrong with this my_realloc():

new_pointer = my_realloc(NULL, 100); /* ok */
new_pointer = my_realloc(NULL, 0); /* ok */
new_pointer = my_realloc(old_pointer, 100); /* old_pointer not free'd */
new_pointer = my_realloc(old_pointer, 0); /* memcpy(NULL, ????, 0); */

In the last example, will the call to memcpy() invoke UB? Or, because
size is 0 (zero), it doesn't matter what the pointers point to?

 

[Michael Wojcik]

The result of realloc should always be stored in a temporary
variable and should be checked for null. If it is null, remember
to free the old value:

char *newkey;
newkey = realloc(key, p);
if (! newkey)
{
free(key);
[perform error handling]
}
key = newkey;

Uh? Doesn't realloc(), when it doesn't fail, call free() all by itself
if there's a need for that? Also suppose the reallocated memory doesn't
'move' (newkey == key); doing a "free(key);" now could be disastrous.

The code above only frees key if realloc failed.

 

[Rod Pemberton]

Hmm, there seems to be something wrong with this my_realloc():

new_pointer = my_realloc(NULL, 100); /* ok */
new_pointer = my_realloc(NULL, 0); /* ok */
new_pointer = my_realloc(old_pointer, 100); /* old_pointer not free'd */

That's a simple fix:
if (s1!=NULL)
{
memcpy(s2, s1, size);
free(s1);
}

new_pointer = my_realloc(old_pointer, 0); /* memcpy(NULL, ????, 0); */

You've got that wrong. The last case is memcpy(new_pointer,old_pointer,0);
i.e., new_pointer = my_realloc(old_pointer, 0); /*
memcpy(new_pointer,old_pointer,0); */

If size is zero, s1 is freed (which doesn't set s1 to NULL) and s2 is
returned which is NULL.

In the last example, will the call to memcpy() invoke UB? Or, because
size is 0 (zero), it doesn't matter what the pointers point to?

As I said, you've got the last one wrong. No NULL is ever passed to
memcpy(), but a size zero is. To eliminate the size zero problem, you need
to know the old size, and check that it isn't zero. Which as Chris Torek's
example shows, it's based on compiler secrets.

The point of my reply was that Chris Torek's solution will pass NULL's for
'old' that _could_ "invoke" (I really don't like "invoke," it implies a
guaranted action...) UB for the first two situations:

new_pointer = my_realloc(NULL, 100); /* not Ok for ChrisT example*/
new_pointer = my_realloc(NULL, 0); /* not Ok for ChrisT example*/

(If you're keeping score, Chris has two UB, one size zero problem. And,
mine had one coding error, and one size zero problem.)

A closer corrected realloc, would be some merger of mine and Chris', with
zero correction for size:

void *realloc (void *s1, size_t size)
{
void *s2=NULL;
size_t oldsize = __some_sort_of_magic_done_here(old);

if (size!=0||s1==NULL)
s2 = malloc(size);
else
free(s1);
if (s1!=NULL)
{
size=oldsize<size?oldsize:newsize;
if(size==0)
size=1;
memcpy(s2, s1, size);
free(s1);
}
return(s2);
}


HTH,

Rod Pemberton

 

[Rod Pemberton]

That's a simple fix:
if (s1!=NULL)
{
memcpy(s2, s1, size);
free(s1);
}


You've got that wrong. The last case is memcpy(new_pointer,old_pointer,0);
i.e., new_pointer = my_realloc(old_pointer, 0); /*
memcpy(new_pointer,old_pointer,0); */

If size is zero, s1 is freed (which doesn't set s1 to NULL) and s2 is
returned which is NULL.


As I said, you've got the last one wrong. No NULL is ever passed to
memcpy(), but a size zero is. To eliminate the size zero problem, you need
to know the old size, and check that it isn't zero. Which as Chris Torek's
example shows, it's based on compiler secrets.

The point of my reply was that Chris Torek's solution will pass NULL's for
'old' that _could_ "invoke" (I really don't like "invoke," it implies a
guaranted action...) UB for the first two situations:
example*/

(If you're keeping score, Chris has two UB, one size zero problem. And,
mine had one coding error, and one size zero problem.)

A closer corrected realloc, would be some merger of mine and Chris', with
zero correction for size:

void *realloc (void *s1, size_t size)
{
void *s2=NULL;
size_t oldsize = __some_sort_of_magic_done_here(old);

if (size!=0||s1==NULL)
s2 = malloc(size);
else
free(s1);
if (s1!=NULL)
{
size=oldsize<size?oldsize:newsize;

Correction:
size=oldsize<size?oldsize:size;

 

[Pedro Graca]

The result of realloc should always be stored in a temporary
variable and should be checked for null. If it is null, remember
to free the old value:

char *newkey;
newkey = realloc(key, p);
if (! newkey)
{
free(key);
[perform error handling]
}
key = newkey;

Uh? Doesn't realloc(), when it doesn't fail, call free() all by itself
if there's a need for that? Also suppose the reallocated memory doesn't
'move' (newkey == key); doing a "free(key);" now could be disastrous.

The code above only frees key if realloc failed.

I think I finally understand why I saw something wrong in that code.

If the reallocation fails and NULL is assigned to newkey, it's supposed
that the "[perform error handling]" doesn't return, so the code will
never get to the "key = newkey;" statement.
I was imagining that the code would go on, and didn't understand the
need to "free(key);" and lose all references to its memory.

 

[Pedro Graca]

That's a simple fix:

[fix incorporated in the code above, check '>' marks]

You've got that wrong. The last case is memcpy(new_pointer,old_pointer,0);
i.e., new_pointer = my_realloc(old_pointer, 0); /*
memcpy(new_pointer,old_pointer,0); */

If size is zero, s1 is freed (which doesn't set s1 to NULL) and s2 is
returned which is NULL.

Right. Let's run "my_realloc(valid_old_pointer, 0);" step by step ...

void *my_realloc (void *s1, size_t size)
{
void *s2=NULL;
/* s1 --> valid_old_pointer
* s2 --> NULL
* size --> 0 */

if (size!=0||s1==NULL)
s2 = malloc(size);
else
free(s1);
/* s1 --> free'd valid_old_pointer
* s2 --> NULL
* size --> 0 */

if (s1!=NULL)
memcpy(s2, s1, size);
/* And here we go ...
* s2 is NULL, s1 is the free'd valid_old_pointer, size is 0
* so
*
* ...
*
* memcpy(NULL, ????, 0); */

return(s2);
}

As I said, you've got the last one wrong. No NULL is ever passed to
memcpy(), but a size zero is.

Where did I go wrong in my step-by-step run of my_realloc()?

To eliminate the size zero problem, you need
to know the old size, and check that it isn't zero. Which as Chris Torek's
example shows, it's based on compiler secrets.

Yes, I understand this is nothing more than an exercise in second
guessing the compiler secrets ... but I feel it's a good exercise to aid
in developing my knowledge of C.