Re: Re: Memory fix

David Hakim <dhakim-Gkm/TONP9n1Wk0Htik3J/[email protected]> Sun, 3 Aug 2003 14:10:38 -0400
Newsgroups gmane.comp.lang.moto.devel
Message-ID <[email protected]>
So the root of the problem in ary3.moto is that in motoi_assign we are 
evaluating the lvalue twice but only throwing the MotoVar returned from 
the evaluation away once.

At the start of motoi_assign we evaluate the lvalue as a MotoVar 
(assigning it to dest) so we can assign to it later

	UnionCell *lval = uc_operand(p,0);
	int op = uc_operand(p, 1)->opcell.opcode;
	MotoVar * dest = NULL;

	/* Save rval UnionCell */
	env->rval_uc = uc_operand(p, 2);

	/* First ... either get value name or process operand 0 */

	if (lval->type == VALUE_TYPE && lval->valuecell.kind == ID_VALUE) {
	
		char *destn = uc_str(p, 0);
		dest = moto_getVar(env, destn);

	} else if((lval->type == OP_TYPE && lval->opcell.opcode == ARRAY_LVAL) 
||
		 (lval->type == OP_TYPE && lval->opcell.opcode == DEREFERENCE_LVAL)) {
		
		dest = NULL;
		
		/* Evaluate the first operand to get the Member MotoVar */
		motoi(lval);

		dest= (MotoVar*)opstack_pop(env);
	
	}

Then, if we are dealing with a potentially overloaded operator we

	else {

		/* do a math operation and leave the result on the top of the stack */
		MotoVal *v1;		
		MotoVal *v2;		
				
		switch (op) {
			case MATH_ADD:	  /* add-assignment */
			case MATH_SUB:	  /* sub-assignment */
			case MATH_MULT:  /* mult-assignment */
			case MATH_DIV:	  /* div-assignment */
			case MATH_MOD:	  /* mod-assignment */
				if (env->rval_uc != NULL) {
					motoi(uc_operand(p, 2));
					env->rval_uc = NULL;
				}
				
				/* FIXME!!! nasty but nice */
				
				if( lval->opcell.opcode == DEREFERENCE_LVAL )
					motoi_dereference_rval(uc_operand(p, 0));
				else
					motoi(uc_operand(p, 0));

				v1 = opstack_pop(env);
				v2 = opstack_pop(env);

				if(motox_try_overloaded_method_op(op,v1,v2,NULL) != MOTO_OK) {
					motoi_domath(v1, v2, op);
					moto_freeVal(env,v1);
					moto_freeVal(env,v2);
				}
			break;
		}	

	}

The really bad thing here is

				if( lval->opcell.opcode == DEREFERENCE_LVAL )
					motoi_dereference_rval(uc_operand(p, 0));
				else
					motoi(uc_operand(p, 0));

But in theory we should not need to re-evaluate lval operand if its a 
DEREFERENCE_LVAL or an ARRAY_LVAL since we already did that earlier (to 
get the dest MotoVar). Moreover since we are not doing a specific check 
here for ARRAY_LVAL (which pops a MotoVar as opposed to a MotoVal onto 
the opstack) we get into deeper trouble when we execute:

v1 = opstack_pop(env);

since we've just assigned a MotoVal* a MotoVar* ...

More investigation is definitely required but that's what I see so far

-Dave

On Sunday, August 3, 2003, at 01:27  PM, David Hakim wrote:

> Hey Stefano!
>
> 	I'm taking a look at your patch now. I replicated at least one 
> interpreter memory leak in ary3.moto . Is this the one you are 
> referring to ? I haven't gone through the other perf tests yet. First 
> I want to add this one to the main test list. Then fix it. Then look 
> at the others :)
>
> On Saturday, August 2, 2003, at 07:37  PM, Stefano Corsi wrote:
>
>> Hi Dave,
>>
>> I'm working to fix some memory problems. Please give a look at this 
>> patch and
>> see if it makes sense... I've fixed a bug in motoi_new (the resulting 
>> MotoVal
>> of motoi_callMDF was not free'd before) and I've create a new strdup 
>> routine
>> for variable names, to be able to free them separately when a frame 
>> is free'd
>> (or when a dereference name is pushed/popped as MotoVar). There are 
>> however
>> some place where we leak memory... I would like to be able to run all 
>> the
>> tests in perfetest.tar.gz in interpreted mode (doesn't matter for how 
>> long
>> :)) without getting memory errors... The funny thing with this patch 
>> is that
>> things go slightly faster!
>>
>> The patch applies with -p1 on moto_0_20_0...
>>
>> Stefano<motomem.patch>
>
>