Calling free() causes program to crash
Tony_C
I come across this weird issue when using free() and malloc(). The
oringinal work is with YACC and LEX. I extract some code from it to
show what the problem is.
There are three tokens to be processed by function dummy(int index):
char* yytext_c = "anotherword";
char* yytext_a = "signed";
char* yytext_b = "thelongestofall";
function dummy(int index) compares the last token name read with
some constant string, and then saves the current token in 'last_name'.
Its only argument 'index' tell it which token (yytext_a or yytext_b or
yytext_c) to process.
'last_name' is declared and intialized as:
static char* last_name=NULL;
so that it may be re-used the next time dummy is called.
The logic seems simple (spot any errors?). However, the execution
crashes. By tracing its execution, the program crashes at this line:
free(last_name);
I am not posting the error message depends on the programming
environment and run-time library.
There are ways to avoid the crash as I figured out today, but i do
not really understand:
- in branch 1, 2 and 3, if we comment out this line
last_name[i] = '\0';
the program will run smoothly.
- if we change the initial value of the three tokens, e.g.:
char* yytext_b = "anotherword";
char* yytext_c = "signed";
char* yytext_a = "thelongestofall";
we too may avoid the crash.
I run out of ideas after screw around for several hours. Any
suggestions on what went wrong?
thanks a lot.
--------------------
-----------------------------------------------------------------------
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char* yytext_c = "anotherword";
char* yytext_a = "signed";
char* yytext_b = "thelongestofall";
void dummy(int index){
static char* last_name=NULL;
int len,i;
char* t1 = "signed";
char* t2 = "unsigned";
if (last_name!=NULL)
printf("******* last token = %s \n", last_name);
if (last_name!=NULL && strcmp(last_name,t1)!=0 &&
strcmp(last_name,t2)!=0)
printf("--- rule 1.1.15 --- violated --- \n");
if( last_name != NULL )
{
free(last_name);
}
if (index==0) // branch 1, save yytext_a
{len = strlen(yytext_a)+1;
last_name =(char*) malloc(len*sizeof(char));
for (i=0; i<len; i++){
last_name[i] = yytext_a[i];
}
last_name[i] = '\0';
}
if (index==1) // branch 2, save yytext_b
{len = strlen(yytext_b)+1;
last_name =(char*) malloc(len*sizeof(char));
for (i=0; i<len; i++){
last_name[i] = yytext_b[i];
}
last_name[i] = '\0';
}
if (index>1) // branch 3, save yytext_c
{
len = strlen(yytext_c)+1;
last_name =(char*) malloc(len*sizeof(char));
for (i=0; i<len; i++){
last_name[i] = yytext_c[i];
}
last_name[i] = '\0';
}
}
int main(){
int j = 0;
int limit = 5;
while (j<limit) {
dummy(j);
j++;
}
return(0);
}
--
comp.lang.c.moderated - moderation address: cl...@plethora.net -- you must
have an appropriate newsgroups line in your header for your mail to be seen,
or the newsgroup name in square brackets in the subject line. Sorry.
Alain Ketterlin
> void dummy(int index){
> static char* last_name=NULL;
Suppose you arrive here with a non-NULL last_name.
> if( last_name != NULL )
> {
> free(last_name);
At this point, last_name still points to the freed memory.
> }
> if (index==0) // branch 1, save yytext_a
[...]
You're reaching a case where none of these tests is true (and last_name
still points to freed memory, because it has not been reassigned).
Then, the next time you call dummy, (with last_name still != NULL but
incorrectly pointing to freed memory), you call free() again. Bang.
Double-free.
I didn't really read all of your code, but it seems that you repeatedly
duplicate constant strings. Why not just carry pointers around, and
avoid all the malloc/free stuff. (And check strdup() if my remark is
nonsense.) Also, use const, as much as you can.
-- Alain.
P/S: please avoid wide lines on Usenet.
Gordon Burditt
>oringinal work is with YACC and LEX. I extract some code from it to
>show what the problem is.
If you commit undefined behavior, free() may crash. Typical no-nos
that may cause free() to crash include:
- Buffer overflows of malloc()ed memory. Actually, *any* buffer overflow.
This seems to be your problem. See below.
- Freeing something twice.
- Freeing something not allocated by malloc() and friends, such
as quoted string constants, auto variables, and static variables.
- Storing something in a buffer after it's been freed.
free(NULL) is always safe with a prototype in scope.
********ERROR*********** You allocated len bytes for last_name, and
then you wrote on last_name[len] here, which
is outside the range of allocated memory.
> }
> if (index==1) // branch 2, save yytext_b
> {len = strlen(yytext_b)+1;
> last_name =(char*) malloc(len*sizeof(char));
> for (i=0; i<len; i++){
> last_name[i] = yytext_b[i];
> }
> last_name[i] = '\0';
********ERROR*********** You allocated len bytes for last_name, and
then you wrote on last_name[len] here, which
is outside the range of allocated memory.
>
> }
>
> if (index>1) // branch 3, save yytext_c
> {
> len = strlen(yytext_c)+1;
> last_name =(char*) malloc(len*sizeof(char));
> for (i=0; i<len; i++){
> last_name[i] = yytext_c[i];
> }
> last_name[i] = '\0';
********ERROR*********** You allocated len bytes for last_name, and
then you wrote on last_name[len] here, which
is outside the range of allocated memory.
Jasen Betts
> Hi All,
> I come across this weird issue when using free() and malloc(). The
> oringinal work is with YACC and LEX. I extract some code from it to
> show what the problem is.
your re-implementation of strdup is at fault.
> {
> len = strlen(yytext_a)+1;
> last_name =(char*) malloc(len*sizeof(char));
> for (i=0; i<len; i++){
> last_name[i] = yytext_ac[i];
> }
> last_name[i] = '\0';
> }
After you can't spot the error I suggest you just use this instead.
last_name = strdup(yytext_a)
the same error is repeated for yytext_b and yytext_c too
also that code dies on the next round if dummy(-1) is called.
FWIW sizeof(char) is, by definition, always 1
--- news://freenews.netfront.net/ - complaints: ne...@netfront.net ---
Dag-Erling Smørgrav
add "last_name = NULL" after free() and your double-free will go away.
DES
--
Dag-Erling Smørgrav - d...@des.no
john
> have an appropriate newsgroups line in your header for your mail to be seen,
> or the newsgroup name in square brackets in the subject line. Sorry.
Tony: I made a few small changes, as you can see below. Not many;
from the changes maybe you can spot what the differences are. I put
down the results.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char* yytext_c = "anotherword";
char* yytext_a = "signed";
char* yytext_b = "thelongestofall";
void dummy(int index) {
static char* last_name = NULL;
int len, i;
char* t1 = "signed";
char* t2 = "unsigned";
if (last_name != NULL)
printf("******* last token = %s \n", last_name);
if (last_name != NULL && strcmp(last_name, t1) !=0 &&
strcmp(last_name,t2) !=0)
printf("--- rule 1.1.15 --- violated --- \n");
/*
*/
if (index==0) { // branch 1, save
yytext_a
len = 1 + strlen(yytext_a);
last_name = (char*) malloc(len*sizeof(char));
for (i = 0; i < (len - 1); i++)
last_name[i] = yytext_a[i];
last_name[i] = '\0';
}
else if (index == 1) { // branch 2, save
yytext_b
len = 1 + strlen(yytext_b);
last_name = (char*) malloc(len*sizeof(char));
for (i = 0; i < (len - 1); i++)
last_name[i] =
yytext_b[i];
last_name[i] = '\0';
}
else if (index > 1) { // branch 3, save
yytext_c
len = 1 + strlen(yytext_c);
last_name = (char*) malloc(len*sizeof(char));
for (i = 0; i < (len - 1); i++)
last_name[i] =
yytext_c[i];
last_name[i] = '\0';
}
else if (last_name != NULL)
free(last_name);
}
int main() {
int j = 0;
int limit = 5;
while (j < limit) {
dummy(j);
j++;
}
return(0);
}
/*
** Output from the above:
john@john-laptop:~/s-r/trans$ gcc prob_from_comp_mod.c -Wall -Wstrict-
prototypes -std=gnu99 -pedantic -o prob
prob_from_comp_mod.c:90: warning: function declaration isn’t a
prototype
john@john-laptop:~/s-r/trans$ ./prob
******* last token = signed
******* last token = thelongestofall
--- rule 1.1.15 --- violated ---
******* last token = anotherword
--- rule 1.1.15 --- violated ---
******* last token = anotherword
--- rule 1.1.15 --- violated ---
**
*/
Hans-Bernhard Bröker
> There are ways to avoid the crash as I figured out today, but i do
> not really understand:
> - in branch 1, 2 and 3, if we comment out this line
> last_name[i] = '\0';
> the program will run smoothly.
That's because that line is your bug. At this point in your program,
last_name points to an malloc()ed object of `len' bytes, and `i' equals
`len' --- but the allowed indices of your array are only [0 .. len-1].
The above line writes one byte _behind_ the end of your allocated space.
That causes undefined behaviour, which you observe.
> - if we change the initial value of the three tokens, e.g.:
> char* yytext_b = "anotherword";
> char* yytext_c = "signed";
> char* yytext_a = "thelongestofall";
> we too may avoid the crash.
That's a coincidence. It doesn't depend on the content of the strings,
but rather on their lengths. Some malloc() implementations round up the
allocated sizes to the next multiple of some granularity. Array index
overflows that fall into that extra space may fail to break the program.
Kevin English
<climb...@gmail.com> wrote:
> There are ways to avoid the crash as I figured out today, but i do
>not really understand:
> - in branch 1, 2 and 3, if we comment out this line
> last_name[i] = '\0';
> the program will run smoothly.
>
Look at the value of i when you make that assignment and you should
see your problem.
--
Kevin English
lawrenc...@siemens.com
> Tony_C wrote:
>
> > There are ways to avoid the crash as I figured out today, but i do
> > not really understand:
> > - in branch 1, 2 and 3, if we comment out this line
> > last_name[i] = '\0';
> > the program will run smoothly.
>
> That's because that line is your bug. At this point in your program,
> last_name points to an malloc()ed object of `len' bytes, and `i' equals
> `len' --- but the allowed indices of your array are only [0 .. len-1].
> The above line writes one byte _behind_ the end of your allocated space.
And it's not needed because the immediately preceding loop has already
copied the null terminator from the original string into your array, so
there's no need to add another one.
--
Larry Jones
Well of course the zipper's going to get stuck if everyone
stands around WATCHING me! -- Calvin