Big leak fished!

[David Hakim]

Hi Dave,
${
int i = 10000;
while (i-- > 0) {
int j = 10000;
while(j-- > 0) {
int k;
}
}
}$
so I noticed that if you take away the declaration and you have only
${
int i = 10000;
while (i-- > 0) {
int j = 10000;
while(j-- > 0) {
}
}
}$
moto does not die.
So the problem is that when moto_freeFrame frees all the frame
variables, it
does not free the strdupped string representing the name and this can
lead to
megabytes of shared memory, in case of such loops (even if probably
they are
if(env->mode != COMPILER_MODE) {
moto_freeVal(env,var->vs);
} else {
opool_release(env->valpool,var->vs);
}
free(var->n); // <-- this was missing
free(var);

I believe the above variable name free was left out on-purpose for the
cases when class member variables were pushed onto the frame. I notice
specifically in motoi_callMDF we have:

e = vec_elements(mcd->memberVarNames);
while(enum_hasNext(e)){
char* varn=(char*)enum_next(e);

/* Fake variable shadowing by simply not loading in a member var
with the same name as an argument */

if(moto_getFrameVar(env,varn) == NULL) {
MotoVar* var = moto_createVar(
env,varn,
mcd_getMemberType(mcd,varn),
0,
'\1',
mci+mcd_getMemberOffset(mcd,varn)
);
stab_put(env->frame->symtab, var->n, var);
}
}
enum_free(e);

which definitely does not duplicate the variable name. Also in
motoi_callMDF we don't duplicate the argument names when calling moto
defined functions:

/* Push arguments onto the frame by declaring them */
for(i=0;i<argc;i++){
UnionCell* argdec = uc_operand(argListUC,i);
UnionCell* atype_uc = uc_operand(argdec,0);

int argdecDim ;

char *aname;
MotoVar* avar;
MotoType* atype;

atype = motox_extractType(atype_uc);
aname = uc_str(argdec,1);
argdecDim = uc_opcount(uc_operand(argdec, 2));

/* Declare the argument variable */
avar = moto_declareVar(env, aname, atype->name, argdecDim, '\0');

moto_setVarVal(env, avar, args[i]);
}

So I'm thinking that calling moto_strdup on the variable name in
motoi_declare is not necessary (since variable names aren't
specifically freed until the end of a page anyhow). I'm going to try
removing it from motoi_declare by changing

/* Get the variable name and dimension */
varn = moto_strdup(env, uc_str(declarator_uc, 0));
vdim = uc_opcount(uc_operand(declarator_uc, 1));

to

/* Get the variable name and dimension */
varn = uc_str(declarator_uc, 0);
vdim = uc_opcount(uc_operand(declarator_uc, 1));

and see what breaks :)

-Dave

BTW: after making this change all tests still pass

Now it works. Slowly, but it works!
Stefano

[David Hakim]

So there was one more moto_strdup in motoi that was being used to
create MotoVar names. This one was in motoi_dereference_lval. It looks
like I can also safely change it from

/* Get the name of the member variable to dereference to from operand
1 */
varn = moto_strdup(env, uc_str(p, 1));

to

/* Get the name of the member variable to dereference to from operand
1 */
varn = uc_str(p, 1);

By making these changes I think we avoid the varnames hashtable and
associated functions you propose in your patch. Is that right ?

-Dave

Post by David Hakim

Hi Dave,
${
int i = 10000;
while (i-- > 0) {
int j = 10000;
while(j-- > 0) {
int k;
}
}
}$
so I noticed that if you take away the declaration and you have only
${
int i = 10000;
while (i-- > 0) {
int j = 10000;
while(j-- > 0) {
}
}
}$
moto does not die.
So the problem is that when moto_freeFrame frees all the frame
variables, it
does not free the strdupped string representing the name and this can
lead to
megabytes of shared memory, in case of such loops (even if probably
they are
if(env->mode != COMPILER_MODE) {
moto_freeVal(env,var->vs);
} else {
opool_release(env->valpool,var->vs);
}
free(var->n); // <-- this was missing
free(var);

I believe the above variable name free was left out on-purpose for the
cases when class member variables were pushed onto the frame. I notice
e = vec_elements(mcd->memberVarNames);
while(enum_hasNext(e)){
char* varn=(char*)enum_next(e);
/* Fake variable shadowing by simply not loading in a member var
with the same name as an argument */
if(moto_getFrameVar(env,varn) == NULL) {
MotoVar* var = moto_createVar(
env,varn,
mcd_getMemberType(mcd,varn),
0,
'\1',
mci+mcd_getMemberOffset(mcd,varn)
);
stab_put(env->frame->symtab, var->n, var);
}
}
enum_free(e);
which definitely does not duplicate the variable name. Also in
motoi_callMDF we don't duplicate the argument names when calling moto
/* Push arguments onto the frame by declaring them */
for(i=0;i<argc;i++){
UnionCell* argdec = uc_operand(argListUC,i);
UnionCell* atype_uc = uc_operand(argdec,0);
int argdecDim ;
char *aname;
MotoVar* avar;
MotoType* atype;
atype = motox_extractType(atype_uc);
aname = uc_str(argdec,1);
argdecDim = uc_opcount(uc_operand(argdec, 2));
/* Declare the argument variable */
avar = moto_declareVar(env, aname, atype->name, argdecDim, '\0');
moto_setVarVal(env, avar, args[i]);
}
So I'm thinking that calling moto_strdup on the variable name in
motoi_declare is not necessary (since variable names aren't
specifically freed until the end of a page anyhow). I'm going to try
removing it from motoi_declare by changing
/* Get the variable name and dimension */
varn = moto_strdup(env, uc_str(declarator_uc, 0));
vdim = uc_opcount(uc_operand(declarator_uc, 1));
to
/* Get the variable name and dimension */
varn = uc_str(declarator_uc, 0);
vdim = uc_opcount(uc_operand(declarator_uc, 1));
and see what breaks :)
-Dave
BTW: after making this change all tests still pass

Now it works. Slowly, but it works!
Stefano