Re: Re: Memory fix

Ok, so I fixed the behavior  of the += *= -= /= and %= operators when 
the lvalue is a subscripted array

This fixes both the memory leak and the functional issues

The fix was to replace:

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

with

				if( lval->opcell.opcode == DEREFERENCE_LVAL ||
					lval->opcell.opcode == ARRAY_LVAL ) {	
					v1 = moto_getVarVal(env,dest);
				} else {
					motoi(uc_operand(p, 0));
					v1 = opstack_pop(env);
				}

I'm not sure if this fix is going to wreak any havoc with the operator 
overloading stuff. Stefano, you might want to take a look at any issues 
there. However all tests pass and I have added a new test to the tests 
directory

arr_assign_succ

to make sure this issue doesn't pop up again

-Dave

On Sunday, August 3, 2003, at 02:10  PM, David Hakim wrote:

> 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>
>>
>>
>
>